Bug 95649

I am seeing something slightly different that started with the same revision on powerpc64 as well.  It is preventing several of the spec2006 tests from compiling.

See pr95662.

Here is another testcase pulled from pr95653 which I'm marking as a duplicate of this one.

This one needs -O2 -fno-tree-scev-cprop to reproduce.

char b (void);
char *d;
int e;
int f;
void
g (char *h)
{
  while (d)
    {
      long i = b ();
      if (h + i > d)
	break;
      if (f > 0 || e)
	do
	  *h++ = *h;
	while (--i);
    }
}

Mine.

*** Bug 95653 has been marked as a duplicate of this bug. ***

The problem here is that the code in propagate_into_phi_args() was previously in evrp but is now in the engine itself and affecting all clients.

CCP and copyprop are two of these clients.  The subst & fold engine is changing the CFG (phis in this case) and neither pass is expecting it.  A hack showing that it's the PHI propagation is attached below.

We could make these passes clean-up the CFG for example, but that may be heavy handed if they're supposed to be lightweight ??.  Instead we could either move propagate_into_phi_args back into evrp, or only run it if a CHANGE_CFG_FLAG or somesuch is passed to the engine constructor.

I'll work on a patch.

-------------------------

NOT A FIX:

diff --git a/gcc/tree-ssa-propagate.c b/gcc/tree-ssa-propagate.c
index 4fda296ef9e..249867d8633 100644
--- a/gcc/tree-ssa-propagate.c
+++ b/gcc/tree-ssa-propagate.c
@@ -1228,7 +1228,7 @@ substitute_and_fold_dom_walker::before_dom_children (basic_block bb)
        }
     }

-  substitute_and_fold_engine->propagate_into_phi_args (bb);
+  //substitute_and_fold_engine->propagate_into_phi_args (bb);

   return NULL;
 }

What precisely is causing the problem?  Are we doing something like mucking up the loop closed ssa form?  Or do these passes have internal data that's invalidated by the PHI changes?  Or something else?

Hard to know what to recommend here without  more info.

Hmmm, previous code propagating into PHI args had:

-         if (vr->singleton_p (&val) && may_propagate_copy (arg, val))
-           propagate_value (use_p, val);

We seem to have lost the check for constants, and we're now propagating any old value from get_value_range-- which may be another SSA name:

+         if (val && may_propagate_copy (arg, val))
+           propagate_value (use_p, val);


Testing the following:

diff --git a/gcc/tree-ssa-propagate.c b/gcc/tree-ssa-propagate.c
index 4fda296ef9e..01ee7fd33eb 100644
--- a/gcc/tree-ssa-propagate.c
+++ b/gcc/tree-ssa-propagate.c
@@ -1035,7 +1035,8 @@ substitute_and_fold_engine::propagate_into_phi_args (basic_block bb)
              || virtual_operand_p (arg))
            continue;
          tree val = get_value (arg, phi);
-         if (val && may_propagate_copy (arg, val))
+         if (val && is_gimple_min_invariant (val)
+             && may_propagate_copy (arg, val))
            propagate_value (use_p, val);
        }
     }

I still don't understand why propagating one SSA_NAME for another is causing headaches later though. 

I don't see anything fundamentally wrong with your patch and it restores previous behavior since singleton_p would only be true for a constant, so consider it pre-approved.

The master branch has been updated by Aldy Hernandez <aldyh@gcc.gnu.org>:

https://gcc.gnu.org/g:8fb4d1d58362b77da78c09740c6b5562124a369e

commit r11-1395-g8fb4d1d58362b77da78c09740c6b5562124a369e
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Tue Jun 16 13:43:57 2020 +0200

    Fix pasto in the substitute_and_fold_engine merge with evrp.
    
    The original code only propagated into PHI arguments if the value was
    a constant.  This behavior was lost in the conversion, allowing
    any value (SSAs for instance) to be propagated into PHIs.
    
    gcc/ChangeLog:
    
            PR tree-optimization/95649
            * tree-ssa-propagate.c (propagate_into_phi_args): Do not propagate unless
            value is a constant.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/tree-ssa/pr95649.C: New test.
            * gcc.dg/tree-ssa/pr95649.c: New test.

(In reply to Jeffrey A. Law from comment #8)
> I still don't understand why propagating one SSA_NAME for another is causing
> headaches later though. 

TBH, I don't either.

When I compile the .c test in this PR with the following options:

-O2 -fno-tree-scev-cprop -fdisable-tree-ccp1  -fdisable-tree-ccp2 -fdisable-tree-copyprop1 -fdisable-tree-evrp -fdisable-tree-vrp1 -fdisable-tree-ccp1 -fdisable-tree-ccp2 -fdisable-tree-ccp3

...I still get an ICE, but there is no change in the IL between trunk without the offending patch, and with the patch that broke the test.  This would indicate that there is some on-the-side structure that is being altered by the phi argument propagation.

I did notice that the IL *does* change in the middle of one of the copyprop (or ccp?) passes, but it gets cleaned up to whatever was there before.

Later in SCEV, we ICE while trying to look at the SSA_NAME_DEF_STMT of an SSA which no longer exists in the IL.  This happens in chrec_contains_symbols_defined_in_loop().  A wild guess is that the loop info is getting mucked up somehow.

I can investigate more deeply if desired, but since this was a silly typo, I'm gonna leave it alone for now.

Closing as fixed in mainline.