On Thu, 31 Aug 2023 10:57:52 PDT (-0700), Vineet Gupta wrote:
On 8/31/23 06:51, Jeff Law wrote:
On 8/30/23 15:57, Vineet Gupta wrote:
This was tripping up gcc.c-torture/execute/pr60003.c at -O1 since the
pattern semantics can't be expressed by zicond instructions.
This involves test code snippet:
if (a == 0)
return 0;
else
return x;
}
which is equivalent to: "x = (a != 0) ? x : a"
Isn't it
x = (a == 0) ? 0 : x
Which seems like it ought to fit zicond just fine.
Logically they are equivalent, but ....
If we take yours;
x = (a != 0) ? x : a
And simplify with the known value of a on the false arm we get:
x = (a != 0 ) ? x : 0;
Which is equivalent to
x = (a == 0) ? 0 : x;
So ISTM this does fit zicond just fine.
I could very well be mistaken, but define_insn is a pattern match and
opt2 has *ne* so the expression has to be in != form and thus needs to
work with that condition. No ?
and matches define_insn "*czero.nez.<GPR:mode><X:mode>.opt2"
| (insn 41 20 38 3 (set (reg/v:DI 136 [ x ])
| (if_then_else:DI (ne (reg/v:DI 134 [ a ])
| (const_int 0 [0]))
| (reg/v:DI 136 [ x ])
| (reg/v:DI 134 [ a ]))) {*czero.nez.didi.opt2}
The corresponding asm pattern generates
czero.nez x, x, a ; %0, %2, %1
implying
"x = (a != 0) ? 0 : a"
I get this from the RTL pattern:
x = (a != 0) ? x : a
x = (a != 0) ? x : 0
This is the issue, for ne, czero.nez can only express
x = (a != 0) ? 0 : x
I think you got the arms reversed.
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.
The rest of the zero juggling looks sane as well -- I'm not sure if the
X vs GPR mismatch will confuse something else, but it should be caught
by the rtx_equal_p() and thus should at least be safe.
What I meant was czero.nez as specified in RTL pattern would generate x
= (a != 0) ? 0 : a, whereas pattern's desired semantics is (a != 0) ? x : 0
And that is a problem because after all equivalents/simplifications, a
ternary operation's middle operand has to be zero to map to czero*, but
it doesn't for the opt2 RTL semantics.
I've sat on this for 2 days, trying to convince myself I was wrong, but
as it stands, it was generating wrong code in the test which is fixed
after the patch.
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
which looks right to me -- though it's not triggering this pattern, so
not sure that means much.
Thx,
-Vineet
