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
