Re: [PATCH] RISC-V: zicond: remove bogus opt2 pattern

[Vineet Gupta <vineetg@rivosinc.com>]

On 9/1/23 10:40, Palmer Dabbelt wrote:
>
> Just working through this in email, as there's a lot of 
> double-negatives and I managed to screw up my Linux PR this morning so 
> I may not be thinking that well...
>
> The docs say "(if_then_else test true-value false-value)".  So in this 
> case it's
>
>    test:  (ne (match_operand:X 1 "register_operand" "r") (const_int 0))
>    true:  (match_operand:GPR 2 "register_operand" "r")
>    false: (match_operand:GPR 3 "register_operand" "1") == 
> (match_operand:X 1 "register_operand" "r")
>
> and we're encoding it as
>
>    czero.nez %0,%2,%1
>
> so that's
>
>    rd:  output
>    rs1: on-true
>    rs2: condition (the value inside the ne in RTL)
>
> That looks correct to me: the instruction's condition source register 
> is inside a "(ne ... 0)", but we're doing the cmov.nez so it looks OK.

Yes it is fine, until you end up having both operand 2 and operand 3 
have non-zero values at runtime and somehow match this pattern Then the 
semantics of czero* are not honored.

> It might be easier for everyone to understand if you add a specific 
> testcase for just the broken codegen.  I'm not having luck 
> constructing a small reproducer (though I don't have a clean tree 
> lying around, so I might have screwed something up here).
>
> IIUC something like
>
>    long func(long x, long a) {
>        if (a != 0)
>          return x;
>        return 0;
>    }
>
> should do it, but I'm getting
>
>    func:
>        czero.eqz       a0,a0,a1
>        ret

Unfortunately tests any simpler don't trigger it - they code seqs just 
get optimized away - otherwise Jeff would have found this 3 weeks ago ;-)
Just use gcc/testsuite/gcc.c-torture/execute/pr60003.c

Thx,
-Vineet