Hi!
The function comment says:
*XOR_P is to be FALSE if EXP might be a XOR used in a compare, in which
case, if XOR_CMP_OP is a zero constant, it will be overridden with *PEXP,
*XOR_P will be set to TRUE, and the left-hand operand of the XOR will be
decoded. If *XOR_P is TRUE, XOR_CMP_OP is supposed to be NULL, and then the
right-hand operand of the XOR will be decoded.
and the comment right above the xor_p handling says
/* Turn (a ^ b) [!]= 0 into a [!]= b. */
but I don't see anything that would actually check that the other operand is
0, in the testcase below it happily optimizes (a ^ 1) == 8 into a == 1.
The following patch adds that check.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Note, there are various other parts of the function I'm worried about, but
haven't had time to construct counterexamples yet.
One worrying thing is the
/* Drop casts, only save the outermost type. We need not worry about
narrowing then widening casts, or vice-versa, for those that are not
essential for the compare have already been optimized out at this
point. */
comment, while obviously there are various optimizations which do optimize
nested casts and the like, I'm not really sure it is safe to rely on them
happening always before this optimization, there are various options to
disable certain optimizations and some IL could appear right before
ifcombine without being optimized yet the way this routine expects.
Plus, the 3 casts are looked through in between various optimizations which
might make those narrowing/widening or vice versa cases necessary.
Also, e.g. for the xor optimization, I think there is a difference between
int a and
(a ^ 0x23) == 0
and
((int) (((unsigned char) a) ^ (unsigned char) 0x23)) == 0
etc.
Another thing I'm worrying about are mixing up the different patterns
together, there is the BIT_AND_EXPR handling, BIT_XOR_EXPR handling,
RSHIFT_EXPR handling and then load handling.
What if all 4 appear together, or 3 of them, 2 of them?
Is the xor optimization still valid if there is BIT_AND_EXPR in between?
I.e. instead of
(a ^ 123) == 0
there is
((a ^ 123) & 234) == 0
?
2024-12-18 Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/118081
* gimple-fold.cc (decode_field_reference): Only set *xor_p to true
if *xor_cmp_op is integer_zerop.
* gcc.dg/pr118081.c: New test.
--- gcc/gimple-fold.cc.jj 2024-12-14 11:28:08.000000000 +0100
+++ gcc/gimple-fold.cc 2024-12-17 17:27:31.294874825 +0100
@@ -7572,7 +7572,7 @@ decode_field_reference (tree *pexp, HOST
/* Not much we can do when xor appears in the right-hand compare
operand. */
return NULL_TREE;
- else
+ else if (integer_zerop (*xor_cmp_op))
{
*xor_p = true;
exp = res_ops[0];
--- gcc/testsuite/gcc.dg/pr118081.c.jj 2024-12-17 17:31:07.522860128 +0100
+++ gcc/testsuite/gcc.dg/pr118081.c 2024-12-17 17:29:59.131813646 +0100
@@ -0,0 +1,28 @@
+/* PR tree-optimization/118081 */
+/* { dg-do run } */
+/* { dg-options "-O2 -fno-tree-vrp -fno-expensive-optimizations" } */
+
+int a, b;
+
+int
+foo (int f)
+{
+ return f ? f || 0 : f;
+}
+
+void
+bar (void)
+{
+ b = a ? a : 1;
+ int i = foo (1 ^ b);
+ signed char h = i - 8;
+ if (h)
+ return;
+ __builtin_abort ();
+}
+
+int
+main ()
+{
+ bar ();
+}
Jakub
