[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