Re: Re: [PATCH 2/4] [ifcvt] if convert x=c ? y+z : y by RISC-V Zicond like insns

["Fei Gao" <gaofei@eswincomputing.com>]

On 2023-11-20 14:46  Jeff Law <jeffreyalaw@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