On Fri, Jul 28, 2023 at 11:03:00 PM Jeff Law <jeffreya...@gmail.com> wrote: > > > >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. It explains the matter.
> >> >>> >>> 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. That's why I did this optimization in riscv.cc riscv_expand_conditional_move. > > >>> >> >> 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. It would be great if we could do this. > > >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. As you expected, V2-patch[3/5] has arrived, and its address is: https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625781.html >THen continue >evaluating the best way to handle unifying the expander/canonicalization >code. That's nice. >Your testcases in patch #3 are particularly helpful to make sure >we're not missing cases. Yes, I have always believed that test cases can be redundant, but they cannot be omitted. As we all know, the compiler will always make some magical changes without our knowledge, which may not be what we expect. And test cases can help us stay away from this risk. > >Jeff Thanks Xiao Zeng