Re: [PATCH 0/5] Recognize Zicond extension

[Jeff Law <jeffreyalaw@gmail.com>]

On 7/28/23 00:34, Xiao Zeng wrote:

>>>
>>> Does that work for you?
>> I'm going to look at 3/5 today pretty closely.  Exposing zicond to
>> mov<node>cc is something we had implemented inside Ventana and I want to
>> compare/contrast your work with ours.
> 
> What a coincidence!
Zicond is a direct descendant of xventanacondops.  The only notable 
difference is in their encodings.

> 
>>
>> What I like about yours is it keeps all the logic in riscv.cc rather
>> than scattering it across riscv.cc and riscv.md.
> 
> Yes, when I use enough test cases, I cannot find a concise way to optimize
> all test cases. When I enumerated all possible cases in the mov<mode>cc
> function of the RISC-V backend, I found a method that satisfied me, which
> is the method in patch [3/5].
I got pulled away to another task yesterday, so didn't get as far as I 
wanted.   The biggest inight from yesterday was determining that some of 
the cases you're handling in riscv_expand_conditional_move were things 
we were doing inside ifcvt.cc.

The difference is likely because the initial work on zicond here was 
primarily driven by changes to ifcvt.  It was only after evaluating that 
initial implementation that we started to the effort to use zicond at 
RTL expansion time.

I could make a case for either approach, but the more I ponder them the 
more I'm inclined to go with something like yours.  We want to capture 
the cases implementable as a conditional move as early as possible in 
the RTL pipeline rather than relying on ifcvt to catch it later.  It 
also avoids polluting ifcvt with transformations that are only likely 
needed on risc-v.


>>
> 
> If it's just for the Zicond instruction set, is it necessary to make judgments
> outside of eq/ne? After all, it does not support comparison actions other
> than eq/ne. Of course, it is also possible to use a special technique to use
> Zicond in non eq/ne comparisons.
It's not necessary, but it's definitely helpful to cover the other 
conditions.  In fact, we can even cover a variety of fp conditions by 
utilizing the sCC type insns.


So what I'm looking at for patch #3 is to split out the costing bits 
into its own patch which can go forward immediately.  THen continue 
evaluating the best way to handle unifying the expander/canonicalization 
code.  Your testcases in patch #3 are particularly helpful to make sure 
we're not missing cases.

Jeff