On 2023-11-20 14:46 Jeff Law <jeffreya...@gmail.com> wrote: > > > >On 10/30/23 21:35, Fei Gao wrote: > >>> So just a few notes to further illustrate why I'm currently looking to >>> take the VRULL+Ventana implementation. The code above would be much >>> better handled by just calling noce_emit_cmove. noce_emit_cmove will go >>> through the conditional move expander. So any improvement we make in >>> the expander "just work" when called from the if-converter. >> noce_emit_czero is used here to make sure czero insns are emited. >> noce_emit_cmove includes SFB and Thead movcc, which will take precedence >> over zicond in RISCV if enabled. Unfortunately we have products with SFB and >> Zicond >> both available and saw such conflict. >> And that is also the reason to add hook TARGET_HAVE_COND_ZERO >> in [PATCH 1/4] to disallow ineffient code emited by SFB enable and Zicond >> disabled case. >I understand what you're trying to do, but I would consider the >TARGET_HAVE_COND_ZERO fundamentally the wrong approach. Hi Jeff
Thanks for your review. I just post the new series. https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg327148.html https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg327151.html https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg327149.html https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg327150.html TARGET_HAVE_COND_ZERO has been deleted. > >I'm willing to defer routing everything through noce_emit_cmove for now, >but that's really where this code needs to be going. If that's causing >a conflict for a particular implementation with both SFB and Zicond, >then we'll have to look at the details and adjust things in the target >files. Agree. We can try noce_emit_cmove later with more TCs integrated recently. Also I tried to solve the conflict found in my TCs in [PATCH 1/4] and [PATCH 4/4]. > > >> Cool and waiting for your submit. Shifts/rotates can be added in >> noce_try_cond_zero_arith. >Fully agreed. Those are easy. Shifts/rotates have been added. BR, Fei > >> I tried to keep noce_try_cond_zero_arith simple without introducing SCC and >> other stuff >> as addtional insns will be generated for greater than like comparision >> but may not be generated for branch-insn based SFB. >And I think the result is we're going to fail to implement many >profitable if-conversions. > > >Jeff
