On Fri, Oct 9, 2015 at 7:26 AM, Alexandre Oliva <aol...@redhat.com> wrote:
> 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?
Ok.
Thanks,
Richard.
>
> [PR67828] don't unswitch on default defs of non-parms
>
> From: Alexandre Oliva <aol...@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
