[PR67828] don't unswitch loops on undefined SSA values (was: Re: [PR64164] drop copyrename, integrate into expand)

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

On Fri, Oct 9, 2015 at 7:26 AM, Alexandre Oliva <aoliva@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 <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