[Bug middle-end/86554] [7/8/9 Regression] Incorrect code generation with signed/unsigned comparison
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86554 --- Comment #9 from Richard Biener --- Author: rguenth Date: Tue Jan 8 13:05:47 2019 New Revision: 267725 URL: https://gcc.gnu.org/viewcvs?rev=267725=gcc=rev Log: 2019-01-08 Richard Biener PR tree-optimization/86554 * tree-ssa-sccvn.c (eliminate_dom_walker, rpo_elim, rpo_avail): Move earlier. (visit_nary_op): When value-numbering to expressions with different overflow behavior make sure there's an available expression on the path. * gcc.dg/torture/pr86554-1.c: New testcase. * gcc.dg/torture/pr86554-2.c: Likewise. Added: trunk/gcc/testsuite/gcc.dg/torture/pr86554-1.c trunk/gcc/testsuite/gcc.dg/torture/pr86554-2.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-sccvn.c
[Bug middle-end/86554] [7/8/9 Regression] Incorrect code generation with signed/unsigned comparison
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86554 Richard Biener changed: What|Removed |Added Target Milestone|7.4 |7.5
[Bug middle-end/86554] [7/8/9 Regression] Incorrect code generation with signed/unsigned comparison
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86554 Richard Biener changed: What|Removed |Added CC||vincent-gcc at vinc17 dot net --- Comment #8 from Richard Biener --- *** Bug 87276 has been marked as a duplicate of this bug. ***
[Bug middle-end/86554] [7/8/9 Regression] Incorrect code generation with signed/unsigned comparison
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86554
--- Comment #7 from Richard Biener ---
So on trunk the remaining offender now is code hoisting. We still do
value-numbering the same but correctly do _not_ use the values definition to
simplify
the comparison:
Value numbering stmt = ret_13 = PHI
Setting value number of ret_13 to ret_12 (changed)
Making available beyond BB5 ret_13 for value ret_12
Value numbering stmt = printf ("ret is %d\n", ret_13);
Setting value number of .MEM_6 to .MEM_6 (changed)
Value numbering stmt = if (ret_13 <= 0)
Recording on edge 5->6 ret_13 le_expr 0 == true
Recording on edge 5->6 ret_13 gt_expr 0 == false
marking outgoing edge 5 -> 6 executable
marking outgoing edge 5 -> 7 executable
for code-hoisting both computations are antic-in in their respective
definition blocks and thus the value is hoisted choosing the computation
based on the expressions available in that block (the non-conversion
one since the unconverted unsigned expression isn't ANTIC_IN).
Note that a possible mitigation is to dumb down VN, not making this kind
of conversions value-number to the same value (but possibly only conversions
from signed to unsigned). OTOH the value-numbering itself isn't the issue
but rather how we compute ANTIC. That is, this looks like an inherent issue
of GVN-PRE.
We can, of course fix up inserts to use unsigned arithmetic but that has
impact on code quality in the majority of cases where no such false
equivalency is detected.
Need to think about this some more.
[Bug middle-end/86554] [7/8/9 Regression] Incorrect code generation with signed/unsigned comparison
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86554 --- Comment #6 from Bill Schmidt --- Anton has been able to work around the problem with a source change (this code is unnecessarily baroque anyway), so I don't think anybody is urgently awaiting a fix. If this will be fixed in your eventual rewrite of FRE, I think that's more than sufficient. Thanks!
[Bug middle-end/86554] [7/8/9 Regression] Incorrect code generation with signed/unsigned comparison
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86554 Richard Biener changed: What|Removed |Added Priority|P3 |P2 Target Milestone|--- |7.4
[Bug middle-end/86554] [7/8/9 Regression] Incorrect code generation with signed/unsigned comparison
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86554
Richard Biener changed:
What|Removed |Added
Known to work||6.4.0
Summary|Incorrect code generation |[7/8/9 Regression]
|with signed/unsigned|Incorrect code generation
|comparison |with signed/unsigned
||comparison
Known to fail|7.0.1 |7.1.0, 8.1.0, 9.0
--- Comment #5 from Richard Biener ---
This regressed with moving fold-const.c patterns to match.pd, specifically
/* Non-equality compare simplifications from fold_binary */
(for cmp (lt gt le ge)
/* Comparisons with the highest or lowest possible integer of
the specified precision will have known values. */
(simplify
(cmp (convert?@2 @0) INTEGER_CST@1)
...
(if (cmp == LE_EXPR)
{ constant_boolean_node (true, type); })
and
/* Transform comparisons of the form X +- C1 CMP C2 to X CMP C2 -+ C1. */
(for cmp (lt le gt ge)
(for op (plus minus)
rop (minus plus)
(simplify
(cmp (op@3 @0 INTEGER_CST@1) INTEGER_CST@2)
...
(if (single_use (@3))
(with
{
fold_overflow_warning (("assuming signed overflow does not occur "
"when changing X +- C1 cmp C2 to "
"X cmp C2 -+ C1"),
WARN_STRICT_OVERFLOW_COMPARISON);
}
(cmp @0 { res; })
The latter only appeared with GCC7.
The issue itself is latent at least since match-and-simplify but possibly
earlier as well where SCCVN used GENERIC expression simplification for
"combining" stmt defs.
It will take some time to fix this (if a fix is possible with reasonable
amount of work), do not hold your breath.
