From 549af7288332d698d45bbbcf3c61aaeb193fb716 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Wed, 18 Dec 2024 15:21:40 +0100 Subject: [PATCH] gimple-fold: Fix up decode_field_reference xor handling [PR118081] 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. 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 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 | 2 +- gcc/testsuite/gcc.dg/pr118081.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gcc.dg/pr118081.c diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc index 6c11654a2c6..b90ad8f0900 100644 --- a/gcc/gimple-fold.cc +++ b/gcc/gimple-fold.cc @@ -7572,7 +7572,7 @@ decode_field_reference (tree *pexp, HOST_WIDE_INT *pbitsize, /* 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]; diff --git a/gcc/testsuite/gcc.dg/pr118081.c b/gcc/testsuite/gcc.dg/pr118081.c new file mode 100644 index 00000000000..7659df7e785 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr118081.c @@ -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 (); +}