Re: [PATCH] Enhance PHI processing in VN

[Richard Sandiford]

David Edelsohn  writes:
> On Wed, Sep 27, 2017 at 6:58 PM, Richard Sandiford
>  wrote:
>> David Edelsohn  writes:
>>> On Fri, Sep 15, 2017 at 2:53 AM, Richard Biener  wrote:
 On Thu, 14 Sep 2017, David Edelsohn wrote:

> * tree-ssa-sccvn.c (visit_phi): Merge undefined values similar
> to VN_TOP.
>
> This seems to have regressed
>
> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
> "Read tp_first_run: 0" 2
> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
> "Read tp_first_run: 2" 1
> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
> "Read tp_first_run: 3" 1

 Hmm, I don't see these FAILs.  Looking at the testcase there are
 no undefined uses so I wonder how the patch could have any effect.

 Can you re-check and open a bugreport?
>>>
>>> It disappeared again.  A different failure appeared and disappeared a
>>> few weeks ago.  Something in the testsuite infrastructure appears to
>>> not be stable, at least on AIX.  Sorry for the incorrect report.
>>
>> Perhaps this is unrelated, but when doing the "has this patch
>> changed assembly on these targets?" testing, I noticed that AIX
>> had differences like:
>>
>> --- old/powerpc-ibm-aix7.0/test/-O3/g++.dg/init/constant1.s
>> +++ new/powerpc-ibm-aix7.0/test/-O3/g++.dg/init/constant1.s
>> @@ -4,21 +4,21 @@
>> .csect ..text.startup[PR],2
>> .align 2
>> .align 4
>> - .globl 
>> _GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401
>> - .globl 
>> ._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401
>> - .csect 
>> _GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401[DS]
>> -_GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401:
>> - .long 
>> ._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401,
>>  TOC[tc0], 0
>> + .globl 
>> _GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9
>> + .globl 
>> ._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9
>> + .csect 
>> _GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9[DS]
>> +_GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9:
>> + .long 
>> ._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9,
>>  TOC[tc0], 0
>>
>> even though -frandom-seed is forced to the same value for both runs.
>>
>> Is this behaviour deliberate?  I thought runs from a few weeks ago
>> had stable names, but maybe I just misremember.
>
> AIX does not have init/fini sections, so it is built at link time as a
> C file by collect2-ld.  I suspect that collect2-ld doesn't use
> -frandom-seed to build its temporary file.

This is just comparing -S output, so the difference seems to be in
the compiler itself.

Thanks,
Richard

Re: [PATCH] Enhance PHI processing in VN

[David Edelsohn]

On Wed, Sep 27, 2017 at 6:58 PM, Richard Sandiford
 wrote:
> David Edelsohn  writes:
>> On Fri, Sep 15, 2017 at 2:53 AM, Richard Biener  wrote:
>>> On Thu, 14 Sep 2017, David Edelsohn wrote:
>>>
 * tree-ssa-sccvn.c (visit_phi): Merge undefined values similar
 to VN_TOP.

 This seems to have regressed

 FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
 "Read tp_first_run: 0" 2
 FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
 "Read tp_first_run: 2" 1
 FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
 "Read tp_first_run: 3" 1
>>>
>>> Hmm, I don't see these FAILs.  Looking at the testcase there are
>>> no undefined uses so I wonder how the patch could have any effect.
>>>
>>> Can you re-check and open a bugreport?
>>
>> It disappeared again.  A different failure appeared and disappeared a
>> few weeks ago.  Something in the testsuite infrastructure appears to
>> not be stable, at least on AIX.  Sorry for the incorrect report.
>
> Perhaps this is unrelated, but when doing the "has this patch
> changed assembly on these targets?" testing, I noticed that AIX
> had differences like:
>
> --- old/powerpc-ibm-aix7.0/test/-O3/g++.dg/init/constant1.s
> +++ new/powerpc-ibm-aix7.0/test/-O3/g++.dg/init/constant1.s
> @@ -4,21 +4,21 @@
> .csect ..text.startup[PR],2
> .align 2
> .align 4
> -   .globl 
> _GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401
> -   .globl 
> ._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401
> -   .csect 
> _GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401[DS]
> -_GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401:
> -   .long 
> ._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401,
>  TOC[tc0], 0
> +   .globl 
> _GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9
> +   .globl 
> ._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9
> +   .csect 
> _GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9[DS]
> +_GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9:
> +   .long 
> ._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9,
>  TOC[tc0], 0
>
> even though -frandom-seed is forced to the same value for both runs.
>
> Is this behaviour deliberate?  I thought runs from a few weeks ago
> had stable names, but maybe I just misremember.

AIX does not have init/fini sections, so it is built at link time as a
C file by collect2-ld.  I suspect that collect2-ld doesn't use
-frandom-seed to build its temporary file.

- David

Re: [PATCH] Enhance PHI processing in VN

[Richard Sandiford]

David Edelsohn  writes:
> On Fri, Sep 15, 2017 at 2:53 AM, Richard Biener  wrote:
>> On Thu, 14 Sep 2017, David Edelsohn wrote:
>>
>>> * tree-ssa-sccvn.c (visit_phi): Merge undefined values similar
>>> to VN_TOP.
>>>
>>> This seems to have regressed
>>>
>>> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
>>> "Read tp_first_run: 0" 2
>>> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
>>> "Read tp_first_run: 2" 1
>>> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
>>> "Read tp_first_run: 3" 1
>>
>> Hmm, I don't see these FAILs.  Looking at the testcase there are
>> no undefined uses so I wonder how the patch could have any effect.
>>
>> Can you re-check and open a bugreport?
>
> It disappeared again.  A different failure appeared and disappeared a
> few weeks ago.  Something in the testsuite infrastructure appears to
> not be stable, at least on AIX.  Sorry for the incorrect report.

Perhaps this is unrelated, but when doing the "has this patch
changed assembly on these targets?" testing, I noticed that AIX
had differences like:

--- old/powerpc-ibm-aix7.0/test/-O3/g++.dg/init/constant1.s
+++ new/powerpc-ibm-aix7.0/test/-O3/g++.dg/init/constant1.s
@@ -4,21 +4,21 @@
.csect ..text.startup[PR],2
.align 2
.align 4
-   .globl 
_GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401
-   .globl 
._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401
-   .csect 
_GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401[DS]
-_GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401:
-   .long 
._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0xbb0d20e181e3a401,
 TOC[tc0], 0
+   .globl 
_GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9
+   .globl 
._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9
+   .csect 
_GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9[DS]
+_GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9:
+   .long 
._GLOBAL__I_65535_0_.._.._.._testsuite_g__.dg_._init_constant1.C__0x5610f7ec143966c9,
 TOC[tc0], 0

even though -frandom-seed is forced to the same value for both runs.

Is this behaviour deliberate?  I thought runs from a few weeks ago
had stable names, but maybe I just misremember.

Thanks,
Richard

Re: [PATCH] Enhance PHI processing in VN

[David Edelsohn]

On Fri, Sep 15, 2017 at 2:53 AM, Richard Biener  wrote:
> On Thu, 14 Sep 2017, David Edelsohn wrote:
>
>> * tree-ssa-sccvn.c (visit_phi): Merge undefined values similar
>> to VN_TOP.
>>
>> This seems to have regressed
>>
>> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
>> "Read tp_first_run: 0" 2
>> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
>> "Read tp_first_run: 2" 1
>> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
>> "Read tp_first_run: 3" 1
>
> Hmm, I don't see these FAILs.  Looking at the testcase there are
> no undefined uses so I wonder how the patch could have any effect.
>
> Can you re-check and open a bugreport?

It disappeared again.  A different failure appeared and disappeared a
few weeks ago.  Something in the testsuite infrastructure appears to
not be stable, at least on AIX.  Sorry for the incorrect report.

- David

Re: [PATCH] Enhance PHI processing in VN

[Richard Biener]

On Thu, 14 Sep 2017, David Edelsohn wrote:

> * tree-ssa-sccvn.c (visit_phi): Merge undefined values similar
> to VN_TOP.
> 
> This seems to have regressed
> 
> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
> "Read tp_first_run: 0" 2
> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
> "Read tp_first_run: 2" 1
> FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
> "Read tp_first_run: 3" 1

Hmm, I don't see these FAILs.  Looking at the testcase there are
no undefined uses so I wonder how the patch could have any effect.

Can you re-check and open a bugreport?

Thanks,
Richard.

Re: [PATCH] Enhance PHI processing in VN

[David Edelsohn]

* tree-ssa-sccvn.c (visit_phi): Merge undefined values similar
to VN_TOP.

This seems to have regressed

FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
"Read tp_first_run: 0" 2
FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
"Read tp_first_run: 2" 1
FAIL: gcc.dg/tree-prof/time-profiler-2.c scan-ipa-dump-times profile
"Read tp_first_run: 3" 1

- David

Re: [PATCH] Enhance PHI processing in VN

[Richard Biener]

On Thu, 7 Sep 2017, Richard Biener wrote:

> On Thu, 7 Sep 2017, Richard Biener wrote:
> 
> > 
> > This enhances VN to do the same PHI handling as CCP, meeting
> > undefined and constant to constant.  I've gone a little bit
> > further (and maybe will revisit this again) in also meeting
> > all-undefined to undefined taking one of the undefined args
> > as the value number.  I feel like this might run into
> > the equation issues I mentioned in the other mail so it
> > would be cleaner to invent a "new" undefined value number
> > here -- but I have to make sure to not create too many
> > default-defs or break iteration convergence (default defs are also
> > expensive given they require a decl - sth I want to change as well).
> > 
> > So for now I guess I'll stick with the slightly bogus(?) way also
> > hoping for a testcase that shows the issue this uncovers.
> > 
> > Note it's required to handle
> > 
> > _3 = PHI <_1(D), _2(D)>
> > ..
> > 
> > _4 = PHI <_3, 1>
> > 
> > consistently with
> > 
> > _4 = PHI <_1(D), _2(D), 1>
> > 
> > aka with/without extra forwarders.
> 
> That said, "fallout" is we simplify
> 
> int foo (int b)
> { 
>   int i, j, k;
>   if (b)
> k = i;
>   else
> k = j;
>   if (k == i)
> return 1;
>   else if (k == j)
> return 2;
>   return 0;
> 
> to
> 
>   if (j == i)
> return 1;
>   else
> return 2;
> 
> or even just
> 
>   return 2;
> 
> dependent on PHI argument order of k = PHI .
> 
> Likewise we'd say that either k - i or k - j is zero.
> 
> The complication with PHIs is that they do not always only appear
> in places where uses of the args dominate it but the other way
> around so we can't really invoke the undefined behavior rule
> on a PHI node with undefined args itself.  The question is whether
> we may for PHIs with just undefined args ... but my guess is no
> so I do have to fix the above.
> 
> Anybody can produce a testcase that we'd consider wrong-code?
> (the above examples clearly are not)

After some pondering I decided for PHIs with all undefs there's
really no way things can go wrong.

Thus the following is what I installed.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Richard.

2017-09-14  Richard Biener  

* tree-ssa-sccvn.c (visit_phi): Merge undefined values similar
to VN_TOP.

* gcc.dg/tree-ssa/ssa-fre-59.c: New testcase.
* gcc.dg/uninit-suppress_2.c: Adjust.
* gcc.dg/tree-ssa/ssa-sccvn-2.c: Likewise.

Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 252062)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -3860,11 +3860,11 @@ visit_reference_op_store (tree lhs, tree
 static bool
 visit_phi (gimple *phi)
 {
-  bool changed = false;
-  tree result;
-  tree sameval = VN_TOP;
-  bool allsame = true;
+  tree result, sameval = VN_TOP, seen_undef = NULL_TREE;
   unsigned n_executable = 0;
+  bool allsame = true;
+  edge_iterator ei;
+  edge e;
 
   /* TODO: We could check for this in init_sccvn, and replace this
  with a gcc_assert.  */
@@ -3873,8 +3873,6 @@ visit_phi (gimple *phi)
 
   /* See if all non-TOP arguments have the same value.  TOP is
  equivalent to everything, so we can ignore it.  */
-  edge_iterator ei;
-  edge e;
   FOR_EACH_EDGE (e, ei, gimple_bb (phi)->preds)
 if (e->flags & EDGE_EXECUTABLE)
   {
@@ -3884,8 +3882,12 @@ visit_phi (gimple *phi)
if (TREE_CODE (def) == SSA_NAME)
  def = SSA_VAL (def);
if (def == VN_TOP)
- continue;
-   if (sameval == VN_TOP)
+ ;
+   /* Ignore undefined defs for sameval but record one.  */
+   else if (TREE_CODE (def) == SSA_NAME
+&& ssa_undefined_value_p (def, false))
+ seen_undef = def;
+   else if (sameval == VN_TOP)
  sameval = def;
else if (!expressions_equal_p (def, sameval))
  {
@@ -3893,30 +3895,39 @@ visit_phi (gimple *phi)
break;
  }
   }
-  
-  /* If none of the edges was executable or all incoming values are
- undefined keep the value-number at VN_TOP.  If only a single edge
- is exectuable use its value.  */
-  if (sameval == VN_TOP
-  || n_executable == 1)
-return set_ssa_val_to (PHI_RESULT (phi), sameval);
 
+
+  /* If none of the edges was executable keep the value-number at VN_TOP,
+ if only a single edge is exectuable use its value.  */
+  if (n_executable <= 1)
+result = seen_undef ? seen_undef : sameval;
+  /* If we saw only undefined values create a new undef SSA name to
+ avoid false equivalences.  */
+  else if (sameval == VN_TOP)
+{
+  gcc_assert (seen_undef);
+  result = seen_undef;
+}
   /* First see if it is equivalent to a phi node in this block.  We prefer
  this as it allows IV elimination - see PRs 66502 and 67167.  */
-  result = vn_phi_lookup (phi);
-  if (result)
-changed = set_ssa_val_to (PHI_RESULT 

Re: [PATCH] Enhance PHI processing in VN

[Richard Biener]

On Thu, 7 Sep 2017, Richard Biener wrote:

> 
> This enhances VN to do the same PHI handling as CCP, meeting
> undefined and constant to constant.  I've gone a little bit
> further (and maybe will revisit this again) in also meeting
> all-undefined to undefined taking one of the undefined args
> as the value number.  I feel like this might run into
> the equation issues I mentioned in the other mail so it
> would be cleaner to invent a "new" undefined value number
> here -- but I have to make sure to not create too many
> default-defs or break iteration convergence (default defs are also
> expensive given they require a decl - sth I want to change as well).
> 
> So for now I guess I'll stick with the slightly bogus(?) way also
> hoping for a testcase that shows the issue this uncovers.
> 
> Note it's required to handle
> 
> _3 = PHI <_1(D), _2(D)>
> ..
> 
> _4 = PHI <_3, 1>
> 
> consistently with
> 
> _4 = PHI <_1(D), _2(D), 1>
> 
> aka with/without extra forwarders.

That said, "fallout" is we simplify

int foo (int b)
{ 
  int i, j, k;
  if (b)
k = i;
  else
k = j;
  if (k == i)
return 1;
  else if (k == j)
return 2;
  return 0;

to

  if (j == i)
return 1;
  else
return 2;

or even just

  return 2;

dependent on PHI argument order of k = PHI .

Likewise we'd say that either k - i or k - j is zero.

The complication with PHIs is that they do not always only appear
in places where uses of the args dominate it but the other way
around so we can't really invoke the undefined behavior rule
on a PHI node with undefined args itself.  The question is whether
we may for PHIs with just undefined args ... but my guess is no
so I do have to fix the above.

Anybody can produce a testcase that we'd consider wrong-code?
(the above examples clearly are not)

Thanks,
Richard.

> Similar to CCP/copyprop this doesn't allow copies to be propagated
> this way as this removes most gcc.dg/Wuninitialized* warnings
> we test for...
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> Richard.
> 
> 2017-09-07  Richard Biener  
> 
>   * tree-ssa-sccnv.c (visit_phi): Handle meeting undefined
>   values.
> 
>   * gcc.dg/tree-ssa/ssa-fre-59.c: New testcase.
> 
> Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-59.c
> ===
> --- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-59.c(nonexistent)
> +++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-59.c(working copy)
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O -fdump-tree-fre1" } */
> +
> +int i;
> +int foo (int b)
> +{
> +  int j;
> +  i = 1;
> +  if (b)
> +j = i;
> +  return i - j;
> +}
> +
> +/* { dg-final { scan-tree-dump "return 0;" "fre1" } } */
> Index: gcc/tree-ssa-sccvn.c
> ===
> --- gcc/tree-ssa-sccvn.c  (revision 251833)
> +++ gcc/tree-ssa-sccvn.c  (working copy)
> @@ -3864,6 +3864,7 @@ visit_phi (gimple *phi)
>tree result;
>tree sameval = VN_TOP;
>bool allsame = true;
> +  tree seen_undef = NULL_TREE;
>unsigned n_executable = 0;
>  
>/* TODO: We could check for this in init_sccvn, and replace this
> @@ -3884,8 +3885,12 @@ visit_phi (gimple *phi)
>   if (TREE_CODE (def) == SSA_NAME)
> def = SSA_VAL (def);
>   if (def == VN_TOP)
> -   continue;
> - if (sameval == VN_TOP)
> +   ;
> + /* Ignore undefined defs here.  */
> + else if (TREE_CODE (def) == SSA_NAME
> +  && ssa_undefined_value_p (def, false))
> +   seen_undef = def;
> + else if (sameval == VN_TOP)
> sameval = def;
>   else if (!expressions_equal_p (def, sameval))
> {
> @@ -3893,7 +3898,18 @@ visit_phi (gimple *phi)
>   break;
> }
>}
> -  
> +  /* If we found undefined values and didn't end up with a constant
> + drop back to varying as otherwise we end up removing most late
> + uninitialized use warnings.  CCP/copyprop have the same restriction.  */
> +  if (allsame && seen_undef)
> +{
> +  /* Unless this was the only value we've seen.  */
> +  if (sameval == VN_TOP)
> + sameval = seen_undef;
> +  else if (! is_gimple_min_invariant (sameval))
> + allsame = false;
> +}
> +
>/* If none of the edges was executable or all incoming values are
>   undefined keep the value-number at VN_TOP.  If only a single edge
>   is exectuable use its value.  */
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

[PATCH] Enhance PHI processing in VN

[Richard Biener]

This enhances VN to do the same PHI handling as CCP, meeting
undefined and constant to constant.  I've gone a little bit
further (and maybe will revisit this again) in also meeting
all-undefined to undefined taking one of the undefined args
as the value number.  I feel like this might run into
the equation issues I mentioned in the other mail so it
would be cleaner to invent a "new" undefined value number
here -- but I have to make sure to not create too many
default-defs or break iteration convergence (default defs are also
expensive given they require a decl - sth I want to change as well).

So for now I guess I'll stick with the slightly bogus(?) way also
hoping for a testcase that shows the issue this uncovers.

Note it's required to handle

_3 = PHI <_1(D), _2(D)>
..

_4 = PHI <_3, 1>

consistently with

_4 = PHI <_1(D), _2(D), 1>

aka with/without extra forwarders.

Similar to CCP/copyprop this doesn't allow copies to be propagated
this way as this removes most gcc.dg/Wuninitialized* warnings
we test for...

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

2017-09-07  Richard Biener  

* tree-ssa-sccnv.c (visit_phi): Handle meeting undefined
values.

* gcc.dg/tree-ssa/ssa-fre-59.c: New testcase.

Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-59.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-59.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-59.c  (working copy)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-fre1" } */
+
+int i;
+int foo (int b)
+{
+  int j;
+  i = 1;
+  if (b)
+j = i;
+  return i - j;
+}
+
+/* { dg-final { scan-tree-dump "return 0;" "fre1" } } */
Index: gcc/tree-ssa-sccvn.c
===
--- gcc/tree-ssa-sccvn.c(revision 251833)
+++ gcc/tree-ssa-sccvn.c(working copy)
@@ -3864,6 +3864,7 @@ visit_phi (gimple *phi)
   tree result;
   tree sameval = VN_TOP;
   bool allsame = true;
+  tree seen_undef = NULL_TREE;
   unsigned n_executable = 0;
 
   /* TODO: We could check for this in init_sccvn, and replace this
@@ -3884,8 +3885,12 @@ visit_phi (gimple *phi)
if (TREE_CODE (def) == SSA_NAME)
  def = SSA_VAL (def);
if (def == VN_TOP)
- continue;
-   if (sameval == VN_TOP)
+ ;
+   /* Ignore undefined defs here.  */
+   else if (TREE_CODE (def) == SSA_NAME
+&& ssa_undefined_value_p (def, false))
+ seen_undef = def;
+   else if (sameval == VN_TOP)
  sameval = def;
else if (!expressions_equal_p (def, sameval))
  {
@@ -3893,7 +3898,18 @@ visit_phi (gimple *phi)
break;
  }
   }
-  
+  /* If we found undefined values and didn't end up with a constant
+ drop back to varying as otherwise we end up removing most late
+ uninitialized use warnings.  CCP/copyprop have the same restriction.  */
+  if (allsame && seen_undef)
+{
+  /* Unless this was the only value we've seen.  */
+  if (sameval == VN_TOP)
+   sameval = seen_undef;
+  else if (! is_gimple_min_invariant (sameval))
+   allsame = false;
+}
+
   /* If none of the edges was executable or all incoming values are
  undefined keep the value-number at VN_TOP.  If only a single edge
  is exectuable use its value.  */