Re: [tree-ssa] PATCH to gimplify_boolean_expr

[law at redhat dot com]

In message <wvlk7908j9s.fsf@prospero.boston.redhat.com>, Jason Merrill writes:
 >On Tue, 26 Aug 2003 02:40:45 -0600, law@redhat.com wrote:
 >
 >> In message <wvlbrud9xsk.fsf@prospero.boston.redhat.com>, Jason Merrill writ
 >es:
 >>  >On Mon, 25 Aug 2003 15:18:08 -0600, law@redhat.com wrote:
 >>  >> In message <wvloeydbny0.fsf@prospero.boston.redhat.com>, Jason Merrill 
 >writes:
 >>  >>  >
 >>  >>  >While testing my patch to always evaluate complex expressions into fo
 >rmal
 >>  >>  >temps, I ran into a new failure on gcc.dg/tree-ssa/20030728-1.c becau
 >se the
 >>  >>  >generated gimple code changed from
 >>  >>  >
 >>  >>  >  T1 = (foo == bar);
 >>  >>  >  if (T1)
 >>  >>  >    ...
 >>  >>  >
 >>  >>  >to
 >>  >>  >
 >>  >>  >  T1 = (foo == bar);   // formal temp
 >>  >>  >  T2 = T1;             // variable temp
 >>  >>  >  if (T2)
 >>  >>  >    ...
 >
 >> So I went back and removed your change so that I could see this supposed
 >> behavior -- in particular I wanted to see why we didn't propagate the value
 >> zero for F.4_10 in the PHI node.
 >>
 >> Unfortunately, after reverting your patch, I get precisely the code I would
 >> expect -- the value of zero is propagated into the PHI node and instead
 >> of a copy we get an initialization to zero for the temporary when we
 >> complete the out-of-ssa pass.
 >>
 >> I'm not suggesting you remove your patch -- but that you instead look at
 >> what other local changes you might have which would inhibit constant
 >> propagation.
 >
 >Yes, as I mentioned at the beginning of my mail, I only see this problem
 >with my patch to evaluate complex expressions into formal temps, because of
 >the change in the gimplifier output described above.  I can send you that
 >patch, if you like.  I still need to figure out why it breaks Java before I
 >can check it in.
And my point was that I still don't see why that change should obscure the
fact that the value is known to be zero in the ELSE arm and that should
have been propagated into the PHI node.

>From your dump:

 >.dom:
 >
 >  F.1_6 = t1_4->common.code;
 >  F.2_7 = (unsigned char)F.1_6;
 >  F.3_8 = F.2_7 == 0;
 >  F.4_10 = t2_9 != 0B;
 >  T.5_11 = F.4_10;
 >  if (F.4_10)
 >    {
 >      # BLOCK 1 (/home/jason/src/ast/gcc/gcc/testsuite/gcc.dg/tree-ssa/200307
 >28-1.c:36).  PRED: 0.  SUCC: 3.
 >      F.6_12 = t2_9->common.code;
 >      F.7_13 = (unsigned char)F.6_12;
 >      F.8_14 = F.7_13 == 0;
 >      T.5_15 = F.8_14;
 >    }
 >  else
 >    {
 >      # BLOCK 2.  PRED: 0.  SUCC: 3.
 >      (void)0
 >    };
 >  
 >  # BLOCK 3 (/home/jason/src/ast/gcc/gcc/testsuite/gcc.dg/tree-ssa/20030728-1
 >.c:36).  PRED: 2 1.  SUCC: 5 4.
 >  #   T.5_1 = PHI <F.8_14(1), F.4_10(2)>;
 >  F.9_16 = F.3_8 ^ T.5_1;

When we enter the else clause we should record that F.4_10 has the value
zero.  That equivalence should be propagated to into the PHI, replacing
F.4_10 in the PHI node with the value zero.

This should happen regardless of whether or not we're forcing the
condition to be evaluated into a formal temporary.  Again, the only thing
I can immediately think of that would prevent that would be a difference
in types -- ie, in one context we want/have a boolean in the other
context we want/have a regular integer node.


Unfortunately, I'm probably not going to have time to do anything with the
patch if you send it my way right now -- I'm rather buried in some non-gcc
activities at the moment.

Remember, the only propagation we're talking about is propagating zero
for F.4_10(2) in the PHI node.  We can't do anything interesting for
F.8_14 in the PHI node.

jeff