Re: [PATCH 2/5] [RISC-V] Generate Zicond instruction for basic semantics

[Richard Sandiford <richard.sandiford@arm.com>]

Jeff Law via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On 7/19/23 04:11, Xiao Zeng wrote:
>> This patch completes the recognition of the basic semantics
>> defined in the spec, namely:
>> 
>> Conditional zero, if condition is equal to zero
>>    rd = (rs2 == 0) ? 0 : rs1
>> Conditional zero, if condition is non zero
>>    rd = (rs2 != 0) ? 0 : rs1
>> 
>> gcc/ChangeLog:
>> 
>> 	* config/riscv/riscv.md: Include zicond.md
>> 	* config/riscv/zicond.md: New file.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 	* gcc.target/riscv/zicond-primitiveSemantics.c: New test.
> So I played with this a bit today.  I originally thought that using 
> match_dup was the right way to go for those 4 secondary patterns.  But 
> after further pondering it's not ideal.
>
> match_dup will require pointer equality within the RTL structure.  That 
> could inhibit detection in two cases.  First, SUBREGs.   SUBREGs are not 
> shared.  So we'd never match if we had a SUBREG expression.
>
> Second, post register allocation we can have the same looking RTX, but 
> it may not be pointer equal.

Where were you seeing the requirement for pointer equality?  genrecog.cc
at least uses rtx_equal_p, and I think it has to.  E.g. some patterns
use (match_dup ...) to match output and input mems, and mem rtxes
shouldn't be shared.

I'd always understood using matching constraints against other inputs
to be a no-no, since the RA doesn't (and can't reasonably be expected to)
make two non-identical inputs match.  So AIUI, using "1" won't lead to
different code generation compared to "r".  Both are relying on the RA
happening to do the right thing.  But "1" would presumably trigger an
ICE if something goes wrong.

Thanks,
Richard

> The SUBREG issue also means that we don't want to use a REGNO (x) == 
> REGNO (y) style check because those macros are only valid on REG 
> expressions.  We could strip the SUBREG, but that's usually awkward to 
> do in a pattern's condition.
>
> The net result is we probably should use rtx_equal_p which I was hoping 
> to avoid.  I'm testing with that change to the 4 secondary patterns 
> right now.  Assuming that passes (and I have no reason to think it 
> won't) then I'll go ahead and commit #1 and #2 from this series which is 
> all I have time for today.
>
>
>
> Jeff