Re: Fix OBJ_TYPE_REF handling in ipa-cp
Bootstrapped/regtesed x86_64-linux. Martin, please can you review the
change?
* ipa-prop.c (ipa_set_jf_known_type): Check that component type
is a record type with BINFO.
(detect_type_change_ssa): Add comp_type argument.
(compute_complex_assign_jump_func): Add param_type argument; pass
it down to detect_type_change_ssa.
(compute_known_type_jump_func): Add expected_type parameter.
Do not bother tracking a non-polymorphic type.
(ipa_get_callee_param_type): New function.
(ipa_compute_jump_functions_for_edge): Pass down calle parm types.
(ipa_analyze_virtual_call_uses): Use class typee as argument
of detect_type_change_1.
(ipa_intraprocedural_devirtualization): Pass down class type.
Hopefully I'll get rid of the component_types in the jump functions
and most of this won't be necesary. Meanwhile, this is OK.
Indeed, I hope these will go. I updated the patch to tree after your changes
and fixed the remaining uses of detect_type_change so detect_type_change_1 can
go completely.
Bootstrapped/regtested x86_64-linux and also tested with Firefox build,
comitted.
Interestingly, we now have smaller static counts of devirtualization on firefox
than two weeks ago. It is not caused by this patch however. While looking for
possible causes I also noticed we don't seem to handle invisible references
well. Consider:
class A {virtual void b(void) { };};
class B : public A {virtual void b(void) { };};
void
q (class A a);
void
qq (class A *a);
void
t (class A a)
{
q(a);
qq(a);
}
void
m()
{
class B b;
t(b);
}
With -O2 -fno-early-inling -flto I get:
m() ()
{
struct B b;
struct A D.2267;
bb 2:
B::B (b);
A::A (D.2267, b.D.2212);
t (D.2267);
D.2267 ={v} {CLOBBER};
b ={v} {CLOBBER};
return;
}
t(A) (struct A restrict a)
{
struct A D.2242;
bb 2:
A::A (D.2242, a_2(D));
q (D.2242);
D.2242 ={v} {CLOBBER};
qq (a_2(D));
return;
}
I.e. t is called with invisible reference.
Now we get:
Jump functions:
Jump functions of caller m()/12:
callsite m()/12 - t(A)/5 :
param 0: UNKNOWN
callsite m()/12 - A::A(A const)/4 :
param 0: KNOWN TYPE: base struct A, offset 0, component struct A
param 1: KNOWN TYPE: base struct B, offset 0, component const struct A
callsite m()/12 - B::B()/11 :
param 0: KNOWN TYPE: base struct B, offset 0, component struct B
Jump functions of caller t(A)/5:
callsite t(A)/5 - qq(A*)/20 :
param 0: PASS THROUGH: 0, op nop_expr, type_preserved
callsite t(A)/5 - q(A)/19 :
param 0: UNKNOWN
callsite t(A)/5 - A::A(A const)/4 :
param 0: KNOWN TYPE: base struct A, offset 0, component struct A
param 1: PASS THROUGH: 0, op nop_expr, type_preserved
I think m()-t() has no type and t-qq is pass thorugh. So we miss the
fact that t-qq is known type of A.
Do we need jump function that is PASS THROGH KNOWN_TYPE at once?
Honza
* ipa-prop.c (ipa_set_jf_known_type): Check that we add
only records.
(detect_type_change_1): Rename to ...
(detect_type_change): ... this one; early return on non-polymorphic
types.
(detect_type_change_ssa): Add comp_type parameter; update
use of detect_type_change.
(compute_complex_assign_jump_func): Add param_type parameter;
update use of detect_type_change_ssa.
(compute_complex_ancestor_jump_func): Likewise.
(ipa_get_callee_param_type): New function.
(ipa_compute_jump_functions_for_edge): Compute parameter type;
update calls to the jump function computation functions.
Index: ipa-prop.c
===
--- ipa-prop.c (revision 202092)
+++ ipa-prop.c (working copy)
@@ -371,6 +371,8 @@ static void
ipa_set_jf_known_type (struct ipa_jump_func *jfunc, HOST_WIDE_INT offset,
tree base_type, tree component_type)
{
+ gcc_assert (TREE_CODE (component_type) == RECORD_TYPE
+ TYPE_BINFO (component_type));
jfunc-type = IPA_JF_KNOWN_TYPE;
jfunc-value.known_type.offset = offset,
jfunc-value.known_type.base_type = base_type;
@@ -633,13 +635,16 @@ check_stmt_for_type_change (ao_ref *ao A
-/* Like detect_type_change but with extra argument COMP_TYPE which will become
- the component type part of new JFUNC of dynamic type change is detected and
- the new base type is identified. */
+/* Detect whether the dynamic type of ARG of COMP_TYPE has changed (before
+ callsite CALL) by looking for assignments to its virtual table pointer. If
+ it is, return true and fill in the jump function JFUNC with relevant type
+ information or set it to unknown. ARG is the object itself (not a pointer
+ to it, unless dereferenced). BASE is the base of the memory access as
+ returned by get_ref_base_and_extent, as is the offset. */
static bool
-detect_type_change_1 (tree arg, tree base,
Re: Fix OBJ_TYPE_REF handling in ipa-cp
Hi, On Thu, Aug 22, 2013 at 05:33:50PM +0200, Jan Hubicka wrote: Hi, this problem was noticed by my verifier that binfo walks are not across type hiearchy. ipa_intraprocedural_devirtualization is one remaining place where we take class of object from OBJ_TYPE_REF_OBJECT instead of obj_type_ref_class_type. Unforutnately I noticed that this problem is propagated quite further across ipa-prop design. We assume that types of pointers taken from gimple call arguments are types of pointers to classes pass down to the callee. This is not true after propagation. I did not fix the all places, only places needed to get parameter for ipa_set_jf_known_type and detect_type_change_ssa right. Also I modified ipa_set_jf_known_type to not record non-polymorphic type. It is a waste of memory and LTO streaming bandwidth. Bootstrapped/regtesed x86_64-linux. Martin, please can you review the change? * ipa-prop.c (ipa_set_jf_known_type): Check that component type is a record type with BINFO. (detect_type_change_ssa): Add comp_type argument. (compute_complex_assign_jump_func): Add param_type argument; pass it down to detect_type_change_ssa. (compute_known_type_jump_func): Add expected_type parameter. Do not bother tracking a non-polymorphic type. (ipa_get_callee_param_type): New function. (ipa_compute_jump_functions_for_edge): Pass down calle parm types. (ipa_analyze_virtual_call_uses): Use class typee as argument of detect_type_change_1. (ipa_intraprocedural_devirtualization): Pass down class type. Hopefully I'll get rid of the component_types in the jump functions and most of this won't be necesary. Meanwhile, this is OK. Thanks, Martin
Fix OBJ_TYPE_REF handling in ipa-cp
Hi,
this problem was noticed by my verifier that binfo walks are not across type
hiearchy. ipa_intraprocedural_devirtualization is one remaining place where we
take class of object from OBJ_TYPE_REF_OBJECT instead of
obj_type_ref_class_type.
Unforutnately I noticed that this problem is propagated quite further across
ipa-prop
design. We assume that types of pointers taken from gimple call arguments
are types of pointers to classes pass down to the callee. This is not true
after propagation.
I did not fix the all places, only places needed to get parameter for
ipa_set_jf_known_type and detect_type_change_ssa right. Also I modified
ipa_set_jf_known_type to not record non-polymorphic type. It is a waste
of memory and LTO streaming bandwidth.
Bootstrapped/regtesed x86_64-linux. Martin, please can you review the change?
* ipa-prop.c (ipa_set_jf_known_type): Check that component type
is a record type with BINFO.
(detect_type_change_ssa): Add comp_type argument.
(compute_complex_assign_jump_func): Add param_type argument; pass
it down to detect_type_change_ssa.
(compute_known_type_jump_func): Add expected_type parameter.
Do not bother tracking a non-polymorphic type.
(ipa_get_callee_param_type): New function.
(ipa_compute_jump_functions_for_edge): Pass down calle parm types.
(ipa_analyze_virtual_call_uses): Use class typee as argument
of detect_type_change_1.
(ipa_intraprocedural_devirtualization): Pass down class type.
Index: ipa-prop.c
===
--- ipa-prop.c (revision 201919)
+++ ipa-prop.c (working copy)
@@ -367,6 +367,8 @@ static void
ipa_set_jf_known_type (struct ipa_jump_func *jfunc, HOST_WIDE_INT offset,
tree base_type, tree component_type)
{
+ gcc_assert (TREE_CODE (component_type) == RECORD_TYPE
+ TYPE_BINFO (component_type));
jfunc-type = IPA_JF_KNOWN_TYPE;
jfunc-value.known_type.offset = offset,
jfunc-value.known_type.base_type = base_type;
@@ -674,20 +676,18 @@ detect_type_change (tree arg, tree base,
/* Like detect_type_change but ARG is supposed to be a non-dereferenced pointer
SSA name (its dereference will become the base and the offset is assumed to
- be zero). */
+ be zero). COMP_TYPE is type of object being tracked. */
static bool
-detect_type_change_ssa (tree arg, gimple call, struct ipa_jump_func *jfunc)
+detect_type_change_ssa (tree arg, tree comp_type,
+ gimple call, struct ipa_jump_func *jfunc)
{
- tree comp_type;
-
gcc_checking_assert (TREE_CODE (arg) == SSA_NAME);
if (!flag_devirtualize
|| !POINTER_TYPE_P (TREE_TYPE (arg))
|| TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) != RECORD_TYPE)
return false;
- comp_type = TREE_TYPE (TREE_TYPE (arg));
arg = build2 (MEM_REF, ptr_type_node, arg,
build_int_cst (ptr_type_node, 0));
@@ -961,13 +961,16 @@ ipa_load_from_parm_agg (struct ipa_node_
INFO is the structure describing individual parameters access different
stages of IPA optimizations. PARMS_AINFO contains the information that is
- only needed for intraprocedural analysis. */
+ only needed for intraprocedural analysis.
+
+ PARAM_TYPE is type of the parameter, NULL if not available. */
static void
compute_complex_assign_jump_func (struct ipa_node_params *info,
struct param_analysis_info *parms_ainfo,
struct ipa_jump_func *jfunc,
- gimple call, gimple stmt, tree name)
+ gimple call, gimple stmt, tree name,
+ tree param_type)
{
HOST_WIDE_INT offset, size, max_size;
tree op1, tc_ssa, base, ssa;
@@ -1006,7 +1009,10 @@ compute_complex_assign_jump_func (struct
gimple_assign_rhs_code (stmt));
}
else if (gimple_assign_single_p (stmt)
- !detect_type_change_ssa (tc_ssa, call, jfunc))
+ param_type
+ !detect_type_change_ssa (tc_ssa,
+ TREE_TYPE (param_type),
+ call, jfunc))
{
bool agg_p = parm_ref_data_pass_through_p (parms_ainfo[index],
call, tc_ssa);
@@ -1171,18 +1177,25 @@ compute_complex_ancestor_jump_func (stru
/* Given OP which is passed as an actual argument to a called function,
determine if it is possible to construct a KNOWN_TYPE jump function for it
- and if so, create one and store it to JFUNC. */
+ and if so, create one and store it to JFUNC.
+ EXPECTED_TYPE represents a type the argument should be in */
static void
compute_known_type_jump_func (tree op, struct ipa_jump_func *jfunc,
- gimple call)
+
