Re: [tree-ssa] New regressions as of 2003-11-04

[law at redhat dot com]

In message <1067956749.23315.3.camel@frodo.toronto.redhat.com>, Diego Novillo w
rites:
 >
 >Lots of changes yesterday produced new regressions in C, C++, Fortran
 >and mudflap.  I think I know what the mudflap problem is, so I'll take
 >care of that.  Could you folks take a look at the new regressions and
 >see if they're related to your changes?
 >	 FAIL: gcc.dg/tree-ssa/20030814-4.c scan-tree-dump-times set = -1 0
 >	 FAIL: gcc.dg/tree-ssa/20030814-5.c scan-tree-dump-times set = -1 0
OK.  After looking at these some more.  These are failures that are a 
combination of the COND_EXPR lowering and some sillyness in PRE.

Daniel -- did you actually bootstrap and regression test your PRE
changes before installing them?



What happens is before PRE we have this:

 # block 2.  pred: 0.  succ: 4 3.
 [ ... ]
 if (set_2 != -1)
    {
      goto <U4570>;
    }
  else
    {
      goto <U4658>;
    };

  # block 3.  pred: 2.  succ: 4.
  <U4570>:;;
  (void)0;
  goto <U4658>;;
  (void)0;
  (void)0;

  # block 4.  pred: 2 3.  succ: -2.
  #   set_1 = PHI <set_2(2), 0(3)>;
  <U4658>:;;
  return set_1;;

After PRE we have:

  # block 2.  pred: 0.  succ: 5 3.
  [ ... ]
   if (set_2 != -1)
    {
      goto <U4570>;
    }
  else
    {
      goto <U89f8>;
    };

  # block 5.  pred: 2.  succ: 4.
  <U89f8>:;;
  critedgetmp.4_13 = 0;
  goto <U4658>;;

  # block 3.  pred: 2.  succ: 4.
  <U4570>:;;
  (void)0;
  goto <U4658>;;
  (void)0;
  (void)0;

  # block 4.  pred: 5 3.  succ: -2.
  #   set_1 = PHI <set_2(5), 0(3)>;
  <U4658>:;;
  return set_1;;
}



It appears PRE decided to split the edge between block 2 and block 4.

That in turn allows the final dominator optimizer to propagate things
a little further and after removing dead code we have:

  # block 2.  pred: 0.  succ: 4 3.
  <U44fc>:;;
  if (set_2 != -1)
    {
      goto <U4658>;
    }
  else
    {
      goto <U89f8>;
    };

  # block 4.  pred: 2.  succ: 3.
  <U89f8>:;;
  (void)0;
  goto <U4658>;;
  (void)0;

  # block 3.  pred: 2 4.  succ: -2.
  #   set_1 = PHI <-1(4), 0(2)>;
  <U4658>:;;
  return set_1;;
}


The key being that we propagated the value -1 for set2 in the PHI node.
This is actually what we want, so PRE really isn't doing anything 
particularly wrong, though it is exposing a problem with the COND_EXPR
changes.

Anyway, when we convert that out of SSA we get:

  # block 2.  pred: 0.  succ: 4 5.
  [ ... ]
   if (set != -1)
    {
      goto <U932c>;
    }
  else
    {
      goto <U89f8>;
    };
  
  # block 5.  pred: 2.  succ: 3.
  <U932c>:;;
  set = 0;
  goto <U4658>;;
  
  # block 4.  pred: 2.  succ: 3.
  <U89f8>:;;
  (void)0;
  set = -1;
  goto <U4658>;;
  (void)0;
  
    # block 3.  pred: 5 4.  succ: -2.
  <U4658>:;;
  return set;;


The key point being that the statement "set = -1" is totally unnecessary
since we know set has the value -1 at that point in the program.

Before the COND_EXPR lowering that would have looked like

  if (set != -1)
    set = 0;
  else
    set = -1;


Which we know how to optimize in remove_useless_stmts_and_vars_cond into

  if (set != -1)
    set = 0;



All of this may seem relatively trivial, but it is something that we need
to solve since it represents a rather trivial case where we're leaving in
useless code which cse1's path following code cleans up -- remember one of
our goals is to make cse1's path following code disappear completely.


Not splitting critical edges will hide this problem, so I don't consider
that a real solution.

I'm not really hot on the idea of expanding the remove_useless_stmts
code to detect this, though I doubt it would be terribly difficult,
except for the fact that the CFG isn't necessarily accurate during
remove_useless_stmts_and_vars.


Another option would be for the out-of-ssa pass to handle it.  I can see
how it's a partitioning problem, but I don't immediately see how to 
detect and optimizing this case in the out-of-ssa pass.

Anyway, given that this is regression caused by the interaction between
Zdenek's lowering code and Daniel's edge splitting code, one of them 
really ought to be responsible for fixing it.

jeff