[PATCH, 4.5] Make fwprop find out that non-virtual constant member pointers are non-virtual

Martin Jambor mjambor@suse.cz
Wed Mar 11 14:39:00 GMT 2009


Hi,

the patch below, aimed at 4.5,  is inspired by PR 3713 (yes, only four
digits).  The problem is that when we compile the following simple C++
code we end  up with a run-time conditional jump  (I confirmed this by
examining the final assembly):

    struct A
    {
            void foo()
            {
            }
    };

    int main()
    {
     void (A::* const p)() = & A::foo;
            A a;
            (a.*p)();
    }

The optimized dump looks like:


======================================================================
void A::foo() (struct A * const this)
{
<bb 2>:
  return;

}



;; Function int main() (main)

int main() ()
{
  A:: * p$__pfn;
  struct A a;
  int D.1728;

<bb 2>:
  D.1728 = (int) foo;
  if (D.1728 & 1 != 0)
    goto <bb 3>;
  else
    goto <bb 5>;

<bb 5>:
  p$__pfn = foo;
  goto <bb 4>;

<bb 3>:
  p$__pfn = (A:: *) *(*(int (*__vtbl_ptr_type) (void) * *) &a + (unsigned int) (D.1728 + -1));

<bb 4>:
  p$__pfn (&a);
  return 0;

}
======================================================================

I believe  the condition can  be evaluated at  run time by  looking at
DECL_ALIGN_UNIT  of foo.   The patch  does  just that  in the  forward
propagation pass and gets rid of the run time conditional jump.

On the other  hand, this patch does not achieve  what the bug reporter
wanted in the  first place since the method foo  does not get inlined.
This is because  this patch requires the member  pointer to be already
broken down by SRA  and we run fwprop before SRA, not  after it in the
early  stage.  Moreover,  fwprop after  SRA did  not help  on  its own
either, I had to add ccp there too.  That seems like too much overhead
for such a test case, we either  need a better place where to put this
or do  some pattern matching along  the lines of what  we currently do
indirect inlining to inline this stuff too.

However, I  believe that situations like  these can be  created by the
inliner  (which might  then decide  not to  inline the  member pointer
call) and so should be handled  after inlining too.  And this seems to
work.

I have bootstrapped and tested the patch on x86_64.  Is it OK for
trunk once stage1 opens again?

Thanks

Martin



2009-03-11  Martin Jambor  <mjambor@suse.cz>

	* tree-ssa-forwprop.c (simplify_bitwise_and): New function.
	(tree_ssa_forward_propagate_single_use_vars): Handle assing statements
	with BIT_AND_EXPR on the RHS by calling simplify_bitwise_and.

Index: iinln/gcc/tree-ssa-forwprop.c
===================================================================
--- iinln.orig/gcc/tree-ssa-forwprop.c
+++ iinln/gcc/tree-ssa-forwprop.c
@@ -147,6 +147,14 @@ along with GCC; see the file COPYING3.
 
      ptr2 = &x[index];
 
+  Or
+    ssa = (int) decl
+    res = ssa & 1
+
+  Provided that decl has known alignment >= 2, will get turned into
+
+    res = 0
+
   We also propagate casts into SWITCH_EXPR and COND_EXPR conditions to
   allow us to remove the cast and {NOT_EXPR,NEG_EXPR} into a subsequent
   {NOT_EXPR,NEG_EXPR}.
@@ -1124,6 +1132,50 @@ simplify_gimple_switch (gimple stmt)
     }
 }
 
+/* Check assignments with a "ssa & 1" on the RHS for operating on addresses
+   known to be aligned to at least two.  If so, the result is known to be
+   zero.
+*/
+
+static void
+simplify_bitwise_and (gimple_stmt_iterator *gsi, gimple stmt)
+{
+  gimple def_stmt;
+  tree op = gimple_assign_rhs1 (stmt);
+  tree val = gimple_assign_rhs2 (stmt);
+  tree def_rhs, deref;
+
+  if (!integer_onep (val)
+      || TREE_CODE (op) != SSA_NAME)
+    return;
+
+  def_stmt = SSA_NAME_DEF_STMT (op);
+  if (!gimple_assign_cast_p (def_stmt)
+      || !INTEGRAL_TYPE_P (gimple_expr_type (def_stmt)))
+    return;
+
+  def_rhs = gimple_assign_rhs1 (def_stmt);
+  if (TREE_CODE (def_rhs) != ADDR_EXPR)
+    return;
+
+  deref = TREE_OPERAND (def_rhs, 0);
+  if (DECL_ALIGN_UNIT (deref) < 2)
+    return;
+
+  if (dump_file)
+    {
+      fprintf (dump_file,
+	       "The RHS is always zero, replacing it with a constant: ");
+      print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
+    }
+
+  gimple_assign_set_rhs_from_tree (gsi,
+				   build_int_cst (gimple_expr_type (stmt), 0));
+
+  update_stmt (stmt);
+  return;
+}
+
 /* Main entry point for the forward propagation optimizer.  */
 
 static unsigned int
@@ -1206,6 +1258,11 @@ tree_ssa_forward_propagate_single_use_va
 		  else
 		    gsi_next (&gsi);
 		}
+	      else if (gimple_assign_rhs_code (stmt) == BIT_AND_EXPR)
+		{
+		  simplify_bitwise_and (&gsi, stmt);
+		  gsi_next (&gsi);
+		}
 	      else
 		gsi_next (&gsi);
 	    }
Index: iinln/gcc/testsuite/g++.dg/tree-ssa/fwprop-align.C
===================================================================
--- /dev/null
+++ iinln/gcc/testsuite/g++.dg/tree-ssa/fwprop-align.C
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-forwprop2" } */
+
+struct A
+{
+  void foo ()
+  {
+  }
+};
+
+int main()
+{
+  void (A::* const p)() = & A::foo;
+  A a;
+  (a.*p)();
+}
+
+/* We should eliminate the check if p points to a virtual function. */
+/* { dg-final { scan-tree-dump-times "The RHS is always zero, replacing it with a constant" 1 "forwprop2" } } */
+/* { dg-final { cleanup-tree-dump "forwprop2" } } */



More information about the Gcc-patches mailing list