Re: [PATCH] ccp for V_MUST_DEF operands

[Diego Novillo <dnovillo at redhat dot com>]

On Wed, 2004-07-14 at 11:07, Brian Booth wrote:

> * tree-ssa-ccp.c (latticevalue): Add VIRTUAL.
> (substitute_and_fold): Propagate into VUSE operands when 
> possible.
> (visit_phi_node): Handle VIRTUAL lattice value.
> (cp_lattice_meet): Handle merging of lattice values when 
> VIRTUAL is present.
> (visit_stmt): Visit assignments with V_MUST_DEFs.
> (visit_assignment): Gather ccp information for V_MUST_DEF 
> operands.
> (ccp_fold): Deal with RHS' that are constant and virtual.
> (evaluate_stmt): Handle VIRTUAL likely values.
> (dump_lattice_value): Dump VIRTUAL lattice values.
> (initialize): Mark statements with V_MUST_DEFs as VARYING 
> only if the V_MUST_DEF operand is VARYING. Fix comment and 
> include VOPS when computing immediate uses.
> (set_lattice_value): Disallow a VIRTUAL->UNDEFINED state 
> transition.
> (replace_vuse_in): New function.
> (likely_value): Remove superfluous call to get_stmt_operands and add
> check of vuse operands.
> (get_default_value): Set the default value of virtually 
> defined variables to VIRTUAL instead of VARYING.
>
Thanks.  Only a few comments.

The new lattice value could use a better name.  Perhaps,
UNDEFINED_GLOBAL.  We talked about UNKNONW_CST, but the semantics of
this lattice value are closer to UNDEFINED than CONSTANT.

Could you add a test case?  Something like this should suffice:

--------------------------------------------------------------------
int G;

foo (int i)
{
   if (i > 0)
     G = 3;
   else
     G = 3;
   if (G != 3)
     link_error ();
}

main ()
{
   foo (0);
   return 0;
}
--------------------------------------------------------------------


--------------------------------------------------------------------
@@ -643,8 +668,9 @@ visit_stmt (tree stmt)
   /* Now examine the statement.  If the statement is an assignment that
      produces a single output value, evaluate its RHS to see if the
lattice
      value of its output has changed.  */
+  v_may_defs = V_MAY_DEF_OPS (ann);
   if (TREE_CODE (stmt) == MODIFY_EXPR
-      && TREE_CODE (TREE_OPERAND (stmt, 0)) == SSA_NAME)
+      && NUM_V_MAY_DEFS (v_may_defs) == 0)
     visit_assignment (stmt);
--------------------------------------------------------------------

Better test for (NUM_V_MUST_DEFS == 1 || TREE_CODE == SSA_NAME). 
Otherwise, we'll visit assignments that are not interesting if we run
CCP before aliases are computed.

 

--------------------------------------------------------------------
@@ -695,14 +716,45 @@ static void
 visit_assignment (tree stmt)
 {
   value val;
-  tree lhs, rhs;
+  tree lhs, rhs, lhs_ssa_name = NULL_TREE;
+  vuse_optype vuses;
 
   lhs = TREE_OPERAND (stmt, 0);
   rhs = TREE_OPERAND (stmt, 1);
+  vuses = STMT_VUSE_OPS (stmt);
+
+  /* We require the SSA version number of the lhs for the value_vector.
+     Make sure we have it.  */
+  if (TREE_CODE (lhs) == SSA_NAME)
+    {
+      lhs_ssa_name = lhs;
+      lhs = SSA_NAME_VAR (lhs);
+    }
+  else if (NUM_V_MUST_DEFS (STMT_V_MUST_DEF_OPS (stmt)) == 0)
+    /* Ignore variables with hidden uses and volatile variables. 
+       They do not have a SSA version number and are not of interest. 
+       Also avoid any partial definitions of these variables. */;
---------------------------------------------------------------------

With the change above, you won't need this one here.


----------------------------------------------------------------------
+#if defined ENABLE_CHECKING
+  if (NUM_V_MAY_DEFS (STMT_V_MAY_DEF_OPS (stmt)) > 0
+      || NUM_V_MUST_DEFS (v_must_defs) != 1)
+    abort ();
+#endif
----------------------------------------------------------------------

This check  can go at the start of visit_assignment.


---------------------------------------------------------------------
+  else if (DECL_P (rhs) 
+           && NUM_VUSES (vuses) == 1
+	   && operand_equal_p (rhs, SSA_NAME_VAR (VUSE_OP (vuses, 0)), 0))
---------------------------------------------------------------------

No need to use operand_equal_p.  Pointer equality is enough for DECLs.


---------------------------------------------------------------------
@@ -1470,8 +1600,6 @@ likely_value (tree stmt)
   if (get_call_expr_in (stmt) != NULL_TREE)
     return VARYING;
 
-  get_stmt_operands (stmt);
-
   uses = USE_OPS (ann);
   for (i = 0; i < NUM_USES (uses); i++)
     {
---------------------------------------------------------------------

Odd.  Why?


---------------------------------------------------------------------
+    
+  vuses = VUSE_OPS (ann);
+  
+#ifdef ENABLE_CHECKING
+  /* There should be no alias loads by this point. */
+  if (NUM_VUSES (vuses) > 1)
+    abort ();
+#endif  
+
---------------------------------------------------------------------

No.  You want to check ann->makes_aliased_loads here.


This is OK to commit with those changes.



Diego.