Re: [PATCH] RISC-V: Add split pattern to generate SFB instructions. [PR113095]

[Jeff Law <jeffreyalaw@gmail.com>]

On 1/19/24 00:09, Kito Cheng wrote:
> Thanks! generally LGTM, but I would wait one more week to see any
> other comments :)Just a note.  113095 isn't marked as a regression, but it most 
definitely is a regression.  So this meets the stage4 criteria.


> 
> On Fri, Jan 19, 2024 at 3:05 PM Monk Chiang <monk.chiang@sifive.com> wrote:
>>
>> Since the match.pd transforms (zero_one == 0) ? y : z <op> y,
>> into ((typeof(y))zero_one * z) <op> y. Add splitters to recongize
>> this expression to generate SFB instructions.
>>
>> gcc/ChangeLog:
>>          PR target/113095
>>          * config/riscv/sfb.md: New splitters to rewrite single bit
>>          sign extension as the condition to SFB instructions.
>>
>> gcc/testsuite/ChangeLog:
>>          * gcc.target/riscv/sfb.c: New test.
I would probably suggest seeing if these still work when the NE nodes do 
not have a mode (ie, replace "ne:X" with just "ne".  Our docs are a bit 
unclear on that topic IIRC and it looks like the RISC-V backend is 
inconsistent.

More importantly, this message doesn't indicate if/how this patch was 
tested.  Given it's conditional on SFB a bug here would be narrow, but 
we should still be doing a regression test.

Jeff