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

login
register
mail settings
Submitter Martin Jambor
Date June 24, 2010, 10:28 p.m.
Message ID <20100624222800.GA2773@alvy.suse.cz>
Download mbox | patch
Permalink /patch/56865/
State New
Headers show

Comments

Martin Jambor - June 24, 2010, 10:28 p.m.
Hi,

On Mon, Jun 21, 2010 at 01:06:46PM +0200, Richard Guenther wrote:
> On Mon, 21 Jun 2010, Martin Jambor wrote:
> 
> Instead of changing the existing testcase please add a new one.

OK

> 
> 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? 

It must be a structure representing a member pointer.

> Why can't it appear
> on the lhs of a call?  Why can't it appear as operand to an
> asm output?

Obviously you're right, I don't what I was thinking when writing that.

> 
> 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.
> 

I have bootstrapped and tested the following.  If there are no
objections, I will commit it tomorrow.

Thanks for checkin it in detail, this was indeed a stupid mistake,

Martin


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

	* ipa-prop.c (determine_cst_member_ptr): Ignore non-clobbering
	statements instead of bailing out on them.  Updated comments.
	(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-2.C: New testcase.

Patch

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;
@@ -806,6 +807,8 @@  determine_cst_member_ptr (gimple call, t
       gimple stmt = gsi_stmt (gsi);
       tree lhs, rhs, fld;
 
+      if (!stmt_may_clobber_ref_p (stmt, arg))
+	continue;
       if (!gimple_assign_single_p (stmt))
 	return;
 
@@ -814,7 +817,7 @@  determine_cst_member_ptr (gimple call, t
 
       if (TREE_CODE (lhs) != COMPONENT_REF
 	  || TREE_OPERAND (lhs, 0) != arg)
-	continue;
+	return;
 
       fld = TREE_OPERAND (lhs, 1);
       if (!method && fld == method_field)
@@ -1030,6 +1033,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 +1044,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 +1055,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 +1116,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 +1136,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 +1145,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-2.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/iinline-2.C
@@ -0,0 +1,61 @@ 
+/* Verify that simple indirect calls are inlined even without early
+   inlining..  */
+/* { dg-do compile } */
+/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining"  } */
+/* { dg-add-options bind_pic_locally } */
+
+extern void non_existent (const char *, int);
+
+class String
+{
+private:
+  const char *data;
+
+public:
+  String (const char *d) : data(d)
+  {}
+
+  int funcOne (int delim) const;
+  int printStuffTwice (int delim) const;
+};
+
+
+int String::funcOne (int delim) const
+{
+  int i;
+  for (i = 0; i < delim; i++)
+    non_existent(data, i);
+
+  return 1;
+}
+
+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 (get_input (), &String::funcOne);
+  non_existent ("done", i);
+  return 0;
+}
+
+/* { dg-final { scan-ipa-dump "String::funcOne\[^\\n\]*inline copy in int main"  "inline"  } } */
+/* { dg-final { cleanup-ipa-dump "inline" } } */