BZ83477 is another case where we had an VR_UNDEFINED range for source operand during jump threading.
In this instance we had a PHI argument that was a constant. During threading we record that the PHI result has the constant as its value. I mistakenly thought that would avoid the need to mess around with the value range unwinding stack. After all if the PHI result has a constant value, we can just use the constant value. But we handle generation of output ranges for a given statement before we do const/copy propagation and simplification. So we used the VR_UNDEFINED range for the condition in a COND_EXPR rhs of a gimple assignment. This resulted in an incorrect output range for the assignment that was later used in other jump threading decisions. The fix is to always push a range for a non-virtual PHI during jump threading. WHen the source operand is an SSA_NAME, we can just copy its range. When the source operand is an INTEGER_CST, we can create a range from that. Otherwise we use VR_VARYING. I'd hoped this would fix the 435.gromacs problem, but it doesn't appear to do so :( Bootstrapped and regression tested on x86_64. Installing on the trunk. Jeff
commit d69c59491a47db627c2429abef7547e7f005fc40 Author: Jeff Law <l...@torsion.usersys.redhat.com> Date: Tue Dec 19 15:07:12 2017 -0500 PR tree-optimization/83477 * tree-ssa-threadedge.c (record_temporary_equivalences_from_phis): For a non-virtual PHI, always push a new range. PR tree-optimization/83477 * gcc.c-torture/execute/pr83477.c: New test. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 4581f41d8fb..8a80f18045b 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,9 @@ +2017-12-18 Jeff Law <l...@redhat.com> + + PR tree-optimization/83477 + * tree-ssa-threadedge.c (record_temporary_equivalences_from_phis): For + a non-virtual PHI, always push a new range. + 2017-12-19 Martin Sebor <mse...@redhat.com> PR middle-end/77608 @@ -195,7 +201,6 @@ * tree-ssa-dom.c (record_equivalences_from_phis): Fix handling of degenerates resulting from ignoring an edge. ->>>>>>> .r255835 2017-12-18 Martin Sebor <mse...@redhat.com> PR middle-end/83373 diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 63d671374e3..b32cb7a09fa 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2017-12-18 Jeff Law <l...@redhat.com> + + PR tree-optimization/83477 + * gcc.c-torture/execute/pr83477.c: New test. + 2017-12-19 Martin Sebor <mse...@redhat.com> PR middle-end/77608 @@ -63,7 +68,6 @@ PR ipa/83346 * g++.dg/ipa/pr82801.C: New test. ->>>>>>> .r255835 2017-12-18 Martin Sebor <mse...@redhat.com> PR middle-end/83373 diff --git a/gcc/testsuite/gcc.c-torture/execute/pr83477.c b/gcc/testsuite/gcc.c-torture/execute/pr83477.c new file mode 100644 index 00000000000..de667fd7d68 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr83477.c @@ -0,0 +1,22 @@ +int yf = 0; + +void +pl (int q5, int nd) +{ + unsigned int hp = q5; + int zx = (q5 == 0) ? hp : (hp / q5); + + yf = ((nd < 2) * zx != 0) ? nd : 0; +} + +int +main (void) +{ + pl (1, !yf); + if (yf != 1) + __builtin_abort (); + + return 0; +} + + diff --git a/gcc/tree-ssa-threadedge.c b/gcc/tree-ssa-threadedge.c index 1fafd7b5932..0c782f51697 100644 --- a/gcc/tree-ssa-threadedge.c +++ b/gcc/tree-ssa-threadedge.c @@ -156,11 +156,37 @@ record_temporary_equivalences_from_phis (edge e, const_and_copies->record_const_or_copy (dst, src); /* Also update the value range associated with DST, using - the range from SRC. */ - if (evrp_range_analyzer && TREE_CODE (src) == SSA_NAME) + the range from SRC. + + Note that even if SRC is a constant we need to set a suitable + output range so that VR_UNDEFINED ranges do not leak through. */ + if (evrp_range_analyzer) { - value_range *vr = evrp_range_analyzer->get_value_range (src); - evrp_range_analyzer->push_value_range (dst, vr); + /* Get an empty new VR we can pass to update_value_range and save + away in the VR stack. */ + vr_values *vr_values = evrp_range_analyzer->get_vr_values (); + value_range *new_vr = vr_values->allocate_value_range (); + memset (new_vr, 0, sizeof (value_range)); + + /* There are three cases to consider: + + First if SRC is an SSA_NAME, then we can copy the value + range from SRC into NEW_VR. + + Second if SRC is an INTEGER_CST, then we can just wet + NEW_VR to a singleton range. + + Otherwise set NEW_VR to varying. This may be overly + conservative. */ + if (TREE_CODE (src) == SSA_NAME) + copy_value_range (new_vr, vr_values->get_value_range (src)); + else if (TREE_CODE (src) == INTEGER_CST) + set_value_range_to_value (new_vr, src, NULL); + else + set_value_range_to_varying (new_vr); + + /* This is a temporary range for DST, so push it. */ + evrp_range_analyzer->push_value_range (dst, new_vr); } } return true;