[1/2] Pattern matching improvements in ipa-prop.c

Hi,

this patch addresses two deficiencies in ipa-prop.c.  Perhaps the more
important one is that the pattern matching code
ipa_analyze_indirect_call_uses which identifies calls to C++
member-pointer parameters required that the SRA-generated loads from
the parameter are in the same BB as the condition determining the
virtual-ness of its value.  That is of course true only if this
branching happens to be in the first basic block.  That is way too
limiting and not necessary so I adjusted the code to look for the BB
with the condition appropriately.

The second deficiency is in code identifying constant member pointer
actual arguments.  The code is very simple, it basically only scans
the BB with the call from the call backwards and looks for
initializations of the structure.  However, currently it bails out
whenever it sees a statement other than a single assignment.  That can
be too restrictive, given that the compiler generated member pointers
happen to be non-addressable.  So I adjusted the code to check for
non-addressability and ignore the non-assignment statements.

I have also slightly modified the testcase that we have for exercising
these code paths to properly test these two improvements.

I have bootstrapped and tested the patch on x86_64-linux without any
issues, OK for trunk?

Thanks,

Martin



2010-06-18  Martin Jambor  <mjambor@suse.cz>

	* ipa-prop.c (determine_cst_member_ptr): Ignore non-assignments
	instead of bailing out on them.  Updated comments.
	(compute_cst_member_ptr_arguments): Check that arg is not
	addressable.
	(ipa_analyze_indirect_call_uses): Do not require that loads from the
	parameter are in the same BB as the condition.  Update comments.

	* testsuite/g++.dg/ipa/iinline-1.C: Adjusted.

On Mon, 21 Jun 2010, Martin Jambor wrote:

> Hi,
> 
> this patch addresses two deficiencies in ipa-prop.c.  Perhaps the more
> important one is that the pattern matching code
> ipa_analyze_indirect_call_uses which identifies calls to C++
> member-pointer parameters required that the SRA-generated loads from
> the parameter are in the same BB as the condition determining the
> virtual-ness of its value.  That is of course true only if this
> branching happens to be in the first basic block.  That is way too
> limiting and not necessary so I adjusted the code to look for the BB
> with the condition appropriately.
> 
> The second deficiency is in code identifying constant member pointer
> actual arguments.  The code is very simple, it basically only scans
> the BB with the call from the call backwards and looks for
> initializations of the structure.  However, currently it bails out
> whenever it sees a statement other than a single assignment.  That can
> be too restrictive, given that the compiler generated member pointers
> happen to be non-addressable.  So I adjusted the code to check for
> non-addressability and ignore the non-assignment statements.
> 
> I have also slightly modified the testcase that we have for exercising
> these code paths to properly test these two improvements.
> 
> I have bootstrapped and tested the patch on x86_64-linux without any
> issues, OK for trunk?

Instead of changing the existing testcase please add a new one.

Also I'm a bit nervous about

>        if (!gimple_assign_single_p (stmt))
> -     return;
> +     continue;

what is 'arg' usually?  Is it a non-SSA name?  Why can't it appear
on the lhs of a call?  Why can't it appear as operand to an
asm output?

Thus, the scanning in determine_cst_member_ptr looks non-conservative
(maybe the followup patch fixes that).  I'd have used

    if (!stmt_may_clobber_ref (stmt, arg))
      continue;

and then

      if (TREE_CODE (lhs) != COMPONENT_REF
          || TREE_OPERAND (lhs, 0) != arg)
        return;

Ok with that changes.

Thanks,
Richard.

> Thanks,
> 
> Martin
> 
> 
> 
> 2010-06-18  Martin Jambor  <mjambor@suse.cz>
> 
> 	* ipa-prop.c (determine_cst_member_ptr): Ignore non-assignments
> 	instead of bailing out on them.  Updated comments.
> 	(compute_cst_member_ptr_arguments): Check that arg is not
> 	addressable.
> 	(ipa_analyze_indirect_call_uses): Do not require that loads from the
> 	parameter are in the same BB as the condition.  Update comments.
> 
> 	* testsuite/g++.dg/ipa/iinline-1.C: Adjusted.
> 
> Index: icln/gcc/ipa-prop.c
> ===================================================================
> --- icln.orig/gcc/ipa-prop.c
> +++ icln/gcc/ipa-prop.c
> @@ -781,10 +781,11 @@ get_ssa_def_if_simple_copy (tree rhs)
>  }
>  
>  /* Traverse statements from CALL backwards, scanning whether the argument ARG
> -   which is a member pointer is filled in with constant values.  If it is, fill
> -   the jump function JFUNC in appropriately.  METHOD_FIELD and DELTA_FIELD are
> -   fields of the record type of the member pointer.  To give an example, we
> -   look for a pattern looking like the following:
> +   which is a member pointer and is not addressable is filled in with constant
> +   values.  If it is, fill the jump function JFUNC in appropriately.
> +   METHOD_FIELD and DELTA_FIELD are fields of the record type of the member
> +   pointer.  To give an example, we look for a pattern looking like the
> +   following:
>  
>       D.2515.__pfn ={v} printStuff;
>       D.2515.__delta ={v} 0;
> @@ -807,7 +808,7 @@ determine_cst_member_ptr (gimple call, t
>        tree lhs, rhs, fld;
>  
>        if (!gimple_assign_single_p (stmt))
> -	return;
> +	continue;
>  
>        lhs = gimple_assign_lhs (stmt);
>        rhs = gimple_assign_rhs1 (stmt);
> @@ -872,6 +873,7 @@ compute_cst_member_ptr_arguments (struct
>        arg = gimple_call_arg (call, num);
>  
>        if (functions[num].type == IPA_JF_UNKNOWN
> +	  && !TREE_ADDRESSABLE (arg)
>  	  && type_like_member_ptr_p (TREE_TYPE (arg), &method_field,
>  				     &delta_field))
>  	determine_cst_member_ptr (call, arg, method_field, delta_field,
> @@ -1030,6 +1032,10 @@ ipa_note_param_call (struct cgraph_node
>       <bb 2>:
>         f$__delta_5 = f.__delta;
>         f$__pfn_24 = f.__pfn;
> +
> +     ...
> +
> +     <bb 5>
>         D.2496_3 = (int) f$__pfn_24;
>         D.2497_4 = D.2496_3 & 1;
>         if (D.2497_4 != 0)
> @@ -1037,7 +1043,7 @@ ipa_note_param_call (struct cgraph_node
>         else
>           goto <bb 4>;
>  
> -     <bb 3>:
> +     <bb 6>:
>         D.2500_7 = (unsigned int) f$__delta_5;
>         D.2501_8 = &S + D.2500_7;
>         D.2502_9 = (int (*__vtbl_ptr_type) (void) * *) D.2501_8;
> @@ -1048,7 +1054,7 @@ ipa_note_param_call (struct cgraph_node
>         D.2507_15 = *D.2506_14;
>         iftmp.11_16 = (String:: *) D.2507_15;
>  
> -     <bb 4>:
> +     <bb 7>:
>         # iftmp.11_1 = PHI <iftmp.11_16(3), f$__pfn_24(2)>
>         D.2500_19 = (unsigned int) f$__delta_5;
>         D.2508_20 = &S + D.2500_19;
> @@ -1109,17 +1115,18 @@ ipa_analyze_indirect_call_uses (struct c
>    d1 = SSA_NAME_DEF_STMT (n1);
>    d2 = SSA_NAME_DEF_STMT (n2);
>  
> +  join = gimple_bb (def);
>    if ((rec = ipa_get_stmt_member_ptr_load_param (d1, false)))
>      {
>        if (ipa_get_stmt_member_ptr_load_param (d2, false))
>  	return;
>  
> -      bb = gimple_bb (d1);
> +      bb = EDGE_PRED (join, 0)->src;
>        virt_bb = gimple_bb (d2);
>      }
>    else if ((rec = ipa_get_stmt_member_ptr_load_param (d2, false)))
>      {
> -      bb = gimple_bb (d2);
> +      bb = EDGE_PRED (join, 1)->src;
>        virt_bb = gimple_bb (d1);
>      }
>    else
> @@ -1128,7 +1135,6 @@ ipa_analyze_indirect_call_uses (struct c
>    /* Second, we need to check that the basic blocks are laid out in the way
>       corresponding to the pattern. */
>  
> -  join = gimple_bb (def);
>    if (!single_pred_p (virt_bb) || !single_succ_p (virt_bb)
>        || single_pred (virt_bb) != bb
>        || single_succ (virt_bb) != join)
> @@ -1138,7 +1144,7 @@ ipa_analyze_indirect_call_uses (struct c
>       significant bit of the pfn. */
>  
>    branch = last_stmt (bb);
> -  if (gimple_code (branch) != GIMPLE_COND)
> +  if (!bb || gimple_code (branch) != GIMPLE_COND)
>      return;
>  
>    if (gimple_cond_code (branch) != NE_EXPR
> Index: icln/gcc/testsuite/g++.dg/ipa/iinline-1.C
> ===================================================================
> --- icln.orig/gcc/testsuite/g++.dg/ipa/iinline-1.C
> +++ icln/gcc/testsuite/g++.dg/ipa/iinline-1.C
> @@ -29,18 +29,30 @@ int String::funcOne (int delim) const
>    return 1;
>  }
>  
> -int docalling (int (String::* f)(int delim) const)
> +extern int global;
> +
> +int docalling (int c, int (String::* f)(int delim) const)
>  {
>    String S ("muhehehe");
>  
> +  if (c > 2)
> +    global = 3;
> +  else
> +    global = 5;
> +
>    return (S.*f)(4);
>  }
>  
> +int __attribute__ ((noinline,noclone)) get_input (void)
> +{
> +  return 1;
> +}
> +
>  int main (int argc, char *argv[])
>  {
>    int i = 0;
>    while (i < 1000)
> -    i += docalling (&String::funcOne);
> +    i += docalling (get_input (), &String::funcOne);
>    non_existent ("done", i);
>    return 0;
>  }
> 
>