This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PR67828] don't unswitch loops on undefined SSA values (was: Re: [PR64164] drop copyrename, integrate into expand)
- From: Alexandre Oliva <aoliva at redhat dot com>
- To: Zhendong Su <su at cs dot ucdavis dot edu>, Richard Biener <richard dot guenther at gmail dot com>
- Cc: Alan Lawrence <alan dot lawrence at arm dot com>, Jeff Law <law at redhat dot com>, James Greenhalgh <James dot Greenhalgh at arm dot com>, "H.J. Lu" <hjl dot tools at gmail dot com>, Segher Boessenkool <segher at kernel dot crashing dot org>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Christophe Lyon <christophe dot lyon at linaro dot org>, David Edelsohn <dje dot gcc at gmail dot com>, Eric Botcazou <ebotcazou at adacore dot com>
- Date: Fri, 09 Oct 2015 02:26:07 -0300
- Subject: [PR67828] don't unswitch loops on undefined SSA values (was: Re: [PR64164] drop copyrename, integrate into expand)
- Authentication-results: sourceware.org; auth=none
- References: <orio9cw10j dot fsf at livre dot home> <CAFiYyc3k9xh_6RGbTdHYOJ-aEmUWVeFRoWy=YzCHFDNSdtTXCA at mail dot gmail dot com> <orwpxqvqnp dot fsf at livre dot home> <20150723203112 dot GB27818 at gate dot crashing dot org> <CAMe9rOpR+2gPxo0tKaRPtcML_Q4=r-_=9iqk+_JZFPkM=eN=BQ at mail dot gmail dot com> <CAMe9rOpbLEyDexVJqJAFJ3W6o4AktNog-jwk2CY4GZkrmT+nfA at mail dot gmail dot com> <or4mkmhgc9 dot fsf at livre dot home> <CAMe9rOp=S5fu1N=i7waswCYqJeLBCrySqYdFYkVa7LV04vpQSg at mail dot gmail dot com> <CAMe9rOrq+ZBAg1nZ1twEcPqwBj4j9+XA+SXQJVWWzjfdvidjtw at mail dot gmail dot com> <or1tfkdjhj dot fsf at livre dot home> <20150810082355 dot GA31149 at arm dot com> <55C8BFC3 dot 3030603 at redhat dot com> <CAKQMxzRzMrGtf921vqXCno5uoBN+uzsnJ5wX2Twmvhp1ziAEcA at mail dot gmail dot com> <or37zlpujd dot fsf at livre dot home> <55E72D4C dot 40705 at arm dot com> <orfv2wxygm dot fsf at livre dot home> <55FC3171 dot 7040509 at arm dot com> <ord1x8nblu dot fsf at livre dot home> <CAFiYyc1x2124-YgLmP_Yt+mBgyv_2Yp=O7G4WX9dbYfEs6z=NQ at mail dot gmail dot com>
This patch fixes a latent bug in loop unswitching exposed by the PR64164
changes.
We would move a test out of a loop that might never have been executed,
and that accessed an uninitialized variable. The uninitialized SSA
name, due to uncprop, now gets coalescesd with other SSA names,
expanding the ill effects of the undefined behavior we introduce: in
spite of the zero initialization introduced in later rtl stages for the
uninitialized pseudo, by then we've already expanded a PHI node that
referenced the unitialized variable in the path coming from a path in
which it would necessarily be zero, to a copy from the coalesced pseudo,
that gets modified between the zero-initialization and the copy, so the
copied zero is no longer zero. Oops.
We might want to be stricter in coalesce conflict detection to avoid
this sort of problem, and perhaps to avoid undefined values in uncprop,
but this would all be attempting to limit the effects of undefined
behavior, which is probably a waste of effort. As long as we avoid
introducing undefined behavior ourselves, we shouldn't have to do any of
that. So, this patch fixes loop unswitching so as to not introduce
undefined behavior.
Regstrapped on x86_64-linux-gnu and i686-linux-gnu. Ok to install?
[PR67828] don't unswitch on default defs of non-parms
From: Alexandre Oliva <aoliva@redhat.com>
for gcc/ChangeLog
PR rtl-optimizatoin/67828
* tree-ssa-loop-unswitch.c: Include tree-ssa.h.
(tree_may_unswitch_on): Don't unswitch on expressions
involving undefined values.
for gcc/testsuite/ChangeLog
PR rtl-optimization/67828
* gcc.dg/torture/pr67828.c: New.
---
gcc/testsuite/gcc.dg/torture/pr67828.c | 43 ++++++++++++++++++++++++++++++++
gcc/tree-ssa-loop-unswitch.c | 5 ++++
2 files changed, 48 insertions(+)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr67828.c
diff --git a/gcc/testsuite/gcc.dg/torture/pr67828.c b/gcc/testsuite/gcc.dg/torture/pr67828.c
new file mode 100644
index 0000000..c7b6965
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr67828.c
@@ -0,0 +1,43 @@
+/* Check that we don't misoptimize the final value of d. We used to
+ apply loop unswitching on if(j), introducing undefined behavior
+ that the original code wouldn't exercise, and this undefined
+ behavior would get later passes to misoptimize the loop. */
+
+/* { dg-do run } */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+int x;
+
+int __attribute__ ((noinline, noclone))
+xprintf (int d) {
+ if (d)
+ {
+ if (x)
+ printf ("%d", d);
+ abort ();
+ }
+}
+
+int a, b;
+short c;
+
+int
+main ()
+{
+ int j, d = 1;
+ for (; c >= 0; c++)
+ {
+ a = d;
+ d = 0;
+ if (b)
+ {
+ xprintf (0);
+ if (j)
+ xprintf (0);
+ }
+ }
+ xprintf (d);
+ exit (0);
+}
diff --git a/gcc/tree-ssa-loop-unswitch.c b/gcc/tree-ssa-loop-unswitch.c
index 4328d6a..d6faa37 100644
--- a/gcc/tree-ssa-loop-unswitch.c
+++ b/gcc/tree-ssa-loop-unswitch.c
@@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. If not see
#include "internal-fn.h"
#include "gimplify.h"
#include "tree-cfg.h"
+#include "tree-ssa.h"
#include "tree-ssa-loop-niter.h"
#include "tree-ssa-loop.h"
#include "tree-into-ssa.h"
@@ -139,6 +140,10 @@ tree_may_unswitch_on (basic_block bb, struct loop *loop)
/* Condition must be invariant. */
FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE)
{
+ /* Unswitching on undefined values would introduce undefined
+ behavior that the original program might never exercise. */
+ if (ssa_undefined_value_p (use, true))
+ return NULL_TREE;
def = SSA_NAME_DEF_STMT (use);
def_bb = gimple_bb (def);
if (def_bb
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer