Re: [tree-ssa] PATCH to gimplify_boolean_expr

[Jason Merrill <jason at redhat dot com>]

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 formal
>  >temps, I ran into a new failure on gcc.dg/tree-ssa/20030728-1.c because the
>  >generated gimple code changed from
>  >
>  >  T1 = (foo == bar);
>  >  if (T1)
>  >    ...
>  >
>  >to
>  >
>  >  T1 = (foo == bar);   // formal temp
>  >  T2 = T1;             // variable temp
>  >  if (T2)
>  >    ...
>  >
>  >dom provided constant defs of T2 within the branches, but ccp replaced uses
>  >of T2 with T1, so we didn't see them.

> I'm not sure I follow.  Dumps would probably help me understand what you
> mean.

.gimple:

  F.1 = t1->common.code;
  F.2 = (unsigned char)F.1;
  F.3 = F.2 == 0;
  F.4 = t2 != 0B;
  T.5 = F.4;
  if (T.5)
    {
      F.6 = t2->common.code;
      F.7 = (unsigned char)F.6;
      F.8 = F.7 == 0;
      T.5 = F.8;
    }
  else
    {
      (void)0
    };
  F.9 = F.3 ^ T.5;

.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/20030728-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;

In dom, we replaced the conditional use of T.5 with F.4, so we introduce
F.4_10==true and F.4_10==false in the arms rather than do the same to T.5.
When we re-unify T.5 in block 3, the last value we have for T.5 on the
false branch is F.4_10.

If we had not done copy prop here, we would have set up the constant values
for T.5 instead, and the false one would have flowed into the PHI.

Recursing to look up the constant value of F.4_10 in block 2 would also fix
this case.

>  >One fix for this is to move the comparison directly into the condition,
>  >i.e.
>  >
>  >if (foo == bar)
>  >  T1 = true;
>  >  ...
>  >  
>  >which seems to be roughly what the expander does.  This also dramatically
>  >simplifies gimplify_boolean_expr; now we can just hand off to the code in
>  >shortcut_cond_expr.
> As long as "foo" and "bar" are temporaries, I don't mind.  One of the
> problems we've got right now is that we can have nontrivial operands 
> inside COND_EXPR_COND.  That inhibits redundant expression elimination.

We can?  Testcase, please.  The operands are supposed to be vals.  Of
course, currently memory variables can be vals, but I'm working on fixing
that.

>  >Another fix is to teach dom to propagate conditional information back up
>  >the def chain; I'll have a preliminary patch for that shortly.
> Now that would be good.   I've seen some tests where that could eliminate
> some useless crud -- particuliarly where we lose information due to 
> casting between different types.

I've attached what I have below; feel free to run with it if you're
interested, I won't do anything more with it for a while.

>  >Another fix would be to delay copy prop until out-of-ssa time, as Morgan
>  >does.
> Note that Morgan does copy-prop during the dominator walk as well.  It is
> not delayed.

Ah, I missed the mention of that in chapter 2; he doesn't mention it in
chapter 8.

Jason

*** tree-ssa-dom.c.~1~	2003-08-25 16:50:26.000000000 -0400
--- tree-ssa-dom.c	2003-08-25 18:05:55.000000000 -0400
*************** get_eq_expr_value (tree if_stmt, int tru
*** 1322,1327 ****
--- 1322,1344 ----
  
    cond = COND_EXPR_COND (if_stmt);
  
+   /* If the condition is a plain variable, it was probably initialized from
+      a comparison.  Look back through the def chain to find it.  */
+   if (TREE_CODE (cond) == SSA_NAME)
+     {
+       tree init = cond;
+       do
+ 	{
+ 	  init = SSA_NAME_DEF_STMT (init);
+ 	  if (TREE_CODE (init) == MODIFY_EXPR)
+ 	    init = TREE_OPERAND (init, 1);
+ 	}
+       while (TREE_CODE (init) == SSA_NAME);
+ 
+       if (TREE_CODE_CLASS (TREE_CODE (init)) == '<')
+ 	cond = init;
+     }
+ 
    /* If we have a comparison expression, then record its result into
       the available expression table.  */
    if (TREE_CODE_CLASS (TREE_CODE (cond)) == '<')