[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513 --- Comment #12 from CVS Commits --- The master branch has been updated by Richard Biener : https://gcc.gnu.org/g:a05cc70a6c1ae0e5b22e16f4d8d13995a38ea1f9 commit r11-6499-ga05cc70a6c1ae0e5b22e16f4d8d13995a38ea1f9 Author: Richard Biener Date: Wed Jan 6 09:26:55 2021 +0100 tree-optimization/98513 - fix bug in range intersection code This fixes a premature optimization in the range intersection code which assumes earlier branches have to be taken, not taking into account that for symbolic ranges we cannot always compare endpoints. The fix is to instantiate the compare deemed redundant (which then fails as undecidable for the testcase). 2021-01-06 Richard Biener PR tree-optimization/98513 * value-range.cc (intersect_ranges): Compare the upper bounds for the expected relation. * gcc.dg/tree-ssa/pr98513.c: New testcase.
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513
--- Comment #11 from Richard Biener ---
So the issue is we cannot decide between
[ (] ) and [ ( ) ]
and the check for [ (] ) elides the "redundant" check for the upper
bound relation. But the check isn't redundant in case the compare
cannot be decided.
So the simplest fix to the legacy code is to instantiate those
not redundant checks which then results in the "expected"
Intersecting
int [-INF, minus_1_3(D) + 2] EQUIVALENCES: { x_6(D) } (1 elements)
and
int ~[-2147483647, -2147483646] EQUIVALENCES: { x_6(D) } (1 elements)
to
int [-INF, minus_1_3(D) + 2] EQUIVALENCES: { x_6(D) } (1 elements)
(if we can't do anything fancy, intersection simply chooses the first
range as result)
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513
--- Comment #10 from Richard Biener ---
/* { dg-do run } */
__attribute__((noipa))
void __GIMPLE (ssa,startwith("evrp"))
foo (int x, int minus_1)
{
int tem;
unsigned int _1;
unsigned int _2;
__BB(2):
tem_4 = minus_1_3(D);
tem_5 = tem_4 + 2;
_1 = (unsigned int) x_6(D);
_2 = _1 + 2147483647u;
if (_2 > 1u)
goto __BB3;
else
goto __BB6;
__BB(3):
if (x_6(D) <= tem_5)
goto __BB4;
else
goto __BB6;
__BB(4):
if (x_6(D) > 5)
goto __BB5;
else
goto __BB6;
__BB(5):
__builtin_exit (0);
__BB(6):
return;
}
int
main()
{
foo (10, 100);
__builtin_abort ();
}
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513
--- Comment #9 from Richard Biener ---
void __attribute__((noipa))
foo (int x, int minus_1)
{
int tem = minus_1;
tem = tem + 2;
if ((unsigned)x + 2147483647 >= 2)
{
if (x <= tem)
{
if (x > 5)
__builtin_exit (0);
}
}
}
int
main()
{
foo (10, 100);
__builtin_abort ();
}
fails with -O2 -fdisable-tree-ccp1 -fdisable-tree-forwprop1 -fdisable-tree-fre1
I'll turn it into a GIMPLE FE unit testcase for EVRP tomorrow. Interestingly
the intersect with [-INF, minus_1_29(D) + 1] works fine (x < tem vs. x <= tem).
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513 Richard Biener changed: What|Removed |Added Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org Status|NEW |ASSIGNED --- Comment #8 from Richard Biener --- OK, since I wrote that code let me poke into it a bit.
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513
--- Comment #7 from Andrew Macleod ---
This is in the legacy intersection code
we have
[-INF, minus_1_29(D) + 2] intersect [ -INF + 1, -INF + 2]
it falls into this block:
else if ((operand_less_p (vr1min, *vr0max) == 1
|| operand_equal_p (vr1min, *vr0max, 0))
&& operand_less_p (*vr0min, vr1min) == 1)
{
/* [ ( ] ) or [ ]( ) */
if (*vr0type == VR_ANTI_RANGE
&& vr1type == VR_ANTI_RANGE)
*vr0max = vr1max;
else if (*vr0type == VR_RANGE
&& vr1type == VR_RANGE)
*vr0min = vr1min;
else if (*vr0type == VR_RANGE
&& vr1type == VR_ANTI_RANGE)
{
if (TREE_CODE (vr1min) == INTEGER_CST)
--> *vr0max = int_const_binop (MINUS_EXPR, vr1min,
build_int_cst (TREE_TYPE (vr1min), 1));
else
*vr0max = vr1min;
}
(gdb) p operand_less_p (vr1min, *vr0max)
$19 = 1
(gdb) p operand_less_p (*vr0min, vr1min)
$21 = 1
and ends up setting vr0max to (vr1min - 1), which is -INF
and so returns [-INF, -INF]
It seems like it *should* have entered an earlier hunk here maybe?
else if ((maxeq || operand_less_p (vr1max, *vr0max) == 1)
&& (mineq || operand_less_p (*vr0min, vr1min) == 1))
{
/* [ ( ) ] or [( ) ] or [ ( )] */
this looks like the [ ( ) ] case? if I interpret this correctly
it fails to enter this block because:
(gdb) p operand_less_p (vr1max, *vr0max)
$22 = -2
which is operand_less_p (-INF + 2, minus_1_29(D) + 2)
so it claims they cannot be compared at compile time, and thus doesn't drop
into this block.
Im not sure what should be done here... The easiest thing to do is simply punt
when we get a -2 back anywhere... and leave vr0 as it is. thats conservative
and safe. Im not even sure how best to add those checks in where needed.
Otherwise we'll have to delve into why we got a -2, and eventually maybe
substitute +INF for vr0max... but really, I think you'd have to do that sort
of check for each of the operand_less_p() calls to be correct, and figure out
when you want to substitute a +INF or -INF and recalculate the expression.
Although maybe you have a more concise idea of how to handle this. Perhaps its
more localized than it appears to me at first glance.
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513
Richard Biener changed:
What|Removed |Added
Blocks||85316
CC||aldyh at gcc dot gnu.org
Priority|P3 |P2
--- Comment #6 from Richard Biener ---
Intersecting
int [-INF, minus_1_29(D) + 2] EQUIVALENCES: { i_3_14 } (1 elements)
and
int ~[-2147483647, -2147483646]
to
int [-INF, -INF] EQUIVALENCES: { i_3_14 } (1 elements)
looks wrong.
Referenced Bugs:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85316
[Bug 85316] [meta-bug] VRP range propagation missed cases
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513
--- Comment #5 from Martin Liška ---
This one is fishy:
Intersecting
int [-INF, minus_1_29(D) + 2] EQUIVALENCES: { i_3_14 } (1 elements)
and
int ~[-2147483647, -2147483646]
to
int [-INF, -INF] EQUIVALENCES: { i_3_14 } (1 elements)
how can we end up with [-INF, -INF]?
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513 Martin Liška changed: What|Removed |Added Assignee|marxin at gcc dot gnu.org |unassigned at gcc dot gnu.org Status|ASSIGNED|NEW
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513
--- Comment #4 from Martin Liška ---
One last version:
$ cat combined.cc
unsigned var;
unsigned array[2];
int zero = 0, minus_2 = -2;
const int &max(const int &a, const int &b) { return a > b ? a : b; }
void test(int minus_1)
{
for (unsigned i_0 = 0; i_0 < 2; i_0++)
{
for (int i_1 = 0; i_1 < zero; i_1++)
for (int i_2 = 0; i_2 < 2; i_2++)
var = max(minus_1, 0);
for (int i_3 = minus_2 + 2; i_3 < minus_1 + 3; i_3++)
array[i_3] = zero;
}
}
int main() { test(-1); }
Apparently, the unswitching is not responsible for the problem, it triggers one
more loop invariant motion
and end with:
175.lim4:
[local count: 29489562]:
_26 = (unsigned int) pretmp_1;
array[_8] = _26;
if (_3 >= i_3_14)
goto ; [66.67%]
else
goto ; [33.33%]
[local count: 19660691]:
array[i_3_14] = _26;
i_3_21 = i_3_14 + 1;
goto ; [100.00%]
while without the option we have:
[local count: 29489562]:
_26 = (unsigned int) pretmp_1;
array[_8] = _26;
i_3_11 = _8 + 1;
if (_3 >= i_3_11)
goto ; [66.67%]
else
goto ; [33.33%]
[local count: 19660691]:
array[i_3_11] = _26;
i_3_21 = i_3_11 + 1;
Anyway I think the problem happens in 189.dom3 where we replace:
Optimizing statement array[i_3_14] = _26;
Replaced 'i_3_14' with constant '-2147483648'
which is bogus (the replace is about the second iteration of the loop i_3).
Can please somebody familiar with VRP explain the transformation:
Visiting controlling predicate if (_3 >= i_3_14)
Adding assert for _3 from _3 >= i_3_14
Adding assert for i_3_14 from i_3_14 <= _3
Intersecting
int [i_3_14, +INF] EQUIVALENCES: { _3 } (1 elements)
and
int [minus_1_29(D) + 2, minus_1_29(D) + 2]
to
int [i_3_14, +INF] EQUIVALENCES: { _3 } (1 elements)
Intersecting
int [-INF, minus_1_29(D) + 2] EQUIVALENCES: { i_3_14 } (1 elements)
and
int ~[-2147483647, -2147483646]
to
int [-INF, -INF] EQUIVALENCES: { i_3_14 } (1 elements)
Intersecting
int [minus_1_29(D) + 2, minus_1_29(D) + 2]
and
int [i_3_14, +INF]
to
int [minus_1_29(D) + 2, minus_1_29(D) + 2]
Intersecting
int ~[-2147483647, -2147483646]
and
int [-INF, -INF]
to
int [-INF, -INF]
pushing new range for i_3_14: int [-INF, -INF] EQUIVALENCES: { i_3_14 } (1
elements)
1>>> STMT 1 = _3 ge_expr i_3_14
1>>> STMT 0 = _3 lt_expr i_3_14
Optimizing statement array[i_3_14] = _26;
Replaced 'i_3_14' with constant '-2147483648'
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513 --- Comment #3 from Martin Liška --- Happens with -O2 -funswitch-loops.
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513
--- Comment #2 from Martin Liška ---
Even more reduced test-case:
$ cat combined.cc
unsigned var;
unsigned array[2];
int zero = 0, minus_2 = -2;
const int &max(const int &a, const int &b) { return a > b ? a : b; }
void test(int minus_1)
{
for (unsigned i_0 = 0; i_0 < 2; i_0++)
{
for (int i_3 = 0; i_3 < zero; i_3++)
for (int i_4 = 0; i_4 < 2; i_4++)
var = max(minus_1, 0);
for (int i_6 = minus_2 + 2; i_6 < minus_1 + 3; i_6++)
array[i_6] = zero;
}
}
int main() { test(-1); }
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513
Martin Liška changed:
What|Removed |Added
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |marxin at gcc dot
gnu.org
--- Comment #1 from Martin Liška ---
A cleaner test-case:
$ cat combined.cc
extern unsigned long long var_20;
extern unsigned short arr_8[][26][1][1][11];
const int &max(int &a, const int &b) { return a > b ? a : b; }
int test___trans_tmp_1, var_5 = -1, var_6 = -2;
void test(int var_5, int var_6,
signed char arr_1[1][1][1]) {
for (unsigned i_0 = 0; i_0 < 21; i_0 += 2)
for (int i_2 = 0; i_2 < 8; i_2 += 82) {
for (int i_3 = 0; i_3 < test___trans_tmp_1; i_3++)
for (short i_4 = 0; i_4 < 20; i_4 += 4)
var_20 = max(var_5, 0);
for (int i_5 = 0; i_5 < 19;
i_5 += 20)
for (int i_6 = var_6 + 2; i_6 < var_5 + 3; i_6++)
arr_8[3][2][i_2][i_5][i_6] = arr_1[0][0][0];
}
}
unsigned long long var_20;
signed char arr_1[1][1][1];
unsigned short arr_8[22][26][1][1][11];
int main() { test(var_5, var_6, arr_1); }
Optimized dump contains:
[local count: 17523394]:
_93 = MEM[(signed char[26][19] *)arr_1_31(D) + 1482B][2][0];
_94 = (short unsigned int) _93;
arr_8[3][2][0][0][-2147483648] = _94; < HERE
if (i_6_103 > _131)
goto ; [11.00%]
else
goto ; [89.00%]
which is instruction that causes the segfault. I'm going to take a look.
[Bug tree-optimization/98513] [10/11 Regression] Wrong code with -O3 since r10-2804-gbf05a3bbb58b3558
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98513 Martin Liška changed: What|Removed |Added Known to work||9.3.0 Last reconfirmed||2021-01-04 Known to fail||10.2.0, 11.0 Target Milestone|--- |10.3 Ever confirmed|0 |1 Status|UNCONFIRMED |NEW
