* [PATCH, PR 45934 0/6] Devirtualization aware of dynamic type changes
@ 2010-12-01 20:22 Martin Jambor
2010-12-01 20:22 ` [PATCH, PR 45934 3/6] More robust compute_complex_ancestor_jump_func Martin Jambor
` (5 more replies)
0 siblings, 6 replies; 29+ messages in thread
From: Martin Jambor @ 2010-12-01 20:22 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Guenther, Jan Hubicka
Hi,
this series of patches is another attempt to address dynamic type
changes, a followup to
http://gcc.gnu.org/ml/gcc-patches/2010-11/msg02296.html.
Sorry it took me so long. It took me a few days to figure out exactly
what to do and how to justify it and then I started splitting the
patch into separate steps and making them at lest somewhat pretty and
that took even more time. Last but not least, this patch set also
fixes PR 46302 which the previous one did not because somehow one
important call to detect_type_change got lost in the process. I have
added an extra testcase for that situation.
The series is divided into the following patches. The division wasn't
smooth all the time, on two or three occasions a subsequent patch
undoes some tiny change introduced in a previous one.
1. A re-post of
http://gcc.gnu.org/ml/gcc-patches/2010-11/msg02291.html, already
reviewed by Richi but still waiting for Honza's approval. Yep,
this is basically a ping.
2. Disabling devirtualization in folding and that based on global
variables which do not work because of dynamic type changes.
3. More robust compute_complex_ancestor_jump_func - a slight
improvement in jump function building.
4. Detecting dynamic type changes of objects with virtual methods and
disabling devirtualization upon their encounter.
5. Identifying the new dynamic type of an object after such change
when we can do so and using it to drive devirtualization instead of
disabling it.
6. Intraprocedural BINFO-based devirtualization. Necessary to perform
devirtualization when there is no IPA transfer of information
involved.
More details about individual steps are in the corresponding email
messages.
I'm about to start working on a separate switch for controlling
devirtualization - I intend to put it in between the last two patches
in the above series.
Thanks for all feedback, including that I have already received as
comments to the previous version of this patch,
Martin
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH, PR 45934 3/6] More robust compute_complex_ancestor_jump_func
2010-12-01 20:22 [PATCH, PR 45934 0/6] Devirtualization aware of dynamic type changes Martin Jambor
@ 2010-12-01 20:22 ` Martin Jambor
2010-12-01 20:28 ` Richard Guenther
2010-12-01 20:23 ` [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks Martin Jambor
` (4 subsequent siblings)
5 siblings, 1 reply; 29+ messages in thread
From: Martin Jambor @ 2010-12-01 20:22 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Guenther, Jan Hubicka
[-- Attachment #1: complex_assign_improvement.diff --]
[-- Type: text/plain, Size: 1693 bytes --]
Hi,
compute_complex_ancestor_jump_func tries to match a pattern like this:
if (obj_2(D) != 0B)
goto <bb 3>;
else
goto <bb 4>;
<bb 3>:
iftmp.1_3 = &obj_2(D)->D.1762;
<bb 4>:
# iftmp.1_1 = PHI <iftmp.1_3(3), 0B(2)>
D.1879_6 = middleman_1 (iftmp.1_1, i_5(D));
return D.1879_6; */
At some point while working at the devirtualization issues I have seen
this failing because the zero was the zeroth parameter in the phi
node. I think it is necessary for some of the new testcases to work
but I am not really sure any more, I only remembered when I was
splitting the big patch into separate ones.
I did not bootstrap this patch separately but it did pas bootstrap and
testsuite on x86_64-linux and make check-c++ on i686 together with the
rest.
Thanks,
Martin
2010-11-30 Martin Jambor <mjambor@suse.cz>
* ipa-prop.c (compute_complex_ancestor_jump_func): Work also if the
zero is the first phi parameter.
Index: icln/gcc/ipa-prop.c
===================================================================
--- icln.orig/gcc/ipa-prop.c
+++ icln/gcc/ipa-prop.c
@@ -461,11 +461,15 @@ compute_complex_ancestor_jump_func (stru
tree tmp, parm, expr;
int index, i;
- if (gimple_phi_num_args (phi) != 2
- || !integer_zerop (PHI_ARG_DEF (phi, 1)))
+ if (gimple_phi_num_args (phi) != 2)
return;
- tmp = PHI_ARG_DEF (phi, 0);
+ if (integer_zerop (PHI_ARG_DEF (phi, 1)))
+ tmp = PHI_ARG_DEF (phi, 0);
+ else if (integer_zerop (PHI_ARG_DEF (phi, 0)))
+ tmp = PHI_ARG_DEF (phi, 1);
+ else
+ return;
if (TREE_CODE (tmp) != SSA_NAME
|| SSA_NAME_IS_DEFAULT_DEF (tmp)
|| !POINTER_TYPE_P (TREE_TYPE (tmp))
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH, PR 45934 4/6] Dynamic type change detection
2010-12-01 20:22 [PATCH, PR 45934 0/6] Devirtualization aware of dynamic type changes Martin Jambor
` (2 preceding siblings ...)
2010-12-01 20:23 ` [PATCH, PR 45934 6/6] Intraprocedural type-based devirtualization Martin Jambor
@ 2010-12-01 20:23 ` Martin Jambor
2010-12-02 15:19 ` Richard Guenther
2010-12-02 23:25 ` Jason Merrill
2010-12-01 20:23 ` [PATCH, PR 45934 5/6] Identify the new dynamic type after change Martin Jambor
2010-12-01 20:23 ` [PATCH, PR 45934 2/6] Remove devirtualizations that cannot be done Martin Jambor
5 siblings, 2 replies; 29+ messages in thread
From: Martin Jambor @ 2010-12-01 20:23 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Guenther, Jan Hubicka
[-- Attachment #1: dynamic_type_change_detection.diff --]
[-- Type: text/plain, Size: 32078 bytes --]
This is the crux of the matter. I'll try to explain what and why I do
in order to detect sub-object type changes during their construction
first, below it there is the detection code as it looks like with the
next patch applied and then there is of course the patch at the end.
I have split the detection code into two patches, this one just
detects that there is a type change in force and the subsequent one
also tries to derive the new type. I did this in order to better
structure the discussion about both and I also intend to commit the
separately, easing potential bisecting (I hope there will be no reason
for that of course). If it was absolutely necessary, we could
postpone the next patch for stage1 but I hope to commit a variant of
it soon.
Because operations like placement new, memcpy and other byte-per-byte
operations with objects that have virtual methods are deemed to
produce code with undefined behavior and there are no unions of
non-POD types, dynamic type change has a special meaning for
devirtualization and only refers to what actual virtual method table a
VMT pointer of an object points to. On the other hand the type in the
sense of what you get from TREE_TYPE of the base of any given access
to it is still the same and it is its static type. An important
property of such objects is that their dynamic types (as given by the
current VMT) can only be altered in constructors and destructors.
This patch makes special assumptions about both constructors and
destructors which are all the functions that are allowed to alter the
dynamic types. It assumes that destructors begin with assignment into
all VMT pointers and that constructors essentially look in the
following way:
1) The very first thing they do is that they call constructors of the
components (including ancestor sub-objects) that have them.
2) Then VMT pointers of this and all its ancestors is set to new
values corresponding to the type corresponding to the constructor.
3) Only afterwards, other stuff such as the code written by the user
is run. Only this may include calling virtual functions, directly
or indirectly.
There is no way to call a constructor of an ancestor sub-object in any
other way (or any component for that matter but type is interesting
only for ancestors).
This means that we do not have to care whether constructors get the
correct type information because they will always change it (in fact,
if we define the type to be given by the VMT pointer, it is
undefined).
The most important fact to derive from the above is that if, for some
statement in the section 3, we try to detect whether the dynamic type
has changed, we can safely ignore all calls as we examine the function
body backwards until we reach statements in section 2 because these
calls cannot be constructors or destructors (if the input is not
bogus) and so do not change the dynamic type. We then must detect
that statements in section 2 change the dynamic type and can try to
derive the new type. That is enough and we can stop, we will never
see the calls into constructors of sub-objects in this code.
Therefore we can safely ignore all call statements that we traverse,
possibly except builtins which we consider as changing the type in an
unknown way and which previous optimization passes might have
introduced.
Constructors of ancestors may be early inlined but that does not
change the above division. Function splitting may lead to inlining of
only a portion of a constructor but it can only inline the beginning
of the constructor, not some part in the middle and so the division
again holds. Moreover, the inlined portion will essentially be
divided into three such sections too (perhaps some of the latter ones
in another function which means that the type we pass to it either
does not matter or is known) and therefore the same arguments will
also hold for the associated sub-object.
Exceptions can change this flow and lead to invocations of destructors
of the completely constructed sub-objects in places where the method
based on examining all possible previous statements might fail.
However, because destructors begin with assignment to all VMT
pointers, the type information passed to them is never used anyway.
Apart from the destructors, user code cannot handle exceptions that
escape an ancestor constructor in any of the constructors of the
descendants and so we will never encounter calls to other functions in
these areas.
Therefore we do the following. Whenever we are about to construct a
jump function that carries type information (known_type, ancestor or
pass_through) or note that a virtual call is based on the type of a
parameter, we invoke a function to check whether the dynamic type has
changed and see whether we can deduce the new type. This function
traverses VDEFs from the call site and examines individual statements
and their potential or known effect on the VMT pointers in question.
When we do this, we employ the TBAA disambiguation using the static
type of the (sub-)object for reasons stated above when defining what
we mean by dynamic type change. I believe we can even use types
derived from pointers because of what conversions are allowed by the
C++ standard.
When examining the statements, we default to assume VMT pointers are
changed in unpredictable ways. There are, however, two exceptions.
The first are calls to non-builtins which we consider harmless for the
reasons above. The second one are assignments to COMPONENT_REFs where
we check whether the FIELD_DECL has the virtual flag set or not. If
it is not set, we assume this is not a store to the VMT pointer. In
the future, we might want to add more such cases (like ignoring
ARRAY_REFs too, for example) but so far I am happy with the two.
If the assignment is a COMPONENT_REF to a field with the virtual flag
set and the base is the same decl or SSA name as the object we are
examining, we try to deduce the new type from the right hand side.
Tho code that does both detection of dynamic type changes and tries to
extract the new dynamic type in some cases is the following:
----------------------------------------------------------------------
/* Structure to be passed in between detect_type_change_anc and
check_stmt_for_type_change. */
struct type_change_info
{
/* The declaration or SSA_NAME pointer of the base that we are checking for
type change. */
tree object;
/* If we actually can tell the type that the object has changed to, it is
stored in this field. Otherwise it remains NULL_TREE. */
tree known_current_type;
/* Set to true if dynamic type change has been detected. */
bool type_maybe_changed;
};
/* Return true if STMT can modify a virtual method table pointer. The function
assumes it will never be called on constructor or destructor calls (since
the search for dynamic type change will always end before reaching the
former and lifetime of the object is over anyway after the latter. */
static bool
stmt_may_be_vtbl_ptr_store (gimple stmt)
{
if (is_gimple_call (stmt))
{
tree fndecl = gimple_call_fndecl (stmt);
if (!fndecl || !DECL_BUILT_IN (fndecl))
return false;
}
else if (is_gimple_assign (stmt))
{
tree lhs = gimple_assign_lhs (stmt);
if (TREE_CODE (lhs) == COMPONENT_REF
&& !DECL_VIRTUAL_P (TREE_OPERAND (lhs, 1)))
return false;
/* In the future we might want to use get_base_ref_and_offset to find
if there is a field corresponding to the offset and if so, proceed
almost like if it was a component ref. */
}
return true;
}
/* If STMT can be proved to be an assignment to the virtual method table
pointer of ANALYZED_OBJ and the type associated witht the new table
identified, return the type. Otherwise return NULL_TREE. */
static tree
extr_type_from_vtbl_ptr_store (gimple stmt, tree analyzed_obj)
{
tree lhs, t, obj;
if (!is_gimple_assign (stmt))
return NULL_TREE;
lhs = gimple_assign_lhs (stmt);
if (TREE_CODE (lhs) != COMPONENT_REF)
return NULL_TREE;
obj = lhs;
if (!DECL_VIRTUAL_P (TREE_OPERAND (lhs, 1)))
return NULL_TREE;
do
{
obj = TREE_OPERAND (obj, 0);
} while (TREE_CODE (obj) == COMPONENT_REF);
if (TREE_CODE (obj) == MEM_REF)
obj = TREE_OPERAND (obj, 0);
if (TREE_CODE (obj) == ADDR_EXPR)
obj = TREE_OPERAND (obj, 0);
if (obj != analyzed_obj)
return NULL_TREE;
t = gimple_assign_rhs1 (stmt);
if (TREE_CODE (t) != ADDR_EXPR)
return NULL_TREE;
t = get_base_address (TREE_OPERAND (t, 0));
if (!t || TREE_CODE (t) != VAR_DECL || !DECL_VIRTUAL_P (t))
return NULL_TREE;
return DECL_CONTEXT (t);
}
/* Callbeck of walk_aliased_vdefs and a helper function for
detect_type_change_anc to check whether a particular statement may modify
the virtual table pointer, and if possible also determine the new type of
the (sub-)object. It stores its result into DATA, which points to a
type_change_info structure. */
static bool
check_stmt_for_type_change (ao_ref *ao ATTRIBUTE_UNUSED, tree vdef, void *data)
{
gimple stmt = SSA_NAME_DEF_STMT (vdef);
struct type_change_info *tci = (struct type_change_info *) data;
if (stmt_may_be_vtbl_ptr_store (stmt))
{
tci->type_maybe_changed = true;
tci->known_current_type = extr_type_from_vtbl_ptr_store (stmt,
tci->object);
return true;
}
else
return false;
}
/* Detect whether the dynamic type of ARG 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. If
ANC_OFFSET is non-zero, look for binfo of its ancestor of that type at that
offset. ARG is supposed to be a dereferenced pointer or a declaration or a
series of component_refs of either. */
static bool
detect_type_change_anc (tree arg, gimple call, struct ipa_jump_func *jfunc,
HOST_WIDE_INT anc_offset)
{
struct type_change_info tci;
tree obj;
ao_ref ar;
/* Const calls cannot call virtual methods through VMT and so type changes do
not matter. */
if (!gimple_vuse (call))
return false;
obj = arg;
while (handled_component_p (obj))
obj = TREE_OPERAND (obj, 0);
if (TREE_CODE (obj) == MEM_REF)
obj = TREE_OPERAND (obj, 0);
if (TREE_CODE (obj) == ADDR_EXPR)
obj = TREE_OPERAND (obj, 0);
tci.object = obj;
tci.known_current_type = NULL_TREE;
tci.type_maybe_changed = false;
ao_ref_init (&ar, arg);
walk_aliased_vdefs (&ar, gimple_vuse (call), check_stmt_for_type_change,
&tci, NULL);
if (!tci.type_maybe_changed)
return false;
if (!tci.known_current_type)
jfunc->type = IPA_JF_UNKNOWN;
else if (anc_offset != 0)
{
tree new_binfo = get_binfo_at_offset (TYPE_BINFO (tci.known_current_type),
anc_offset, TREE_TYPE (arg));
if (new_binfo)
{
jfunc->type = IPA_JF_KNOWN_TYPE;
jfunc->value.base_binfo = new_binfo;
}
else
jfunc->type = IPA_JF_UNKNOWN;
}
else
{
/* If an offset of a searched-for sub-object actually happens to be zero,
we do not have to worry about non-artificialness. Either there are no
virtual methods anyway or there is a VMT pointer at offset zero and so
artificial sub-objects start at higher offsets. */
jfunc->type = IPA_JF_KNOWN_TYPE;
jfunc->value.base_binfo = TYPE_BINFO (tci.known_current_type);
}
return true;
}
/* Like detect_type_change_anc but ARG is supposed to be a non-dereferenced
pointer SSA name. */
static bool
detect_type_change (tree arg, gimple call, struct ipa_jump_func *jfunc)
{
if (!POINTER_TYPE_P (TREE_TYPE (arg))
|| TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) != RECORD_TYPE)
return false;
gcc_checking_assert (TREE_CODE (arg) != ADDR_EXPR);
arg = build_simple_mem_ref (arg);
/* Building a MEM_REF just for the sake of ao_ref_init is probably
wasteful here, I'll be happy to discuss alternatives. */
return detect_type_change_anc (arg, call, jfunc, 0);
}
----------------------------------------------------------------------
The patch implementing just the detection of changes with all the
callers is below, the rest is in the next patch.
I have bootstrapped and tested this patch separately (i.e. with the
previous ones but not the subsequent ones) on x86-64-linux and it has
also passed make check-c++ on i686.
Thanks for any comments,
Martin
2010-11-29 Martin Jambor <mjambor@suse.cz>
PR tree-optimization/45934
PR tree-optimization/46302
* ipa-prop.c (type_change_info): New type.
(stmt_may_be_vtbl_ptr_store): New function.
(check_stmt_for_type_change): Likewise.
(detect_type_change_1): Likewise.
(detect_type_change): Likewise.
(compute_complex_assign_jump_func): Check for dynamic type change.
(compute_complex_ancestor_jump_func): Likewise.
(compute_known_type_jump_func): Likewise.
(compute_scalar_jump_functions): Likewise.
(ipa_analyze_virtual_call_uses): Likewise.
* testsuite/g++.dg/ipa/devirt-c-1.C: New test.
* testsuite/g++.dg/ipa/devirt-c-2.C: Likewise.
* testsuite/g++.dg/ipa/devirt-c-3.C: Likewise.
* testsuite/g++.dg/ipa/devirt-c-4.C: Likewise.
* testsuite/g++.dg/ipa/devirt-c-5.C: Likewise.
* testsuite/g++.dg/ipa/devirt-d-1.C: Likewise.
* testsuite/g++.dg/torture/pr45934.C: Likewise.
Index: icln/gcc/ipa-prop.c
===================================================================
--- icln.orig/gcc/ipa-prop.c
+++ icln/gcc/ipa-prop.c
@@ -350,6 +350,111 @@ ipa_print_all_jump_functions (FILE *f)
}
}
+/* Structure to be passed in between detect_type_change_anc and
+ check_stmt_for_type_change. */
+
+struct type_change_info
+{
+ /* Set to true if dynamic type change has been detected. */
+ bool type_maybe_changed;
+};
+
+/* Return true if STMT can modify a virtual method table pointer. The function
+ assumes it will never be called on constructor or destructor calls (since
+ the search for dynamic type change will always end before reaching the
+ former and lifetime of the object is over anyway after the latter. */
+
+static bool
+stmt_may_be_vtbl_ptr_store (gimple stmt)
+{
+ if (is_gimple_call (stmt))
+ {
+ tree fndecl = gimple_call_fndecl (stmt);
+ if (!fndecl || !DECL_BUILT_IN (fndecl))
+ return false;
+ }
+ else if (is_gimple_assign (stmt))
+ {
+ tree lhs = gimple_assign_lhs (stmt);
+
+ if (TREE_CODE (lhs) == COMPONENT_REF
+ && !DECL_VIRTUAL_P (TREE_OPERAND (lhs, 1)))
+ return false;
+ /* In the future we might want to use get_base_ref_and_offset to find
+ if there is a field corresponding to the offset and if so, proceed
+ almost like if it was a component ref. */
+ }
+ return true;
+}
+
+/* Callbeck of walk_aliased_vdefs and a helper function for
+ detect_type_change_anc to check whether a particular statement may modify
+ the virtual table pointer, and if possible also determine the new type of
+ the (sub-)object. It stores its result into DATA, which points to a
+ type_change_info structure. */
+
+static bool
+check_stmt_for_type_change (ao_ref *ao ATTRIBUTE_UNUSED, tree vdef, void *data)
+{
+ gimple stmt = SSA_NAME_DEF_STMT (vdef);
+ struct type_change_info *tci = (struct type_change_info *) data;
+
+ if (stmt_may_be_vtbl_ptr_store (stmt))
+ {
+ tci->type_maybe_changed = true;
+ return true;
+ }
+ else
+ return false;
+}
+
+/* Detect whether the dynamic type of ARG 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. If
+ ANC_OFFSET is non-zero, look for binfo of its ancestor of that type at that
+ offset. ARG is supposed to be a dereferenced pointer or a declaration or a
+ series of component_refs of either. */
+
+static bool
+detect_type_change_anc (tree arg, gimple call, struct ipa_jump_func *jfunc,
+ HOST_WIDE_INT anc_offset ATTRIBUTE_UNUSED)
+{
+ struct type_change_info tci;
+ ao_ref ar;
+
+ /* Const calls cannot call virtual methods through VMT and so type changes do
+ not matter. */
+ if (!gimple_vuse (call))
+ return false;
+
+ tci.type_maybe_changed = false;
+
+ ao_ref_init (&ar, arg);
+ walk_aliased_vdefs (&ar, gimple_vuse (call), check_stmt_for_type_change,
+ &tci, NULL);
+ if (!tci.type_maybe_changed)
+ return false;
+
+ jfunc->type = IPA_JF_UNKNOWN;
+ return true;
+}
+
+/* Like detect_type_change_anc but ARG is supposed to be a non-dereferenced
+ pointer SSA name. */
+
+static bool
+detect_type_change (tree arg, gimple call, struct ipa_jump_func *jfunc)
+{
+ if (!POINTER_TYPE_P (TREE_TYPE (arg))
+ || TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) != RECORD_TYPE)
+ return false;
+ gcc_checking_assert (TREE_CODE (arg) != ADDR_EXPR);
+ arg = build_simple_mem_ref (arg);
+
+ return detect_type_change_anc (arg, call, jfunc, 0);
+}
+
+
/* Given that an actual argument is an SSA_NAME (given in NAME) and is a result
of an assignment statement STMT, try to find out whether NAME can be
described by a (possibly polynomial) pass-through jump-function or an
@@ -359,10 +464,10 @@ ipa_print_all_jump_functions (FILE *f)
static void
compute_complex_assign_jump_func (struct ipa_node_params *info,
struct ipa_jump_func *jfunc,
- gimple stmt, tree name)
+ gimple call, gimple stmt, tree name)
{
HOST_WIDE_INT offset, size, max_size;
- tree op1, op2, base, type;
+ tree op1, op2, base;
int index;
op1 = gimple_assign_rhs1 (stmt);
@@ -388,7 +493,8 @@ compute_complex_assign_jump_func (struct
jfunc->value.pass_through.operation = gimple_assign_rhs_code (stmt);
jfunc->value.pass_through.operand = op2;
}
- else if (gimple_assign_unary_nop_p (stmt))
+ else if (gimple_assign_unary_nop_p (stmt)
+ && !detect_type_change (op1, call, jfunc))
{
jfunc->type = IPA_JF_PASS_THROUGH;
jfunc->value.pass_through.formal_id = index;
@@ -399,10 +505,8 @@ compute_complex_assign_jump_func (struct
if (TREE_CODE (op1) != ADDR_EXPR)
return;
-
op1 = TREE_OPERAND (op1, 0);
- type = TREE_TYPE (op1);
- if (TREE_CODE (type) != RECORD_TYPE)
+ if (TREE_CODE (TREE_TYPE (op1)) != RECORD_TYPE)
return;
base = get_ref_base_and_extent (op1, &offset, &size, &max_size);
if (TREE_CODE (base) != MEM_REF
@@ -419,12 +523,13 @@ compute_complex_assign_jump_func (struct
/* Dynamic types are changed only in constructors and destructors and */
index = ipa_get_param_decl_index (info, SSA_NAME_VAR (base));
- if (index >= 0)
+ if (index >= 0
+ && !detect_type_change_anc (op1, call, jfunc, offset))
{
jfunc->type = IPA_JF_ANCESTOR;
jfunc->value.ancestor.formal_id = index;
jfunc->value.ancestor.offset = offset;
- jfunc->value.ancestor.type = type;
+ jfunc->value.ancestor.type = TREE_TYPE (op1);
}
}
@@ -453,12 +558,12 @@ compute_complex_assign_jump_func (struct
static void
compute_complex_ancestor_jump_func (struct ipa_node_params *info,
struct ipa_jump_func *jfunc,
- gimple phi)
+ gimple call, gimple phi)
{
HOST_WIDE_INT offset, size, max_size;
gimple assign, cond;
basic_block phi_bb, assign_bb, cond_bb;
- tree tmp, parm, expr;
+ tree tmp, parm, expr, obj;
int index, i;
if (gimple_phi_num_args (phi) != 2)
@@ -486,6 +591,7 @@ compute_complex_ancestor_jump_func (stru
if (TREE_CODE (expr) != ADDR_EXPR)
return;
expr = TREE_OPERAND (expr, 0);
+ obj = expr;
expr = get_ref_base_and_extent (expr, &offset, &size, &max_size);
if (TREE_CODE (expr) != MEM_REF
@@ -513,7 +619,6 @@ compute_complex_ancestor_jump_func (stru
|| !integer_zerop (gimple_cond_rhs (cond)))
return;
-
phi_bb = gimple_bb (phi);
for (i = 0; i < 2; i++)
{
@@ -522,10 +627,13 @@ compute_complex_ancestor_jump_func (stru
return;
}
- jfunc->type = IPA_JF_ANCESTOR;
- jfunc->value.ancestor.formal_id = index;
- jfunc->value.ancestor.offset = offset;
- jfunc->value.ancestor.type = TREE_TYPE (TREE_TYPE (tmp));
+ if (!detect_type_change_anc (obj, call, jfunc, offset))
+ {
+ jfunc->type = IPA_JF_ANCESTOR;
+ jfunc->value.ancestor.formal_id = index;
+ jfunc->value.ancestor.offset = offset;
+ jfunc->value.ancestor.type = TREE_TYPE (obj);;
+ }
}
/* Given OP whch is passed as an actual argument to a called function,
@@ -533,7 +641,8 @@ compute_complex_ancestor_jump_func (stru
and if so, create one and store it to JFUNC. */
static void
-compute_known_type_jump_func (tree op, struct ipa_jump_func *jfunc)
+compute_known_type_jump_func (tree op, struct ipa_jump_func *jfunc,
+ gimple call)
{
HOST_WIDE_INT offset, size, max_size;
tree base, binfo;
@@ -551,6 +660,9 @@ compute_known_type_jump_func (tree op, s
|| is_global_var (base))
return;
+ if (detect_type_change_anc (op, call, jfunc, offset))
+ return;
+
binfo = TYPE_BINFO (TREE_TYPE (base));
if (!binfo)
return;
@@ -592,7 +704,8 @@ compute_scalar_jump_functions (struct ip
{
int index = ipa_get_param_decl_index (info, SSA_NAME_VAR (arg));
- if (index >= 0)
+ if (index >= 0
+ && !detect_type_change (arg, call, &functions[num]))
{
functions[num].type = IPA_JF_PASS_THROUGH;
functions[num].value.pass_through.formal_id = index;
@@ -604,14 +717,14 @@ compute_scalar_jump_functions (struct ip
gimple stmt = SSA_NAME_DEF_STMT (arg);
if (is_gimple_assign (stmt))
compute_complex_assign_jump_func (info, &functions[num],
- stmt, arg);
+ call, stmt, arg);
else if (gimple_code (stmt) == GIMPLE_PHI)
compute_complex_ancestor_jump_func (info, &functions[num],
- stmt);
+ call, stmt);
}
}
else
- compute_known_type_jump_func (arg, &functions[num]);
+ compute_known_type_jump_func (arg, &functions[num], call);
}
}
@@ -1218,6 +1331,7 @@ ipa_analyze_virtual_call_uses (struct cg
struct ipa_node_params *info, gimple call,
tree target)
{
+ struct ipa_jump_func jfunc;
tree obj = OBJ_TYPE_REF_OBJECT (target);
tree var;
int index;
@@ -1241,7 +1355,8 @@ ipa_analyze_virtual_call_uses (struct cg
var = SSA_NAME_VAR (obj);
index = ipa_get_param_decl_index (info, var);
- if (index >= 0)
+ if (index >= 0
+ && !detect_type_change (obj, call, &jfunc))
ipa_note_param_call (node, index, call, true);
}
Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-1.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-1.C
@@ -0,0 +1,71 @@
+/* Verify that ipa-cp correctly detects the dynamic type of an object
+ under construction when doing devirtualization. */
+/* { dg-do run } */
+/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */
+
+extern "C" void abort (void);
+
+class A
+{
+public:
+ int data;
+ A();
+ virtual int foo (int i);
+};
+
+class B : public A
+{
+public:
+ virtual int foo (int i);
+};
+
+class C : public A
+{
+public:
+ virtual int foo (int i);
+};
+
+int A::foo (int i)
+{
+ return i + 1;
+}
+
+int B::foo (int i)
+{
+ return i + 2;
+}
+
+int C::foo (int i)
+{
+ return i + 3;
+}
+
+static int middleman (class A *obj, int i)
+{
+ return obj->foo (i);
+}
+
+int __attribute__ ((noinline,noclone)) get_input(void)
+{
+ return 1;
+}
+
+A::A ()
+{
+ if (middleman (this, get_input ()) != 2)
+ abort ();
+}
+
+static void bah ()
+{
+ class B b;
+}
+
+int main (int argc, char *argv[])
+{
+ int i;
+
+ for (i = 0; i < 10; i++)
+ bah ();
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-2.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-2.C
@@ -0,0 +1,79 @@
+/* Verify that ipa-cp correctly detects the dynamic type of an object
+ under construction when doing devirtualization. */
+/* { dg-do run } */
+/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */
+
+extern "C" void abort (void);
+
+class Distraction
+{
+public:
+ float f;
+ double d;
+ Distraction ()
+ {
+ f = 8.3;
+ d = 10.2;
+ }
+ virtual float bar (float z);
+};
+
+class A
+{
+public:
+ int data;
+ A();
+ virtual int foo (int i);
+};
+
+class B : public Distraction, public A
+{
+public:
+ virtual int foo (int i);
+};
+
+float Distraction::bar (float z)
+{
+ f += z;
+ return f/2;
+}
+
+int A::foo (int i)
+{
+ return i + 1;
+}
+
+int B::foo (int i)
+{
+ return i + 2;
+}
+
+int __attribute__ ((noinline,noclone)) get_input(void)
+{
+ return 1;
+}
+
+static int middleman (class A *obj, int i)
+{
+ return obj->foo (i);
+}
+
+A::A()
+{
+ if (middleman (this, get_input ()) != 2)
+ abort ();
+}
+
+static void bah ()
+{
+ class B b;
+}
+
+int main (int argc, char *argv[])
+{
+ int i;
+
+ for (i = 0; i < 10; i++)
+ bah ();
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-3.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-3.C
@@ -0,0 +1,80 @@
+/* Verify that ipa-cp correctly detects the dynamic type of an object
+ under construction when doing devirtualization. */
+/* { dg-do run } */
+/* { dg-options "-O3 -fno-inline" } */
+
+extern "C" void abort (void);
+
+class Distraction
+{
+public:
+ float f;
+ double d;
+ Distraction ()
+ {
+ f = 8.3;
+ d = 10.2;
+ }
+ virtual float bar (float z);
+};
+
+class A
+{
+public:
+ int data;
+ A();
+ virtual int foo (int i);
+};
+
+class B : public Distraction, public A
+{
+public:
+ virtual int foo (int i);
+};
+
+float Distraction::bar (float z)
+{
+ f += z;
+ return f/2;
+}
+
+int A::foo (int i)
+{
+ return i + 1;
+}
+
+int B::foo (int i)
+{
+ return i + 2;
+}
+
+int __attribute__ ((noinline,noclone)) get_input(void)
+{
+ return 1;
+}
+
+static int __attribute__ ((noinline))
+middleman (class A *obj, int i)
+{
+ return obj->foo (i);
+}
+
+inline __attribute__ ((always_inline)) A::A()
+{
+ if (middleman (this, get_input ()) != 2)
+ abort ();
+}
+
+static void bah ()
+{
+ class B b;
+}
+
+int main (int argc, char *argv[])
+{
+ int i;
+
+ for (i = 0; i < 10; i++)
+ bah ();
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-4.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-4.C
@@ -0,0 +1,110 @@
+/* Verify that ipa-cp correctly detects the dynamic type of an object
+ under construction when doing devirtualization. */
+/* { dg-do run } */
+/* { dg-options "-O3 -fno-inline" } */
+
+extern "C" void abort (void);
+
+class Distraction
+{
+public:
+ float f;
+ double d;
+ Distraction ()
+ {
+ f = 8.3;
+ d = 10.2;
+ }
+ virtual float bar (float z);
+};
+
+class A
+{
+public:
+ int data;
+ A();
+ virtual int foo (int i);
+};
+
+class B : public Distraction, public A
+{
+public:
+ B();
+ virtual int foo (int i);
+};
+
+class C : public B
+{
+public:
+ virtual int foo (int i);
+};
+
+
+float Distraction::bar (float z)
+{
+ f += z;
+ return f/2;
+}
+
+int A::foo (int i)
+{
+ return i + 1;
+}
+
+int B::foo (int i)
+{
+ return i + 2;
+}
+
+int C::foo (int i)
+{
+ return i + 3;
+}
+
+int __attribute__ ((noinline,noclone)) get_input(void)
+{
+ return 1;
+}
+
+static int __attribute__ ((noinline))
+middleman (class A *obj, int i)
+{
+ return obj->foo (i);
+}
+
+static void __attribute__ ((noinline))
+sth2 (A *a)
+{
+ if (a->foo (get_input ()) != 3)
+ abort ();
+}
+
+inline void __attribute__ ((always_inline)) sth1 (B *b)
+{
+ sth2 (b);
+}
+
+inline __attribute__ ((always_inline)) A::A()
+{
+ if (middleman (this, get_input ()) != 2)
+ abort ();
+}
+
+B::B() : Distraction(), A()
+{
+ sth1 (this);
+}
+
+static void bah ()
+{
+ class C c;
+}
+
+int main (int argc, char *argv[])
+{
+ int i;
+
+ for (i = 0; i < 10; i++)
+ bah ();
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/devirt-d-1.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/devirt-d-1.C
@@ -0,0 +1,71 @@
+/* Verify that ipa-cp correctly detects the dynamic type of an object
+ under destruction when doing devirtualization. */
+/* { dg-do run } */
+/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */
+
+extern "C" void abort (void);
+
+class A
+{
+public:
+ int data;
+ ~A();
+ virtual int foo (int i);
+};
+
+class B : public A
+{
+public:
+ virtual int foo (int i);
+};
+
+class C : public A
+{
+public:
+ virtual int foo (int i);
+};
+
+int A::foo (int i)
+{
+ return i + 1;
+}
+
+int B::foo (int i)
+{
+ return i + 2;
+}
+
+int C::foo (int i)
+{
+ return i + 3;
+}
+
+static int middleman (class A *obj, int i)
+{
+ return obj->foo (i);
+}
+
+int __attribute__ ((noinline,noclone)) get_input(void)
+{
+ return 1;
+}
+
+A::~A ()
+{
+ if (middleman (this, get_input ()) != 2)
+ abort ();
+}
+
+static void bah ()
+{
+ class B b;
+}
+
+int main (int argc, char *argv[])
+{
+ int i;
+
+ for (i = 0; i < 10; i++)
+ bah ();
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/torture/pr45934.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/torture/pr45934.C
@@ -0,0 +1,23 @@
+/* { dg-do run } */
+
+extern "C" void abort ();
+
+struct B *b;
+
+struct B
+{
+ virtual void f () { }
+ ~B() { b->f(); }
+};
+
+struct D : public B
+{
+ virtual void f () { abort (); }
+};
+
+int main ()
+{
+ D d;
+ b = &d;
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-5.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-5.C
@@ -0,0 +1,79 @@
+/* Verify that ipa-cp correctly detects the dynamic type of an object
+ under construction when doing devirtualization. */
+/* { dg-do run } */
+/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */
+
+extern "C" void abort (void);
+
+class B;
+
+class A
+{
+public:
+ int data;
+ A();
+ A(B *b);
+ virtual int foo (int i);
+};
+
+class B : public A
+{
+public:
+ virtual int foo (int i);
+};
+
+class C : public A
+{
+public:
+ virtual int foo (int i);
+};
+
+int A::foo (int i)
+{
+ return i + 1;
+}
+
+int B::foo (int i)
+{
+ return i + 2;
+}
+
+int C::foo (int i)
+{
+ return i + 3;
+}
+
+static int middleman (class A *obj, int i)
+{
+ return obj->foo (i);
+}
+
+int __attribute__ ((noinline,noclone)) get_input(void)
+{
+ return 1;
+}
+
+A::A ()
+{
+}
+
+A::A (B *b)
+{
+ if (middleman (b, get_input ()) != 3)
+ abort ();
+}
+
+static void bah ()
+{
+ B b;
+ A a(&b);
+}
+
+int main (int argc, char *argv[])
+{
+ int i;
+
+ for (i = 0; i < 10; i++)
+ bah ();
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-6.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-6.C
@@ -0,0 +1,72 @@
+/* Verify that ipa-cp correctly detects the dynamic type of an object
+ under construction when doing devirtualization. */
+/* { dg-do run } */
+/* { dg-options "-O3 -fno-inline" } */
+
+extern "C" void abort (void);
+
+class A
+{
+public:
+ int data;
+ A();
+ virtual int foo (int i);
+};
+
+class B : public A
+{
+public:
+ virtual int foo (int i);
+};
+
+class C : public A
+{
+public:
+ virtual int foo (int i);
+};
+
+int A::foo (int i)
+{
+ return i + 1;
+}
+
+int B::foo (int i)
+{
+ return i + 2;
+}
+
+int C::foo (int i)
+{
+ return i + 3;
+}
+
+static inline int __attribute__ ((always_inline))
+middleman (class A *obj, int i)
+{
+ return obj->foo (i);
+}
+
+int __attribute__ ((noinline,noclone)) get_input(void)
+{
+ return 1;
+}
+
+__attribute__ ((noinline)) A::A ()
+{
+ if (middleman (this, get_input ()) != 2)
+ abort ();
+}
+
+static void bah ()
+{
+ class B b;
+}
+
+int main (int argc, char *argv[])
+{
+ int i;
+
+ for (i = 0; i < 10; i++)
+ bah ();
+ return 0;
+}
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH, PR 45934 2/6] Remove devirtualizations that cannot be done
2010-12-01 20:22 [PATCH, PR 45934 0/6] Devirtualization aware of dynamic type changes Martin Jambor
` (4 preceding siblings ...)
2010-12-01 20:23 ` [PATCH, PR 45934 5/6] Identify the new dynamic type after change Martin Jambor
@ 2010-12-01 20:23 ` Martin Jambor
2010-12-01 20:35 ` Richard Guenther
5 siblings, 1 reply; 29+ messages in thread
From: Martin Jambor @ 2010-12-01 20:23 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Guenther, Jan Hubicka
[-- Attachment #1: remove_impossible_devirt.diff --]
[-- Type: text/plain, Size: 17149 bytes --]
This patch removes parts of the current devirtualization machinery
that cannot work because of potential dynamic type change which might
be under way during sub-object construction and destruction. In the
general case we simply cannot know this about global objects and we do
not want to proceed with the necessary detection when folding. So
devirtualization based on global decls, global IPA-CP constants is
being removed and devirtualization in folding is dumbed down to never
work on sub-objects.
This patch removes the two testcases that test devirtualization based
on global variables. After this patch, g++.dg/otr-fold-[12].C,
g++.dg/tree-ssa/pr43411.C and g++.dg/tree-ssa/pr45605.C fail, this is
addressed by the very last patch in the series.
Bootstrapped and tested separately on x86-64-linux, I think I also did
make check-c++ on i686 but I am no longer really sure, it certainly
passed along with the rest.
Thanks,
Martin
2010-11-30 Martin Jambor <mjambor@suse.cz>
PR tree-optimization/45934
PR tree-optimization/46302
* gimple-fold.c (get_base_binfo_for_type): Removed.
(gimple_get_relevant_ref_binfo): Likewise.
(gimple_fold_obj_type_ref_call): Dumb down to 4.5 functionality,
removed parameter inplace, updated the caller.
* gimple.h (gimple_get_relevant_ref_binfo): Remove declaration.
* ipa-cp.c (ipcp_propagate_types): Do not derive types from constants.
(ipcp_discover_new_direct_edges): Do not do devirtualization based on
constants.
* ipa-prop.c (mem_ref_offset): New variable base to store the result
of get_ref_base_and_extent.
(compute_known_type_jump_func): Use get_ref_base_and_extent and
get_binfo_at_offset instead of gimple_get_relevant_ref_binfo.
(ipa_analyze_node): Push and pop cfun, set current_function_decl.
(update_jump_functions_after_inlining): Do not derive types from
constants.
(try_make_edge_direct_virtual_call): Likewise.
* tree.c (get_binfo_at_offset): Get type from non-artificial fields.
* testsuite/g++.dg/ipa/ipcp-ivi-1.C: Removed.
* testsuite/g++.dg/ipa/ivinline-6.C: Likewise.
Index: icln/gcc/gimple-fold.c
===================================================================
--- icln.orig/gcc/gimple-fold.c
+++ icln/gcc/gimple-fold.c
@@ -1360,88 +1360,6 @@ gimple_fold_builtin (gimple stmt)
return result;
}
-/* Search for a base binfo of BINFO that corresponds to TYPE and return it if
- it is found or NULL_TREE if it is not. */
-
-static tree
-get_base_binfo_for_type (tree binfo, tree type)
-{
- int i;
- tree base_binfo;
- tree res = NULL_TREE;
-
- for (i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
- if (TREE_TYPE (base_binfo) == type)
- {
- gcc_assert (!res);
- res = base_binfo;
- }
-
- return res;
-}
-
-/* Return a binfo describing the part of object referenced by expression REF.
- Return NULL_TREE if it cannot be determined. REF can consist of a series of
- COMPONENT_REFs of a declaration or of an INDIRECT_REF or it can also be just
- a simple declaration, indirect reference or an SSA_NAME. If the function
- discovers an INDIRECT_REF or an SSA_NAME, it will assume that the
- encapsulating type is described by KNOWN_BINFO, if it is not NULL_TREE.
- Otherwise the first non-artificial field declaration or the base declaration
- will be examined to get the encapsulating type. */
-
-tree
-gimple_get_relevant_ref_binfo (tree ref, tree known_binfo)
-{
- while (true)
- {
- if (TREE_CODE (ref) == COMPONENT_REF)
- {
- tree par_type;
- tree binfo;
- tree field = TREE_OPERAND (ref, 1);
-
- if (!DECL_ARTIFICIAL (field))
- {
- tree type = TREE_TYPE (field);
- if (TREE_CODE (type) == RECORD_TYPE)
- return TYPE_BINFO (type);
- else
- return NULL_TREE;
- }
-
- par_type = TREE_TYPE (TREE_OPERAND (ref, 0));
- binfo = TYPE_BINFO (par_type);
- if (!binfo
- || BINFO_N_BASE_BINFOS (binfo) == 0)
- return NULL_TREE;
-
- /* Offset 0 indicates the primary base, whose vtable contents are
- represented in the binfo for the derived class. */
- if (int_bit_position (field) != 0)
- {
- tree d_binfo;
-
- /* Get descendant binfo. */
- d_binfo = gimple_get_relevant_ref_binfo (TREE_OPERAND (ref, 0),
- known_binfo);
- if (!d_binfo)
- return NULL_TREE;
- return get_base_binfo_for_type (d_binfo, TREE_TYPE (field));
- }
-
- ref = TREE_OPERAND (ref, 0);
- }
- else if (DECL_P (ref) && TREE_CODE (TREE_TYPE (ref)) == RECORD_TYPE)
- return TYPE_BINFO (TREE_TYPE (ref));
- else if (known_binfo
- && (TREE_CODE (ref) == SSA_NAME
- || TREE_CODE (ref) == MEM_REF))
- return known_binfo;
- else
- return NULL_TREE;
- }
-}
-
/* Return a declaration of a function which an OBJ_TYPE_REF references. TOKEN
is integer form of OBJ_TYPE_REF_TOKEN of the reference expression.
KNOWN_BINFO carries the binfo describing the true type of
@@ -1525,7 +1443,7 @@ gimple_adjust_this_by_delta (gimple_stmt
INPLACE is false. Return true iff the statement was changed. */
static bool
-gimple_fold_obj_type_ref_call (gimple_stmt_iterator *gsi, bool inplace)
+gimple_fold_obj_type_ref_call (gimple_stmt_iterator *gsi)
{
gimple stmt = gsi_stmt (*gsi);
tree ref = gimple_call_fn (stmt);
@@ -1533,27 +1451,21 @@ gimple_fold_obj_type_ref_call (gimple_st
tree binfo, fndecl, delta;
HOST_WIDE_INT token;
- if (TREE_CODE (obj) == ADDR_EXPR)
- obj = TREE_OPERAND (obj, 0);
- else
+ if (TREE_CODE (obj) != ADDR_EXPR)
return false;
-
- binfo = gimple_get_relevant_ref_binfo (obj, NULL_TREE);
+ obj = TREE_OPERAND (obj, 0);
+ if (!DECL_P (obj)
+ || TREE_CODE (TREE_TYPE (obj)) != RECORD_TYPE)
+ return false;
+ binfo = TYPE_BINFO (TREE_TYPE (obj));
if (!binfo)
return false;
+
token = tree_low_cst (OBJ_TYPE_REF_TOKEN (ref), 1);
- fndecl = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta,
- !DECL_P (obj));
+ fndecl = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta, false);
if (!fndecl)
return false;
-
- if (integer_nonzerop (delta))
- {
- if (inplace)
- return false;
- gimple_adjust_this_by_delta (gsi, delta);
- }
-
+ gcc_assert (integer_zerop (delta));
gimple_call_set_fndecl (stmt, fndecl);
return true;
}
@@ -1591,7 +1503,7 @@ gimple_fold_call (gimple_stmt_iterator *
here where we can just smash the call operand. */
callee = gimple_call_fn (stmt);
if (TREE_CODE (callee) == OBJ_TYPE_REF)
- return gimple_fold_obj_type_ref_call (gsi, inplace);
+ return gimple_fold_obj_type_ref_call (gsi);
}
return false;
Index: icln/gcc/gimple.h
===================================================================
--- icln.orig/gcc/gimple.h
+++ icln/gcc/gimple.h
@@ -892,7 +892,6 @@ unsigned get_gimple_rhs_num_ops (enum tr
gimple gimple_alloc_stat (enum gimple_code, unsigned MEM_STAT_DECL);
const char *gimple_decl_printable_name (tree, int);
bool gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace);
-tree gimple_get_relevant_ref_binfo (tree ref, tree known_binfo);
tree gimple_get_virt_mehtod_for_binfo (HOST_WIDE_INT, tree, tree *, bool);
void gimple_adjust_this_by_delta (gimple_stmt_iterator *, tree);
/* Returns true iff T is a valid GIMPLE statement. */
Index: icln/gcc/ipa-cp.c
===================================================================
--- icln.orig/gcc/ipa-cp.c
+++ icln/gcc/ipa-cp.c
@@ -781,26 +781,16 @@ ipcp_propagate_types (struct ipa_node_pa
struct ipa_node_params *callee_info,
struct ipa_jump_func *jf, int i)
{
- tree cst, binfo;
-
switch (jf->type)
{
case IPA_JF_UNKNOWN:
case IPA_JF_CONST_MEMBER_PTR:
+ case IPA_JF_CONST:
break;
case IPA_JF_KNOWN_TYPE:
return ipcp_add_param_type (callee_info, i, jf->value.base_binfo);
- case IPA_JF_CONST:
- cst = jf->value.constant;
- if (TREE_CODE (cst) != ADDR_EXPR)
- break;
- binfo = gimple_get_relevant_ref_binfo (TREE_OPERAND (cst, 0), NULL_TREE);
- if (!binfo)
- break;
- return ipcp_add_param_type (callee_info, i, binfo);
-
case IPA_JF_PASS_THROUGH:
case IPA_JF_ANCESTOR:
return ipcp_copy_types (caller_info, callee_info, i, jf);
@@ -1292,35 +1282,13 @@ ipcp_discover_new_direct_edges (struct c
for (ie = node->indirect_calls; ie; ie = next_ie)
{
struct cgraph_indirect_call_info *ici = ie->indirect_info;
- tree target, delta = NULL_TREE;
next_ie = ie->next_callee;
- if (ici->param_index != index)
+ if (ici->param_index != index
+ || ici->polymorphic)
continue;
- if (ici->polymorphic)
- {
- tree binfo;
- HOST_WIDE_INT token;
-
- if (TREE_CODE (cst) != ADDR_EXPR)
- continue;
-
- binfo = gimple_get_relevant_ref_binfo (TREE_OPERAND (cst, 0),
- NULL_TREE);
- if (!binfo)
- continue;
- gcc_assert (ie->indirect_info->anc_offset == 0);
- token = ie->indirect_info->otr_token;
- target = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta,
- true);
- if (!target)
- continue;
- }
- else
- target = cst;
-
- ipa_make_edge_direct_to_target (ie, target, delta);
+ ipa_make_edge_direct_to_target (ie, cst, NULL_TREE);
}
}
Index: icln/gcc/ipa-prop.c
===================================================================
--- icln.orig/gcc/ipa-prop.c
+++ icln/gcc/ipa-prop.c
@@ -362,7 +362,7 @@ compute_complex_assign_jump_func (struct
gimple stmt, tree name)
{
HOST_WIDE_INT offset, size, max_size;
- tree op1, op2, type;
+ tree op1, op2, base, type;
int index;
op1 = gimple_assign_rhs1 (stmt);
@@ -404,20 +404,21 @@ compute_complex_assign_jump_func (struct
type = TREE_TYPE (op1);
if (TREE_CODE (type) != RECORD_TYPE)
return;
- op1 = get_ref_base_and_extent (op1, &offset, &size, &max_size);
- if (TREE_CODE (op1) != MEM_REF
+ base = get_ref_base_and_extent (op1, &offset, &size, &max_size);
+ if (TREE_CODE (base) != MEM_REF
/* If this is a varying address, punt. */
|| max_size == -1
|| max_size != size)
return;
- offset += mem_ref_offset (op1).low * BITS_PER_UNIT;
- op1 = TREE_OPERAND (op1, 0);
- if (TREE_CODE (op1) != SSA_NAME
- || !SSA_NAME_IS_DEFAULT_DEF (op1)
+ offset += mem_ref_offset (base).low * BITS_PER_UNIT;
+ base = TREE_OPERAND (base, 0);
+ if (TREE_CODE (base) != SSA_NAME
+ || !SSA_NAME_IS_DEFAULT_DEF (base)
|| offset < 0)
return;
- index = ipa_get_param_decl_index (info, SSA_NAME_VAR (op1));
+ /* Dynamic types are changed only in constructors and destructors and */
+ index = ipa_get_param_decl_index (info, SSA_NAME_VAR (base));
if (index >= 0)
{
jfunc->type = IPA_JF_ANCESTOR;
@@ -530,13 +531,26 @@ compute_complex_ancestor_jump_func (stru
static void
compute_known_type_jump_func (tree op, struct ipa_jump_func *jfunc)
{
- tree binfo;
+ HOST_WIDE_INT offset, size, max_size;
+ tree base, binfo;
- if (TREE_CODE (op) != ADDR_EXPR)
+ if (TREE_CODE (op) != ADDR_EXPR
+ || TREE_CODE (TREE_TYPE (TREE_TYPE (op))) != RECORD_TYPE)
return;
op = TREE_OPERAND (op, 0);
- binfo = gimple_get_relevant_ref_binfo (op, NULL_TREE);
+ base = get_ref_base_and_extent (op, &offset, &size, &max_size);
+ if (!DECL_P (base)
+ || max_size == -1
+ || max_size != size
+ || TREE_CODE (TREE_TYPE (base)) != RECORD_TYPE
+ || is_global_var (base))
+ return;
+
+ binfo = TYPE_BINFO (TREE_TYPE (base));
+ if (!binfo)
+ return;
+ binfo = get_binfo_at_offset (binfo, offset, TREE_TYPE (op));
if (binfo)
{
jfunc->type = IPA_JF_KNOWN_TYPE;
@@ -1346,6 +1360,9 @@ ipa_analyze_node (struct cgraph_node *no
struct param_analysis_info *parms_info;
int i, param_count;
+ push_cfun (DECL_STRUCT_FUNCTION (node->decl));
+ current_function_decl = node->decl;
+
ipa_initialize_node_params (node);
param_count = ipa_get_param_count (info);
@@ -1358,6 +1375,9 @@ ipa_analyze_node (struct cgraph_node *no
for (i = 0; i < param_count; i++)
if (parms_info[i].visited_statements)
BITMAP_FREE (parms_info[i].visited_statements);
+
+ current_function_decl = NULL;
+ pop_cfun ();
}
@@ -1416,17 +1436,6 @@ update_jump_functions_after_inlining (st
src = ipa_get_ith_jump_func (top, dst->value.ancestor.formal_id);
if (src->type == IPA_JF_KNOWN_TYPE)
combine_known_type_and_ancestor_jfs (src, dst);
- else if (src->type == IPA_JF_CONST)
- {
- struct ipa_jump_func kt_func;
-
- kt_func.type = IPA_JF_UNKNOWN;
- compute_known_type_jump_func (src->value.constant, &kt_func);
- if (kt_func.type == IPA_JF_KNOWN_TYPE)
- combine_known_type_and_ancestor_jfs (&kt_func, dst);
- else
- dst->type = IPA_JF_UNKNOWN;
- }
else if (src->type == IPA_JF_PASS_THROUGH
&& src->value.pass_through.operation == NOP_EXPR)
dst->value.ancestor.formal_id = src->value.pass_through.formal_id;
@@ -1539,15 +1548,6 @@ try_make_edge_direct_virtual_call (struc
if (jfunc->type == IPA_JF_KNOWN_TYPE)
binfo = jfunc->value.base_binfo;
- else if (jfunc->type == IPA_JF_CONST)
- {
- tree cst = jfunc->value.constant;
- if (TREE_CODE (cst) == ADDR_EXPR)
- binfo = gimple_get_relevant_ref_binfo (TREE_OPERAND (cst, 0),
- NULL_TREE);
- else
- return NULL;
- }
else
return NULL;
Index: icln/gcc/tree.c
===================================================================
--- icln.orig/gcc/tree.c
+++ icln/gcc/tree.c
@@ -10950,8 +10950,7 @@ get_binfo_at_offset (tree binfo, HOST_WI
if (type == expected_type)
return binfo;
- if (TREE_CODE (type) != RECORD_TYPE
- || offset < 0)
+ if (offset < 0)
return NULL_TREE;
for (fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
@@ -10964,12 +10963,18 @@ get_binfo_at_offset (tree binfo, HOST_WI
if (pos <= offset && (pos + size) > offset)
break;
}
- if (!fld || !DECL_ARTIFICIAL (fld))
+ if (!fld || TREE_CODE (TREE_TYPE (fld)) != RECORD_TYPE)
return NULL_TREE;
+ if (!DECL_ARTIFICIAL (fld))
+ {
+ binfo = TYPE_BINFO (TREE_TYPE (fld));
+ if (!binfo)
+ return NULL_TREE;
+ }
/* Offset 0 indicates the primary base, whose vtable contents are
represented in the binfo for the derived class. */
- if (offset != 0)
+ else if (offset != 0)
{
tree base_binfo, found_binfo = NULL_TREE;
for (i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
Index: icln/gcc/testsuite/g++.dg/ipa/ipcp-ivi-1.C
===================================================================
--- icln.orig/gcc/testsuite/g++.dg/ipa/ipcp-ivi-1.C
+++ /dev/null
@@ -1,65 +0,0 @@
-/* Verify that simple virtual calls are inlined even without early
- inlining. */
-/* { dg-do run } */
-/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining" } */
-
-extern "C" void abort (void);
-
-class A
-{
-public:
- int data;
- virtual int foo (int i);
-};
-
-class B : public A
-{
-public:
- virtual int foo (int i);
-};
-
-class C : public A
-{
-public:
- virtual int foo (int i);
-};
-
-int A::foo (int i)
-{
- return i + 1;
-}
-
-int B::foo (int i)
-{
- return i + 2;
-}
-
-int C::foo (int i)
-{
- return i + 3;
-}
-
-int __attribute__ ((noinline)) middleman (class A *obj, int i)
-{
- return obj->foo (i);
-}
-
-int __attribute__ ((noinline,noclone)) get_input(void)
-{
- return 1;
-}
-
-class B b;
-
-int main (int argc, char *argv[])
-{
- int i;
-
- for (i = 0; i < get_input (); i++)
- if (middleman (&b, get_input ()) != 3)
- abort ();
- return 0;
-}
-
-/* { dg-final { scan-ipa-dump "B::foo\[^\\n\]*inline copy in int.*middleman" "inline" } } */
-/* { dg-final { cleanup-ipa-dump "inline" } } */
Index: icln/gcc/testsuite/g++.dg/ipa/ivinline-6.C
===================================================================
--- icln.orig/gcc/testsuite/g++.dg/ipa/ivinline-6.C
+++ /dev/null
@@ -1,58 +0,0 @@
-/* Verify that virtual call inlining works also when it has to get the
- type from an ipa invariant and that even in this case it does not
- pick a wrong method when there is a user defined ancestor in an
- object. */
-/* { dg-do run } */
-/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining -fno-ipa-cp" } */
-
-extern "C" void abort (void);
-
-class A
-{
-public:
- int data;
- virtual int foo (int i);
-};
-
-class B : public A
-{
-public:
- class A confusion;
- virtual int foo (int i);
-};
-
-int A::foo (int i)
-{
- return i + 1;
-}
-
-int B::foo (int i)
-{
- return i + 2;
-}
-
-int middleman (class A *obj, int i)
-{
- return obj->foo (i);
-}
-
-int __attribute__ ((noinline,noclone)) get_input(void)
-{
- return 1;
-}
-
-class B b;
-
-int main (int argc, char *argv[])
-{
- int i, j = get_input ();
-
- for (i = 0; i < j; i++)
- if ((middleman (&b, j) + 100 * middleman (&b.confusion, j)) != 203)
- abort ();
- return 0;
-}
-
-/* { dg-final { scan-ipa-dump "A::foo\[^\\n\]*inline copy in int main" "inline" } } */
-/* { dg-final { scan-ipa-dump "B::foo\[^\\n\]*inline copy in int main" "inline" } } */
-/* { dg-final { cleanup-ipa-dump "inline" } } */
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH, PR 45934 6/6] Intraprocedural type-based devirtualization
2010-12-01 20:22 [PATCH, PR 45934 0/6] Devirtualization aware of dynamic type changes Martin Jambor
2010-12-01 20:22 ` [PATCH, PR 45934 3/6] More robust compute_complex_ancestor_jump_func Martin Jambor
2010-12-01 20:23 ` [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks Martin Jambor
@ 2010-12-01 20:23 ` Martin Jambor
2010-12-02 15:40 ` Richard Guenther
2010-12-01 20:23 ` [PATCH, PR 45934 4/6] Dynamic type change detection Martin Jambor
` (2 subsequent siblings)
5 siblings, 1 reply; 29+ messages in thread
From: Martin Jambor @ 2010-12-01 20:23 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Guenther, Jan Hubicka
[-- Attachment #1: direct_devirt_v2.diff --]
[-- Type: text/plain, Size: 19591 bytes --]
This patch is in some ways still in the RFC shape because it certainly
depends very much on how the previous ones will look like in the end
and I'd like to discuss where this functionality should be placed. On
the other hand, putting it into the cgraph rebuilding pass seems the
best solution so far to me. It certainly looks better then when I
tried to cram it into ipa-prop analysis phase, if not for any other
reasons then because cfg cleanup might be necessary.
Another reason is that I still want to try some more test-cases.
However, I did not want to wait with the whole series any longer and
so I am sending it out with the rest now.
This patch compensates for not devirtualizing when folding. It looks
for OBJ_TYPE_REF calls, and tries to deduce their type by watching out
for dynamic type changes. If the type is known and the call can be
converted to a direct one with that knowledge, it is done so.
This patch is necessary for (modified variants of) testcases
g++.dg/otr-fold-[12].C, g++.dg/tree-ssa/pr43411.C and
g++.dg/tree-ssa/pr45605.C to pass again. Without it, early inlining
can be detrimental to devirtualization by making it a purely
intraprocedural issue and slow down simple testcases three-fold.
Despite being an RFC patch, it also passes bootstrap and testing on
x86_64-linux and make check-c++ on i686. I'm looking forward to all
comments and suggestions.
Thanks,
Martin
2010-12-01 Martin Jambor <mjambor@suse.cz>
* ipa-prop.c (get_ancestor_base_and_offset): New function.
(compute_complex_assign_jump_func): Use it, check that op2 is NULL
in the ancestor case.
(compute_complex_ancestor_jump_func): Likewise.
(ipa_try_devirtualize_immediately): New fuction.
* cgraphbuild.c (rebuild_cgraph_edges): Renamed to
rebuild_cgraph_edges_1, new parameter devirtualize, call
ipa_try_devirtualize_immediately if it is true.
(rebuild_cgraph_edges): New function.
(rebuild_cgraph_edges_devirtualize): Likewise.
(pass_rebuild_cgraph_edges_and_devirt): New variable.
* passes.c (init_optimization_passes): Use it.
* tree-pass.h (pass_rebuild_cgraph_edges_and_devirt): Declare.
* ipa-prop.h (ipa_try_devirtualize_immediately): Declare.
* testsuite/g++.dg/otr-fold-1.C: Moved to...
* testsuite/g++.dg/ipa/imm-devirt-1.C: ...here, compiling with -O2.
* testsuite/g++.dg/otr-fold-2.C: Moved to...
* testsuite/g++.dg/ipa/imm-devirt-2.C: ...here, compiling with -O2.
* testsuite/g++.dg/tree-ssa/pr45605.C: Compile with O2, scan optimized
dump.
Index: icln/gcc/ipa-prop.c
===================================================================
--- icln.orig/gcc/ipa-prop.c
+++ icln/gcc/ipa-prop.c
@@ -539,6 +539,24 @@ detect_type_change (tree arg, gimple cal
return detect_type_change_anc (arg, call, jfunc, 0);
}
+/* !!! Doc */
+
+static tree
+get_ancestor_base_and_offset (tree op, HOST_WIDE_INT *offset)
+{
+ HOST_WIDE_INT size, max_size;
+ tree base;
+
+ base = get_ref_base_and_extent (op, offset, &size, &max_size);
+ if (TREE_CODE (base) != MEM_REF
+ /* If this is a varying address, punt. */
+ || max_size == -1
+ || max_size != size)
+ return NULL_TREE;
+ *offset += mem_ref_offset (base).low * BITS_PER_UNIT;
+ return TREE_OPERAND (base, 0);
+}
+
/* Given that an actual argument is an SSA_NAME (given in NAME) and is a result
of an assignment statement STMT, try to find out whether NAME can be
@@ -551,7 +569,7 @@ compute_complex_assign_jump_func (struct
struct ipa_jump_func *jfunc,
gimple call, gimple stmt, tree name)
{
- HOST_WIDE_INT offset, size, max_size;
+ HOST_WIDE_INT offset;
tree op1, op2, base;
int index;
@@ -588,20 +606,14 @@ compute_complex_assign_jump_func (struct
return;
}
- if (TREE_CODE (op1) != ADDR_EXPR)
+ if (op2 || TREE_CODE (op1) != ADDR_EXPR)
return;
op1 = TREE_OPERAND (op1, 0);
if (TREE_CODE (TREE_TYPE (op1)) != RECORD_TYPE)
return;
- base = get_ref_base_and_extent (op1, &offset, &size, &max_size);
- if (TREE_CODE (base) != MEM_REF
- /* If this is a varying address, punt. */
- || max_size == -1
- || max_size != size)
- return;
- offset += mem_ref_offset (base).low * BITS_PER_UNIT;
- base = TREE_OPERAND (base, 0);
- if (TREE_CODE (base) != SSA_NAME
+ base = get_ancestor_base_and_offset (op1, &offset);
+ if (!base
+ || TREE_CODE (base) != SSA_NAME
|| !SSA_NAME_IS_DEFAULT_DEF (base)
|| offset < 0)
return;
@@ -645,10 +657,10 @@ compute_complex_ancestor_jump_func (stru
struct ipa_jump_func *jfunc,
gimple call, gimple phi)
{
- HOST_WIDE_INT offset, size, max_size;
+ HOST_WIDE_INT offset;
gimple assign, cond;
basic_block phi_bb, assign_bb, cond_bb;
- tree tmp, parm, expr, obj;
+ tree tmp, parm, expr;
int index, i;
if (gimple_phi_num_args (phi) != 2)
@@ -676,17 +688,9 @@ compute_complex_ancestor_jump_func (stru
if (TREE_CODE (expr) != ADDR_EXPR)
return;
expr = TREE_OPERAND (expr, 0);
- obj = expr;
- expr = get_ref_base_and_extent (expr, &offset, &size, &max_size);
-
- if (TREE_CODE (expr) != MEM_REF
- /* If this is a varying address, punt. */
- || max_size == -1
- || max_size != size)
- return;
- offset += mem_ref_offset (expr).low * BITS_PER_UNIT;
- parm = TREE_OPERAND (expr, 0);
- if (TREE_CODE (parm) != SSA_NAME
+ parm = get_ancestor_base_and_offset (expr, &offset);
+ if (!parm
+ || TREE_CODE (parm) != SSA_NAME
|| !SSA_NAME_IS_DEFAULT_DEF (parm)
|| offset < 0)
return;
@@ -712,12 +716,12 @@ compute_complex_ancestor_jump_func (stru
return;
}
- if (!detect_type_change_anc (obj, call, jfunc, offset))
+ if (!detect_type_change_anc (expr, call, jfunc, offset))
{
jfunc->type = IPA_JF_ANCESTOR;
jfunc->value.ancestor.formal_id = index;
jfunc->value.ancestor.offset = offset;
- jfunc->value.ancestor.type = TREE_TYPE (obj);;
+ jfunc->value.ancestor.type = TREE_TYPE (expr);;
}
}
@@ -1407,6 +1411,77 @@ ipa_analyze_indirect_call_uses (struct c
return;
}
+/* If a call to an OBJ_TYPE_REF given by GSI can be turned into a direct one
+ according to the type of its object right away and if so, do it and return
+ true. If CFG is changed in the process, *CFG_CHANGED is set to true. */
+
+tree
+ipa_try_devirtualize_immediately (gimple_stmt_iterator *gsi, bool *cfg_changed)
+{
+ struct ipa_jump_func jfunc;
+ tree obj, fndecl, delta, token;
+ gimple call = gsi_stmt (*gsi);
+ tree target = gimple_call_fn (call);
+
+ if (TREE_CODE (target) != OBJ_TYPE_REF)
+ return NULL_TREE;
+ jfunc.type = IPA_JF_UNKNOWN;
+ obj = OBJ_TYPE_REF_OBJECT (target);
+ if (TREE_CODE (obj) == SSA_NAME)
+ {
+ HOST_WIDE_INT offset;
+ tree op, base;
+ gimple stmt = SSA_NAME_DEF_STMT (obj);
+ if (!stmt || !is_gimple_assign (stmt) || gimple_assign_rhs2 (stmt))
+ return NULL_TREE;
+ op = gimple_assign_rhs1 (stmt);
+ if (TREE_CODE (op) != ADDR_EXPR)
+ return NULL_TREE;
+ op = TREE_OPERAND (op, 0);
+ if (TREE_CODE (TREE_TYPE (op)) != RECORD_TYPE)
+ return NULL_TREE;
+ base = get_ancestor_base_and_offset (op, &offset);
+ if (!base || offset < 0)
+ return NULL_TREE;
+ detect_type_change_anc (op, call, &jfunc, offset);
+ }
+ else
+ compute_known_type_jump_func (obj, &jfunc, call);
+ if (jfunc.type != IPA_JF_KNOWN_TYPE)
+ return NULL_TREE;
+
+ token = OBJ_TYPE_REF_TOKEN (target);
+ fndecl = gimple_get_virt_mehtod_for_binfo (tree_low_cst (token, 1),
+ jfunc.value.base_binfo,
+ &delta, true);
+ if (!fndecl)
+ return NULL_TREE;
+ if (dump_file)
+ {
+ fprintf (dump_file, "ipa-prop: Immediately devirtualizing call ");
+ print_gimple_expr (dump_file, call, 0, 0);
+ }
+
+ if (integer_nonzerop (delta))
+ gimple_adjust_this_by_delta (gsi, delta);
+ gimple_call_set_fndecl (call, fndecl);
+ update_stmt (call);
+ if (maybe_clean_eh_stmt (call)
+ && gimple_purge_dead_eh_edges (gimple_bb (call)))
+ *cfg_changed = true;
+
+ if (dump_file)
+ {
+ fprintf (dump_file, "\n into ");
+ print_gimple_expr (dump_file, call, 0, 0);
+ fprintf (dump_file, "\n with delta ");
+ print_generic_expr (dump_file, delta, 0);
+ fprintf (dump_file, "\n");
+ }
+
+ return fndecl;
+}
+
/* Analyze a CALL to an OBJ_TYPE_REF which is passed in TARGET and if the
object referenced in the expression is a formal parameter of the caller
(described by INFO), create a call note for the statement. */
Index: icln/gcc/testsuite/g++.dg/ipa/imm-devirt-1.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/imm-devirt-1.C
@@ -0,0 +1,76 @@
+/* Verify that virtual calls are folded even when a typecast to an
+ ancestor is involved along the way. */
+/* { dg-do run } */
+/* { dg-options "-O2 -fdump-tree-optimized-slim" } */
+
+extern "C" void abort (void);
+
+class Distraction
+{
+public:
+ float f;
+ double d;
+ Distraction ()
+ {
+ f = 8.3;
+ d = 10.2;
+ }
+ virtual float bar (float z);
+};
+
+class A
+{
+public:
+ int data;
+ virtual int foo (int i);
+};
+
+
+class B : public Distraction, public A
+{
+public:
+ virtual int foo (int i);
+};
+
+float Distraction::bar (float z)
+{
+ f += z;
+ return f/2;
+}
+
+int __attribute__ ((noinline)) A::foo (int i)
+{
+ return i + 1;
+}
+
+int __attribute__ ((noinline)) B::foo (int i)
+{
+ return i + 2;
+}
+
+int __attribute__ ((noinline,noclone)) get_input(void)
+{
+ return 1;
+}
+
+static inline int middleman_1 (class A *obj, int i)
+{
+ return obj->foo (i);
+}
+
+static inline int middleman_2 (class B *obj, int i)
+{
+ return middleman_1 (obj, i);
+}
+
+int main (int argc, char *argv[])
+{
+ class B b;
+
+ if (middleman_2 (&b, get_input ()) != 3)
+ abort ();
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump "= B::.*foo" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: icln/gcc/testsuite/g++.dg/ipa/imm-devirt-2.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/imm-devirt-2.C
@@ -0,0 +1,88 @@
+/* Verify that virtual calls are folded even when a typecast to an
+ ancestor is involved along the way. */
+/* { dg-do run } */
+/* { dg-options "-O2 -fdump-tree-optimized-slim" } */
+
+extern "C" void abort (void);
+
+class Distraction
+{
+public:
+ float f;
+ double d;
+ Distraction ()
+ {
+ f = 8.3;
+ d = 10.2;
+ }
+ virtual float bar (float z);
+};
+
+class A
+{
+public:
+ int data;
+ virtual int foo (int i);
+};
+
+class A_2 : public A
+{
+public:
+ int data_2;
+ virtual int baz (int i);
+};
+
+
+class B : public Distraction, public A_2
+{
+public:
+ virtual int foo (int i);
+};
+
+float __attribute__ ((noinline)) Distraction::bar (float z)
+{
+ f += z;
+ return f/2;
+}
+
+int __attribute__ ((noinline)) A::foo (int i)
+{
+ return i + 1;
+}
+
+int __attribute__ ((noinline)) A_2::baz (int i)
+{
+ return i * 15;
+}
+
+int __attribute__ ((noinline)) B::foo (int i)
+{
+ return i + 2;
+}
+
+int __attribute__ ((noinline,noclone)) get_input(void)
+{
+ return 1;
+}
+
+static inline int middleman_1 (class A *obj, int i)
+{
+ return obj->foo (i);
+}
+
+static inline int middleman_2 (class A *obj, int i)
+{
+ return middleman_1 (obj, i);
+}
+
+int main (int argc, char *argv[])
+{
+ class B b;
+
+ if (middleman_2 (&b, get_input ()) != 3)
+ abort ();
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump "= B::.*foo" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: icln/gcc/testsuite/g++.dg/otr-fold-1.C
===================================================================
--- icln.orig/gcc/testsuite/g++.dg/otr-fold-1.C
+++ /dev/null
@@ -1,76 +0,0 @@
-/* Verify that virtual calls are folded even when a typecast to an
- ancestor is involved along the way. */
-/* { dg-do run } */
-/* { dg-options "-O -fdump-tree-optimized-slim" } */
-
-extern "C" void abort (void);
-
-class Distraction
-{
-public:
- float f;
- double d;
- Distraction ()
- {
- f = 8.3;
- d = 10.2;
- }
- virtual float bar (float z);
-};
-
-class A
-{
-public:
- int data;
- virtual int foo (int i);
-};
-
-
-class B : public Distraction, public A
-{
-public:
- virtual int foo (int i);
-};
-
-float Distraction::bar (float z)
-{
- f += z;
- return f/2;
-}
-
-int A::foo (int i)
-{
- return i + 1;
-}
-
-int B::foo (int i)
-{
- return i + 2;
-}
-
-int __attribute__ ((noinline,noclone)) get_input(void)
-{
- return 1;
-}
-
-static inline int middleman_1 (class A *obj, int i)
-{
- return obj->foo (i);
-}
-
-static inline int middleman_2 (class B *obj, int i)
-{
- return middleman_1 (obj, i);
-}
-
-int main (int argc, char *argv[])
-{
- class B b;
-
- if (middleman_2 (&b, get_input ()) != 3)
- abort ();
- return 0;
-}
-
-/* { dg-final { scan-tree-dump "= B::.*foo" "optimized" } } */
-/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: icln/gcc/testsuite/g++.dg/otr-fold-2.C
===================================================================
--- icln.orig/gcc/testsuite/g++.dg/otr-fold-2.C
+++ /dev/null
@@ -1,88 +0,0 @@
-/* Verify that virtual calls are folded even when a typecast to an
- ancestor is involved along the way. */
-/* { dg-do run } */
-/* { dg-options "-O -fdump-tree-optimized-slim" } */
-
-extern "C" void abort (void);
-
-class Distraction
-{
-public:
- float f;
- double d;
- Distraction ()
- {
- f = 8.3;
- d = 10.2;
- }
- virtual float bar (float z);
-};
-
-class A
-{
-public:
- int data;
- virtual int foo (int i);
-};
-
-class A_2 : public A
-{
-public:
- int data_2;
- virtual int baz (int i);
-};
-
-
-class B : public Distraction, public A_2
-{
-public:
- virtual int foo (int i);
-};
-
-float Distraction::bar (float z)
-{
- f += z;
- return f/2;
-}
-
-int A::foo (int i)
-{
- return i + 1;
-}
-
-int A_2::baz (int i)
-{
- return i * 15;
-}
-
-int B::foo (int i)
-{
- return i + 2;
-}
-
-int __attribute__ ((noinline,noclone)) get_input(void)
-{
- return 1;
-}
-
-static inline int middleman_1 (class A *obj, int i)
-{
- return obj->foo (i);
-}
-
-static inline int middleman_2 (class A *obj, int i)
-{
- return middleman_1 (obj, i);
-}
-
-int main (int argc, char *argv[])
-{
- class B b;
-
- if (middleman_2 (&b, get_input ()) != 3)
- abort ();
- return 0;
-}
-
-/* { dg-final { scan-tree-dump "= B::.*foo" "optimized" } } */
-/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: icln/gcc/testsuite/g++.dg/tree-ssa/pr45605.C
===================================================================
--- icln.orig/gcc/testsuite/g++.dg/tree-ssa/pr45605.C
+++ icln/gcc/testsuite/g++.dg/tree-ssa/pr45605.C
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-O1 -fdump-tree-ssa" } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
extern "C" void abort();
bool destructor_called = false;
@@ -33,5 +33,5 @@ int main() {
/* We should devirtualize call to D::Run */
-/* { dg-final { scan-tree-dump-times "D::Run \\(" 1 "ssa"} } */
-/* { dg-final { cleanup-tree-dump "ssa" } } */
+/* { dg-final { scan-tree-dump-times "D::Run \\(" 1 "optimized"} } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: icln/gcc/cgraphbuild.c
===================================================================
--- icln.orig/gcc/cgraphbuild.c
+++ icln/gcc/cgraphbuild.c
@@ -33,6 +33,9 @@ along with GCC; see the file COPYING3.
#include "tree-pass.h"
#include "ipa-utils.h"
#include "except.h"
+#include "ipa-prop.h"
+
+/* !!! Add ipa-prop.h to dependencies in Makefile.in */
/* Context of record_reference. */
struct record_reference_ctx
@@ -429,12 +432,13 @@ record_references_in_initializer (tree d
/* Rebuild cgraph edges for current function node. This needs to be run after
passes that don't update the cgraph. */
-unsigned int
-rebuild_cgraph_edges (void)
+static unsigned int
+rebuild_cgraph_edges_1 (bool devirtualize)
{
basic_block bb;
struct cgraph_node *node = cgraph_node (current_function_decl);
gimple_stmt_iterator gsi;
+ bool cfg_changed = false;
cgraph_node_remove_callees (node);
ipa_remove_all_references (&node->ref_list);
@@ -453,6 +457,8 @@ rebuild_cgraph_edges (void)
int freq = compute_call_stmt_bb_frequency (current_function_decl,
bb);
decl = gimple_call_fndecl (stmt);
+ if (!decl && devirtualize)
+ decl = ipa_try_devirtualize_immediately (&gsi, &cfg_changed);
if (decl)
cgraph_create_edge (node, cgraph_node (decl), stmt,
bb->count, freq,
@@ -474,7 +480,26 @@ rebuild_cgraph_edges (void)
record_eh_tables (node, cfun);
gcc_assert (!node->global.inlined_to);
- return 0;
+ if (cfg_changed)
+ return TODO_cleanup_cfg;
+ else
+ return 0;
+}
+
+/* !!!Doc */
+
+unsigned int
+rebuild_cgraph_edges (void)
+{
+ return rebuild_cgraph_edges_1 (false);
+}
+
+/* !!!Doc */
+
+static unsigned int
+rebuild_cgraph_edges_devirtualize (void)
+{
+ return rebuild_cgraph_edges_1 (true);
}
/* Rebuild cgraph edges for current function node. This needs to be run after
@@ -518,6 +543,25 @@ struct gimple_opt_pass pass_rebuild_cgra
NULL, /* sub */
NULL, /* next */
0, /* static_pass_number */
+ TV_CGRAPH, /* tv_id */
+ PROP_cfg, /* properties_required */
+ 0, /* properties_provided */
+ 0, /* properties_destroyed */
+ 0, /* todo_flags_start */
+ 0, /* todo_flags_finish */
+ }
+};
+
+struct gimple_opt_pass pass_rebuild_cgraph_edges_and_devirt =
+{
+ {
+ GIMPLE_PASS,
+ "*rebuild_cgraph_edges_d", /* name */
+ NULL, /* gate */
+ rebuild_cgraph_edges_devirtualize, /* execute */
+ NULL, /* sub */
+ NULL, /* next */
+ 0, /* static_pass_number */
TV_CGRAPH, /* tv_id */
PROP_cfg, /* properties_required */
0, /* properties_provided */
Index: icln/gcc/ipa-prop.h
===================================================================
--- icln.orig/gcc/ipa-prop.h
+++ icln/gcc/ipa-prop.h
@@ -429,6 +429,9 @@ void ipa_initialize_node_params (struct
bool ipa_propagate_indirect_call_infos (struct cgraph_edge *cs,
VEC (cgraph_edge_p, heap) **new_edges);
+/* Intraprocedural devirtualization. */
+tree ipa_try_devirtualize_immediately (gimple_stmt_iterator *, bool *);
+
/* Indirect edge and binfo processing. */
struct cgraph_edge *ipa_make_edge_direct_to_target (struct cgraph_edge *, tree,
tree);
Index: icln/gcc/passes.c
===================================================================
--- icln.orig/gcc/passes.c
+++ icln/gcc/passes.c
@@ -791,7 +791,7 @@ init_optimization_passes (void)
NEXT_PASS (pass_split_functions);
}
NEXT_PASS (pass_release_ssa_names);
- NEXT_PASS (pass_rebuild_cgraph_edges);
+ NEXT_PASS (pass_rebuild_cgraph_edges_and_devirt);
NEXT_PASS (pass_inline_parameters);
}
NEXT_PASS (pass_ipa_tree_profile);
Index: icln/gcc/tree-pass.h
===================================================================
--- icln.orig/gcc/tree-pass.h
+++ icln/gcc/tree-pass.h
@@ -436,6 +436,7 @@ extern struct gimple_opt_pass pass_uncpr
extern struct gimple_opt_pass pass_return_slot;
extern struct gimple_opt_pass pass_reassoc;
extern struct gimple_opt_pass pass_rebuild_cgraph_edges;
+extern struct gimple_opt_pass pass_rebuild_cgraph_edges_and_devirt;
extern struct gimple_opt_pass pass_remove_cgraph_callee_edges;
extern struct gimple_opt_pass pass_build_cgraph_edges;
extern struct gimple_opt_pass pass_local_pure_const;
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH, PR 45934 5/6] Identify the new dynamic type after change
2010-12-01 20:22 [PATCH, PR 45934 0/6] Devirtualization aware of dynamic type changes Martin Jambor
` (3 preceding siblings ...)
2010-12-01 20:23 ` [PATCH, PR 45934 4/6] Dynamic type change detection Martin Jambor
@ 2010-12-01 20:23 ` Martin Jambor
2010-12-01 20:23 ` [PATCH, PR 45934 2/6] Remove devirtualizations that cannot be done Martin Jambor
5 siblings, 0 replies; 29+ messages in thread
From: Martin Jambor @ 2010-12-01 20:23 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Guenther, Jan Hubicka
[-- Attachment #1: identify_new_dynamic_type.diff --]
[-- Type: text/plain, Size: 8520 bytes --]
This patch adds code attempting to identify of the new dynamic type
after it has been changed, which has been missing in the previous
patch. All the details are in the previous email.
Just for the record, this has been also bootstrapped and tested
(without the last patch applied) on x86-64-linux and has passed make
check-c++ on i686.
Thanks,
Martin
2010-11-30 Martin Jambor <mjambor@suse.cz>
* ipa-prop.c (type_change_info): New fields object and
known_current_type.
(extr_type_from_vtbl_ptr_store): New function.
(check_stmt_for_type_change): Call it.
(detect_type_change_1): Set up new fields in tci, build knonw type
jump functions if the new type chan be identified.
* testsuite/g++.dg/ipa/devirt-c-1.C: Add dump scans.
* testsuite/g++.dg/ipa/devirt-c-2.C: Likewise.
* testsuite/g++.dg/ipa/devirt-c-3.C: Likewise.
* testsuite/g++.dg/ipa/devirt-c-4.C: Likewise.
Index: icln/gcc/ipa-prop.c
===================================================================
--- icln.orig/gcc/ipa-prop.c
+++ icln/gcc/ipa-prop.c
@@ -355,6 +355,12 @@ ipa_print_all_jump_functions (FILE *f)
struct type_change_info
{
+ /* The declaration or SSA_NAME pointer of the base that we are checking for
+ type change. */
+ tree object;
+ /* If we actually can tell the type that the object has changed to, it is
+ stored in this field. Otherwise it remains NULL_TREE. */
+ tree known_current_type;
/* Set to true if dynamic type change has been detected. */
bool type_maybe_changed;
};
@@ -387,6 +393,49 @@ stmt_may_be_vtbl_ptr_store (gimple stmt)
return true;
}
+/* If STMT can be proved to be an assignment to the virtual method table
+ pointer of ANALYZED_OBJ and the type associated witht the new table
+ identified, return the type. Otherwise return NULL_TREE. */
+
+static tree
+extr_type_from_vtbl_ptr_store (gimple stmt, tree analyzed_obj)
+{
+ tree lhs, t, obj;
+
+ if (!is_gimple_assign (stmt))
+ return NULL_TREE;
+
+ lhs = gimple_assign_lhs (stmt);
+
+ if (TREE_CODE (lhs) != COMPONENT_REF)
+ return NULL_TREE;
+ obj = lhs;
+
+ if (!DECL_VIRTUAL_P (TREE_OPERAND (lhs, 1)))
+ return NULL_TREE;
+
+ do
+ {
+ obj = TREE_OPERAND (obj, 0);
+ } while (TREE_CODE (obj) == COMPONENT_REF);
+ if (TREE_CODE (obj) == MEM_REF)
+ obj = TREE_OPERAND (obj, 0);
+ if (TREE_CODE (obj) == ADDR_EXPR)
+ obj = TREE_OPERAND (obj, 0);
+ if (obj != analyzed_obj)
+ return NULL_TREE;
+
+
+ t = gimple_assign_rhs1 (stmt);
+ if (TREE_CODE (t) != ADDR_EXPR)
+ return NULL_TREE;
+ t = get_base_address (TREE_OPERAND (t, 0));
+ if (!t || TREE_CODE (t) != VAR_DECL || !DECL_VIRTUAL_P (t))
+ return NULL_TREE;
+
+ return DECL_CONTEXT (t);
+}
+
/* Callbeck of walk_aliased_vdefs and a helper function for
detect_type_change_anc to check whether a particular statement may modify
the virtual table pointer, and if possible also determine the new type of
@@ -402,6 +451,8 @@ check_stmt_for_type_change (ao_ref *ao A
if (stmt_may_be_vtbl_ptr_store (stmt))
{
tci->type_maybe_changed = true;
+ tci->known_current_type = extr_type_from_vtbl_ptr_store (stmt,
+ tci->object);
return true;
}
else
@@ -417,9 +468,10 @@ check_stmt_for_type_change (ao_ref *ao A
static bool
detect_type_change_anc (tree arg, gimple call, struct ipa_jump_func *jfunc,
- HOST_WIDE_INT anc_offset ATTRIBUTE_UNUSED)
+ HOST_WIDE_INT anc_offset)
{
struct type_change_info tci;
+ tree obj;
ao_ref ar;
/* Const calls cannot call virtual methods through VMT and so type changes do
@@ -427,6 +479,15 @@ detect_type_change_anc (tree arg, gimple
if (!gimple_vuse (call))
return false;
+ obj = arg;
+ while (handled_component_p (obj))
+ obj = TREE_OPERAND (obj, 0);
+ if (TREE_CODE (obj) == MEM_REF)
+ obj = TREE_OPERAND (obj, 0);
+ if (TREE_CODE (obj) == ADDR_EXPR)
+ obj = TREE_OPERAND (obj, 0);
+ tci.object = obj;
+ tci.known_current_type = NULL_TREE;
tci.type_maybe_changed = false;
ao_ref_init (&ar, arg);
@@ -435,7 +496,31 @@ detect_type_change_anc (tree arg, gimple
if (!tci.type_maybe_changed)
return false;
- jfunc->type = IPA_JF_UNKNOWN;
+ if (!tci.known_current_type)
+ jfunc->type = IPA_JF_UNKNOWN;
+ else if (anc_offset != 0)
+ {
+ tree new_binfo = get_binfo_at_offset (TYPE_BINFO (tci.known_current_type),
+ anc_offset, TREE_TYPE (arg));
+
+ if (new_binfo)
+ {
+ jfunc->type = IPA_JF_KNOWN_TYPE;
+ jfunc->value.base_binfo = new_binfo;
+ }
+ else
+ jfunc->type = IPA_JF_UNKNOWN;
+ }
+ else
+ {
+ /* If an offset of a searched-for sub-object actually happens to be zero,
+ we do not have to worry about non-artificialness. Either there are no
+ virtual methods anyway or there is a VMT pointer at offset zero and so
+ artificial sub-objects start at higher offsets. */
+ jfunc->type = IPA_JF_KNOWN_TYPE;
+ jfunc->value.base_binfo = TYPE_BINFO (tci.known_current_type);
+ }
+
return true;
}
Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-1.C
===================================================================
--- icln.orig/gcc/testsuite/g++.dg/ipa/devirt-c-1.C
+++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-1.C
@@ -1,7 +1,7 @@
/* Verify that ipa-cp correctly detects the dynamic type of an object
under construction when doing devirtualization. */
/* { dg-do run } */
-/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */
+/* { dg-options "-O3 -fno-early-inlining -fno-inline -fdump-ipa-cp -fdump-tree-optimized" } */
extern "C" void abort (void);
@@ -69,3 +69,8 @@ int main (int argc, char *argv[])
bah ();
return 0;
}
+
+/* { dg-final { scan-ipa-dump "Discovered a virtual call to a known target.*A::foo" "cp" } } */
+/* { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "optimized"} } */
+/* { dg-final { cleanup-ipa-dump "cp" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-2.C
===================================================================
--- icln.orig/gcc/testsuite/g++.dg/ipa/devirt-c-2.C
+++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-2.C
@@ -1,7 +1,7 @@
/* Verify that ipa-cp correctly detects the dynamic type of an object
under construction when doing devirtualization. */
/* { dg-do run } */
-/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */
+/* { dg-options "-O3 -fno-early-inlining -fno-inline -fdump-ipa-cp -fdump-tree-optimized" } */
extern "C" void abort (void);
@@ -77,3 +77,8 @@ int main (int argc, char *argv[])
bah ();
return 0;
}
+
+/* { dg-final { scan-ipa-dump "Discovered a virtual call to a known target.*A::foo" "cp" } } */
+/* { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "optimized"} } */
+/* { dg-final { cleanup-ipa-dump "cp" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-3.C
===================================================================
--- icln.orig/gcc/testsuite/g++.dg/ipa/devirt-c-3.C
+++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-3.C
@@ -1,7 +1,7 @@
/* Verify that ipa-cp correctly detects the dynamic type of an object
under construction when doing devirtualization. */
/* { dg-do run } */
-/* { dg-options "-O3 -fno-inline" } */
+/* { dg-options "-O3 -fno-inline -fdump-ipa-cp -fdump-tree-optimized" } */
extern "C" void abort (void);
@@ -78,3 +78,8 @@ int main (int argc, char *argv[])
bah ();
return 0;
}
+
+/* { dg-final { scan-ipa-dump "Discovered a virtual call to a known target.*A::foo" "cp" } } */
+/* { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "optimized"} } */
+/* { dg-final { cleanup-ipa-dump "cp" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-4.C
===================================================================
--- icln.orig/gcc/testsuite/g++.dg/ipa/devirt-c-4.C
+++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-4.C
@@ -1,7 +1,7 @@
/* Verify that ipa-cp correctly detects the dynamic type of an object
under construction when doing devirtualization. */
/* { dg-do run } */
-/* { dg-options "-O3 -fno-inline" } */
+/* { dg-options "-O3 -fno-inline -fdump-tree-optimized" } */
extern "C" void abort (void);
@@ -108,3 +108,6 @@ int main (int argc, char *argv[])
bah ();
return 0;
}
+
+/* { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "optimized"} } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks
2010-12-01 20:22 [PATCH, PR 45934 0/6] Devirtualization aware of dynamic type changes Martin Jambor
2010-12-01 20:22 ` [PATCH, PR 45934 3/6] More robust compute_complex_ancestor_jump_func Martin Jambor
@ 2010-12-01 20:23 ` Martin Jambor
2010-12-01 20:58 ` Jan Hubicka
2010-12-17 14:14 ` H.J. Lu
2010-12-01 20:23 ` [PATCH, PR 45934 6/6] Intraprocedural type-based devirtualization Martin Jambor
` (3 subsequent siblings)
5 siblings, 2 replies; 29+ messages in thread
From: Martin Jambor @ 2010-12-01 20:23 UTC (permalink / raw)
To: GCC Patches; +Cc: Richard Guenther, Jan Hubicka
[-- Attachment #1: pr46053.diff --]
[-- Type: text/plain, Size: 28688 bytes --]
Hi,
this is a yet another ping for the patch below. It has already been
approved by Richi but still awaits Honza's assessment.
Re-bootstrapped and re-tested on x86-64-linux this Monday.
Thanks,
Martin
----------------------------------------
Hi,
we currently cannot really represent direct calls to thunks in the
call graph (i.e. in the edges) and this means we can omit the
necessary thunk adjustments when using that edge in IPA
transformations. This then causes problems filed as PR 46053 and PR
46287. In 4.5 there were no such calls so it did not matter but with
devirtualization we currently have them and we either need to handle
them or avoid them. The patch below does both.
Certainly, soon we will need to be able to represent this information
on the edges in the full generality. In particular we will have to
once we start devirtualizing by folding loads from the VMT. However,
dealing with result adjusting and virtual_offset adjusting thunks will
require more changes than I am comfortable to push in stage three,
mainly because they are nested but there will also be other unforseen
problems too.
Therefore, I currently handle the simple this adjustment thunks and
punt when a more complex one is discovered. Calls to this adjusting
thunks are then converted to a POINTER_PLUS in the caller and a call
to the ordinary function declaration. This way, we still don't have
any direct calls to thunks in the IL. These adjustments in callees
are also performed during IPA inline and IPA-CP devirtualizations.
I do not perform the thunk check if the second argument of a folded
OBJ_TYPE_REF is an ADDR_EXPR of a simple declaration (as opposed to a
COMPONENT_REF) so that g++.dg/opt/devirt1.C passes. If I did, the
thunk test would not be possible because there is no call graph node
for the called method (which is not defined in this compilation unit).
In this case there should be no thunks involved ( it is not a call to
an ancestor's virtual method) and it is what previous versions of gcc
do so we should not regress.
Bootstrapped and tested on x86-64-linux without any issues, a very
similar patch has also passed make check-c++ on i686. OK for trunk?
Thanks,
Martin
2010-11-08 Martin Jambor <mjambor@suse.cz>
PR tree-optimization/46053
PR middle-end/46287
* cgraph.h (cgraph_indirect_call_info): New field.
* gimple.h (gimple_fold_obj_type_ref): Declaration removed.
(gimple_fold_call): Declare.
(gimple_adjust_this_by_delta): Likewise.
* cgraph.c (cgraph_make_edge_direct): New parameter delta. Updated
all users.
(cgraph_clone_edge): Create a copy of indirect_info also for direct
edges.
* cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Adjust this
parameters.
* gimple-fold.c (gimple_fold_obj_type_ref_known_binfo): Renamed to
gimple_get_virt_mehtod_for_binfo, new parameter delta. Do not search
through thunks, in fact bail out if we encounter one, check that
BINFO_VIRTUALS is not NULL.
(gimple_adjust_this_by_delta): New function.
(gimple_fold_obj_type_ref): Removed.
(gimple_fold_obj_type_ref_call): New function.
(fold_gimple_call): Renamed to gimple_fold_call, made external.
Updated users. Call gimple_fold_obj_type_ref_call instead of
gimple_fold_obj_type_ref.
* ipa-cp.c (ipcp_process_devirtualization_opportunities): Process
thunk deltas.
(ipcp_discover_new_direct_edges): Likewise.
* ipa-prop.c (ipa_make_edge_direct_to_target): New parameter delta.
Updated callers.
(ipa_write_indirect_edge_info): Stream thunk_delta.
(ipa_read_indirect_edge_info): Likewise.
* tree-ssa-ccp.c (ccp_fold_stmt): Use gimple_fold_call instead of
gimple_fold_obj_type_ref.
* testsuite/g++.dg/ipa/pr46053.C: New test.
* testsuite/g++.dg/ipa/pr46287-1.C: Likewise.
* testsuite/g++.dg/ipa/pr46287-2.C: Likewise.
* testsuite/g++.dg/ipa/pr46287-3.C: Likewise.
* testsuite/g++.dg/torture/covariant-1.C: Likewise.
* testsuite/g++.dg/torture/pr46287.C: Likewise.
Index: icln/gcc/cgraph.c
===================================================================
--- icln.orig/gcc/cgraph.c
+++ icln/gcc/cgraph.c
@@ -859,7 +859,7 @@ cgraph_set_call_stmt (struct cgraph_edge
indirect call into a direct one. */
struct cgraph_node *new_callee = cgraph_node (decl);
- cgraph_make_edge_direct (e, new_callee);
+ cgraph_make_edge_direct (e, new_callee, NULL);
}
push_cfun (DECL_STRUCT_FUNCTION (e->caller->decl));
@@ -1194,12 +1194,15 @@ cgraph_redirect_edge_callee (struct cgra
}
/* Make an indirect EDGE with an unknown callee an ordinary edge leading to
- CALLEE. */
+ CALLEE. DELTA, if non-NULL, is an integer constant that is to be added to
+ the this pointer (first parameter). */
void
-cgraph_make_edge_direct (struct cgraph_edge *edge, struct cgraph_node *callee)
+cgraph_make_edge_direct (struct cgraph_edge *edge, struct cgraph_node *callee,
+ tree delta)
{
edge->indirect_unknown_callee = 0;
+ edge->indirect_info->thunk_delta = delta;
/* Get the edge out of the indirect edge list. */
if (edge->prev_callee)
@@ -2115,8 +2118,16 @@ cgraph_clone_edge (struct cgraph_edge *e
}
}
else
- new_edge = cgraph_create_edge (n, e->callee, call_stmt, count, freq,
- e->loop_nest + loop_nest);
+ {
+ new_edge = cgraph_create_edge (n, e->callee, call_stmt, count, freq,
+ e->loop_nest + loop_nest);
+ if (e->indirect_info)
+ {
+ new_edge->indirect_info
+ = ggc_alloc_cleared_cgraph_indirect_call_info ();
+ *new_edge->indirect_info = *e->indirect_info;
+ }
+ }
new_edge->inline_failed = e->inline_failed;
new_edge->indirect_inlining_edge = e->indirect_inlining_edge;
Index: icln/gcc/cgraph.h
===================================================================
--- icln.orig/gcc/cgraph.h
+++ icln/gcc/cgraph.h
@@ -385,6 +385,9 @@ struct GTY(()) cgraph_indirect_call_info
HOST_WIDE_INT otr_token;
/* Type of the object from OBJ_TYPE_REF_OBJECT. */
tree otr_type;
+ /* Delta by which must be added to this parameter. For polymorphic calls
+ only. */
+ tree thunk_delta;
/* Index of the parameter that is called. */
int param_index;
/* ECF flags determined from the caller. */
@@ -572,7 +575,7 @@ struct cgraph_node * cgraph_clone_node (
int, bool, VEC(cgraph_edge_p,heap) *);
void cgraph_redirect_edge_callee (struct cgraph_edge *, struct cgraph_node *);
-void cgraph_make_edge_direct (struct cgraph_edge *, struct cgraph_node *);
+void cgraph_make_edge_direct (struct cgraph_edge *, struct cgraph_node *, tree);
struct cgraph_asm_node *cgraph_add_asm_node (tree);
Index: icln/gcc/cgraphunit.c
===================================================================
--- icln.orig/gcc/cgraphunit.c
+++ icln/gcc/cgraphunit.c
@@ -2130,6 +2130,8 @@ cgraph_redirect_edge_call_stmt_to_callee
{
tree decl = gimple_call_fndecl (e->call_stmt);
gimple new_stmt;
+ gimple_stmt_iterator gsi;
+ bool gsi_computed = false;
#ifdef ENABLE_CHECKING
struct cgraph_node *node;
#endif
@@ -2162,9 +2164,24 @@ cgraph_redirect_edge_call_stmt_to_callee
}
}
+ if (e->indirect_info && e->indirect_info->thunk_delta
+ && integer_nonzerop (e->indirect_info->thunk_delta))
+ {
+ if (cgraph_dump_file)
+ {
+ fprintf (cgraph_dump_file, " Thunk delta is ");
+ print_generic_expr (cgraph_dump_file,
+ e->indirect_info->thunk_delta, 0);
+ fprintf (cgraph_dump_file, "\n");
+ }
+ gsi = gsi_for_stmt (e->call_stmt);
+ gsi_computed = true;
+ gimple_adjust_this_by_delta (&gsi, e->indirect_info->thunk_delta);
+ e->indirect_info->thunk_delta = NULL_TREE;
+ }
+
if (e->callee->clone.combined_args_to_skip)
{
- gimple_stmt_iterator gsi;
int lp_nr;
new_stmt
@@ -2176,7 +2193,8 @@ cgraph_redirect_edge_call_stmt_to_callee
&& TREE_CODE (gimple_vdef (new_stmt)) == SSA_NAME)
SSA_NAME_DEF_STMT (gimple_vdef (new_stmt)) = new_stmt;
- gsi = gsi_for_stmt (e->call_stmt);
+ if (!gsi_computed)
+ gsi = gsi_for_stmt (e->call_stmt);
gsi_replace (&gsi, new_stmt, false);
/* We need to defer cleaning EH info on the new statement to
fixup-cfg. We may not have dominator information at this point
Index: icln/gcc/gimple-fold.c
===================================================================
--- icln.orig/gcc/gimple-fold.c
+++ icln/gcc/gimple-fold.c
@@ -1442,17 +1442,26 @@ gimple_get_relevant_ref_binfo (tree ref,
}
}
-/* Fold a OBJ_TYPE_REF expression to the address of a function. TOKEN is
- integer form of OBJ_TYPE_REF_TOKEN of the reference expression. KNOWN_BINFO
- carries the binfo describing the true type of OBJ_TYPE_REF_OBJECT(REF). */
+/* Return a declaration of a function which an OBJ_TYPE_REF references. TOKEN
+ is integer form of OBJ_TYPE_REF_TOKEN of the reference expression.
+ KNOWN_BINFO carries the binfo describing the true type of
+ OBJ_TYPE_REF_OBJECT(REF). If a call to the function must be accompanied
+ with a this adjustment, the constant which should be added to this pointer
+ is stored to *DELTA. If REFUSE_THUNKS is true, return NULL if the function
+ is a thunk (other than a this adjustment which is dealt with by DELTA). */
tree
-gimple_fold_obj_type_ref_known_binfo (HOST_WIDE_INT token, tree known_binfo)
+gimple_get_virt_mehtod_for_binfo (HOST_WIDE_INT token, tree known_binfo,
+ tree *delta, bool refuse_thunks)
{
HOST_WIDE_INT i;
- tree v, fndecl, delta;
+ tree v, fndecl;
+ struct cgraph_node *node;
v = BINFO_VIRTUALS (known_binfo);
+ /* If there is no virtual methods leave the OBJ_TYPE_REF alone. */
+ if (!v)
+ return NULL_TREE;
i = 0;
while (i != token)
{
@@ -1462,62 +1471,91 @@ gimple_fold_obj_type_ref_known_binfo (HO
}
fndecl = TREE_VALUE (v);
- delta = TREE_PURPOSE (v);
- gcc_assert (host_integerp (delta, 0));
-
- if (integer_nonzerop (delta))
- {
- struct cgraph_node *node = cgraph_get_node (fndecl);
- HOST_WIDE_INT off = tree_low_cst (delta, 0);
-
- if (!node)
- return NULL;
- for (node = node->same_body; node; node = node->next)
- if (node->thunk.thunk_p && off == node->thunk.fixed_offset)
- break;
- if (node)
- fndecl = node->decl;
- else
- return NULL;
- }
+ node = cgraph_get_node_or_alias (fndecl);
+ if (refuse_thunks
+ && (!node
+ /* Bail out if it is a thunk declaration. Since simple this_adjusting
+ thunks are represented by a constant in TREE_PURPOSE of items in
+ BINFO_VIRTUALS, this is a more complicate type which we cannot handle as
+ yet.
+
+ FIXME: Remove the following condition once we are able to represent
+ thunk information on call graph edges. */
+ || (node->same_body_alias && node->thunk.thunk_p)))
+ return NULL_TREE;
/* When cgraph node is missing and function is not public, we cannot
devirtualize. This can happen in WHOPR when the actual method
ends up in other partition, because we found devirtualization
possibility too late. */
- if (!can_refer_decl_in_current_unit_p (fndecl))
- return NULL;
- return build_fold_addr_expr (fndecl);
+ if (!can_refer_decl_in_current_unit_p (TREE_VALUE (v)))
+ return NULL_TREE;
+
+ *delta = TREE_PURPOSE (v);
+ gcc_checking_assert (host_integerp (*delta, 0));
+ return fndecl;
}
+/* Generate code adjusting the first parameter of a call statement determined
+ by GSI by DELTA. */
-/* Fold a OBJ_TYPE_REF expression to the address of a function. If KNOWN_TYPE
- is not NULL_TREE, it is the true type of the outmost encapsulating object if
- that comes from a pointer SSA_NAME. If the true outmost encapsulating type
- can be determined from a declaration OBJ_TYPE_REF_OBJECT(REF), it is used
- regardless of KNOWN_TYPE (which thus can be NULL_TREE). */
+void
+gimple_adjust_this_by_delta (gimple_stmt_iterator *gsi, tree delta)
+{
+ gimple call_stmt = gsi_stmt (*gsi);
+ tree parm, tmp;
+ gimple new_stmt;
+
+ delta = fold_convert (sizetype, delta);
+ gcc_assert (gimple_call_num_args (call_stmt) >= 1);
+ parm = gimple_call_arg (call_stmt, 0);
+ gcc_assert (POINTER_TYPE_P (TREE_TYPE (parm)));
+ tmp = create_tmp_var (TREE_TYPE (parm), NULL);
+ add_referenced_var (tmp);
+
+ tmp = make_ssa_name (tmp, NULL);
+ new_stmt = gimple_build_assign_with_ops (POINTER_PLUS_EXPR, tmp, parm, delta);
+ SSA_NAME_DEF_STMT (tmp) = new_stmt;
+ gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
+ gimple_call_set_arg (call_stmt, 0, tmp);
+}
-tree
-gimple_fold_obj_type_ref (tree ref, tree known_type)
+/* Fold a call statement to OBJ_TYPE_REF to a direct call, if possible. GSI
+ determines the statement, generating new statements is allowed only if
+ INPLACE is false. Return true iff the statement was changed. */
+
+static bool
+gimple_fold_obj_type_ref_call (gimple_stmt_iterator *gsi, bool inplace)
{
+ gimple stmt = gsi_stmt (*gsi);
+ tree ref = gimple_call_fn (stmt);
tree obj = OBJ_TYPE_REF_OBJECT (ref);
- tree known_binfo = known_type ? TYPE_BINFO (known_type) : NULL_TREE;
- tree binfo;
+ tree binfo, fndecl, delta;
+ HOST_WIDE_INT token;
if (TREE_CODE (obj) == ADDR_EXPR)
obj = TREE_OPERAND (obj, 0);
+ else
+ return false;
+
+ binfo = gimple_get_relevant_ref_binfo (obj, NULL_TREE);
+ if (!binfo)
+ return false;
+ token = tree_low_cst (OBJ_TYPE_REF_TOKEN (ref), 1);
+ fndecl = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta,
+ !DECL_P (obj));
+ if (!fndecl)
+ return false;
- binfo = gimple_get_relevant_ref_binfo (obj, known_binfo);
- if (binfo)
+ if (integer_nonzerop (delta))
{
- HOST_WIDE_INT token = tree_low_cst (OBJ_TYPE_REF_TOKEN (ref), 1);
- /* If there is no virtual methods leave the OBJ_TYPE_REF alone. */
- if (!BINFO_VIRTUALS (binfo))
- return NULL_TREE;
- return gimple_fold_obj_type_ref_known_binfo (token, binfo);
+ if (inplace)
+ return false;
+ gimple_adjust_this_by_delta (gsi, delta);
}
- else
- return NULL_TREE;
+
+ gimple_call_set_fndecl (stmt, fndecl);
+ return true;
}
/* Attempt to fold a call statement referenced by the statement iterator GSI.
@@ -1525,8 +1563,8 @@ gimple_fold_obj_type_ref (tree ref, tree
simplifies to a constant value. Return true if any changes were made.
It is assumed that the operands have been previously folded. */
-static bool
-fold_gimple_call (gimple_stmt_iterator *gsi, bool inplace)
+bool
+gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace)
{
gimple stmt = gsi_stmt (*gsi);
@@ -1552,18 +1590,8 @@ fold_gimple_call (gimple_stmt_iterator *
copying EH region info to the new node. Easier to just do it
here where we can just smash the call operand. */
callee = gimple_call_fn (stmt);
- if (TREE_CODE (callee) == OBJ_TYPE_REF
- && TREE_CODE (OBJ_TYPE_REF_OBJECT (callee)) == ADDR_EXPR)
- {
- tree t;
-
- t = gimple_fold_obj_type_ref (callee, NULL_TREE);
- if (t)
- {
- gimple_call_set_fn (stmt, t);
- return true;
- }
- }
+ if (TREE_CODE (callee) == OBJ_TYPE_REF)
+ return gimple_fold_obj_type_ref_call (gsi, inplace);
}
return false;
@@ -1617,7 +1645,7 @@ fold_stmt_1 (gimple_stmt_iterator *gsi,
changed = true;
}
}
- changed |= fold_gimple_call (gsi, inplace);
+ changed |= gimple_fold_call (gsi, inplace);
break;
case GIMPLE_ASM:
Index: icln/gcc/gimple.h
===================================================================
--- icln.orig/gcc/gimple.h
+++ icln/gcc/gimple.h
@@ -891,10 +891,10 @@ unsigned get_gimple_rhs_num_ops (enum tr
#define gimple_alloc(c, n) gimple_alloc_stat (c, n MEM_STAT_INFO)
gimple gimple_alloc_stat (enum gimple_code, unsigned MEM_STAT_DECL);
const char *gimple_decl_printable_name (tree, int);
-tree gimple_fold_obj_type_ref (tree, tree);
+bool gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace);
tree gimple_get_relevant_ref_binfo (tree ref, tree known_binfo);
-tree gimple_fold_obj_type_ref_known_binfo (HOST_WIDE_INT, tree);
-
+tree gimple_get_virt_mehtod_for_binfo (HOST_WIDE_INT, tree, tree *, bool);
+void gimple_adjust_this_by_delta (gimple_stmt_iterator *, tree);
/* Returns true iff T is a valid GIMPLE statement. */
extern bool is_gimple_stmt (tree);
Index: icln/gcc/ipa-cp.c
===================================================================
--- icln.orig/gcc/ipa-cp.c
+++ icln/gcc/ipa-cp.c
@@ -1214,7 +1214,7 @@ ipcp_process_devirtualization_opportunit
{
int param_index, types_count, j;
HOST_WIDE_INT token;
- tree target;
+ tree target, delta;
next_ie = ie->next_callee;
if (!ie->indirect_info->polymorphic)
@@ -1231,7 +1231,8 @@ ipcp_process_devirtualization_opportunit
for (j = 0; j < types_count; j++)
{
tree binfo = VEC_index (tree, info->params[param_index].types, j);
- tree t = gimple_fold_obj_type_ref_known_binfo (token, binfo);
+ tree d;
+ tree t = gimple_get_virt_mehtod_for_binfo (token, binfo, &d, true);
if (!t)
{
@@ -1239,8 +1240,11 @@ ipcp_process_devirtualization_opportunit
break;
}
else if (!target)
- target = t;
- else if (target != t)
+ {
+ target = t;
+ delta = d;
+ }
+ else if (target != t || !tree_int_cst_equal (delta, d))
{
target = NULL_TREE;
break;
@@ -1248,7 +1252,7 @@ ipcp_process_devirtualization_opportunit
}
if (target)
- ipa_make_edge_direct_to_target (ie, target);
+ ipa_make_edge_direct_to_target (ie, target, delta);
}
}
@@ -1288,6 +1292,7 @@ ipcp_discover_new_direct_edges (struct c
for (ie = node->indirect_calls; ie; ie = next_ie)
{
struct cgraph_indirect_call_info *ici = ie->indirect_info;
+ tree target, delta = NULL_TREE;
next_ie = ie->next_callee;
if (ici->param_index != index)
@@ -1307,12 +1312,15 @@ ipcp_discover_new_direct_edges (struct c
continue;
gcc_assert (ie->indirect_info->anc_offset == 0);
token = ie->indirect_info->otr_token;
- cst = gimple_fold_obj_type_ref_known_binfo (token, binfo);
- if (!cst)
+ target = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta,
+ true);
+ if (!target)
continue;
}
+ else
+ target = cst;
- ipa_make_edge_direct_to_target (ie, cst);
+ ipa_make_edge_direct_to_target (ie, target, delta);
}
}
Index: icln/gcc/ipa-prop.c
===================================================================
--- icln.orig/gcc/ipa-prop.c
+++ icln/gcc/ipa-prop.c
@@ -1458,35 +1458,43 @@ update_jump_functions_after_inlining (st
}
/* If TARGET is an addr_expr of a function declaration, make it the destination
- of an indirect edge IE and return the edge. Otherwise, return NULL. */
+ of an indirect edge IE and return the edge. Otherwise, return NULL. Delta,
+ if non-NULL, is an integer constant that must be added to this pointer
+ (first parameter). */
struct cgraph_edge *
-ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target)
+ipa_make_edge_direct_to_target (struct cgraph_edge *ie, tree target, tree delta)
{
struct cgraph_node *callee;
- if (TREE_CODE (target) != ADDR_EXPR)
- return NULL;
- target = TREE_OPERAND (target, 0);
+ if (TREE_CODE (target) == ADDR_EXPR)
+ target = TREE_OPERAND (target, 0);
if (TREE_CODE (target) != FUNCTION_DECL)
return NULL;
callee = cgraph_node (target);
if (!callee)
return NULL;
ipa_check_create_node_params ();
- cgraph_make_edge_direct (ie, callee);
+
+ cgraph_make_edge_direct (ie, callee, delta);
if (dump_file)
{
fprintf (dump_file, "ipa-prop: Discovered %s call to a known target "
- "(%s/%i -> %s/%i) for stmt ",
+ "(%s/%i -> %s/%i), for stmt ",
ie->indirect_info->polymorphic ? "a virtual" : "an indirect",
cgraph_node_name (ie->caller), ie->caller->uid,
cgraph_node_name (ie->callee), ie->callee->uid);
-
if (ie->call_stmt)
print_gimple_stmt (dump_file, ie->call_stmt, 2, TDF_SLIM);
else
fprintf (dump_file, "with uid %i\n", ie->lto_stmt_uid);
+
+ if (delta)
+ {
+ fprintf (dump_file, " Thunk delta is ");
+ print_generic_expr (dump_file, delta, 0);
+ fprintf (dump_file, "\n");
+ }
}
if (ipa_get_cs_argument_count (IPA_EDGE_REF (ie))
@@ -1514,7 +1522,7 @@ try_make_edge_direct_simple_call (struct
else
return NULL;
- return ipa_make_edge_direct_to_target (ie, target);
+ return ipa_make_edge_direct_to_target (ie, target, NULL_TREE);
}
/* Try to find a destination for indirect edge IE that corresponds to a
@@ -1526,7 +1534,7 @@ static struct cgraph_edge *
try_make_edge_direct_virtual_call (struct cgraph_edge *ie,
struct ipa_jump_func *jfunc)
{
- tree binfo, type, target;
+ tree binfo, type, target, delta;
HOST_WIDE_INT token;
if (jfunc->type == IPA_JF_KNOWN_TYPE)
@@ -1550,12 +1558,12 @@ try_make_edge_direct_virtual_call (struc
type = ie->indirect_info->otr_type;
binfo = get_binfo_at_offset (binfo, ie->indirect_info->anc_offset, type);
if (binfo)
- target = gimple_fold_obj_type_ref_known_binfo (token, binfo);
+ target = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta, true);
else
return NULL;
if (target)
- return ipa_make_edge_direct_to_target (ie, target);
+ return ipa_make_edge_direct_to_target (ie, target, delta);
else
return NULL;
}
@@ -2546,6 +2554,7 @@ ipa_write_indirect_edge_info (struct out
{
lto_output_sleb128_stream (ob->main_stream, ii->otr_token);
lto_output_tree (ob, ii->otr_type, true);
+ lto_output_tree (ob, ii->thunk_delta, true);
}
}
@@ -2568,6 +2577,7 @@ ipa_read_indirect_edge_info (struct lto_
{
ii->otr_token = (HOST_WIDE_INT) lto_input_sleb128 (ib);
ii->otr_type = lto_input_tree (ib, data_in);
+ ii->thunk_delta = lto_input_tree (ib, data_in);
}
}
Index: icln/gcc/ipa-prop.h
===================================================================
--- icln.orig/gcc/ipa-prop.h
+++ icln/gcc/ipa-prop.h
@@ -430,7 +430,8 @@ bool ipa_propagate_indirect_call_infos (
VEC (cgraph_edge_p, heap) **new_edges);
/* Indirect edge and binfo processing. */
-struct cgraph_edge *ipa_make_edge_direct_to_target (struct cgraph_edge *, tree);
+struct cgraph_edge *ipa_make_edge_direct_to_target (struct cgraph_edge *, tree,
+ tree);
/* Debugging interface. */
Index: icln/gcc/tree-ssa-ccp.c
===================================================================
--- icln.orig/gcc/tree-ssa-ccp.c
+++ icln/gcc/tree-ssa-ccp.c
@@ -2317,16 +2317,8 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi
{
tree expr = OBJ_TYPE_REF_EXPR (callee);
OBJ_TYPE_REF_EXPR (callee) = valueize_op (expr);
- if (TREE_CODE (OBJ_TYPE_REF_EXPR (callee)) == ADDR_EXPR)
- {
- tree t;
- t = gimple_fold_obj_type_ref (callee, NULL_TREE);
- if (t)
- {
- gimple_call_set_fn (stmt, t);
- changed = true;
- }
- }
+ if (gimple_fold_call (gsi, false))
+ changed = true;
OBJ_TYPE_REF_EXPR (callee) = expr;
}
Index: icln/gcc/testsuite/g++.dg/ipa/pr46053.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/pr46053.C
@@ -0,0 +1,41 @@
+/* { dg-do run } */
+/* { dg-options "-O -fipa-cp -fno-early-inlining" } */
+
+extern "C" void abort ();
+
+struct A
+{
+ virtual void foo () = 0;
+};
+
+struct B : A
+{
+ virtual void foo () = 0;
+};
+
+struct C : A
+{
+};
+
+struct D : C, B
+{
+ int i;
+ D () : i(0xaaaa) {}
+ virtual void foo ()
+ {
+ if (i != 0xaaaa)
+ abort();
+ }
+};
+
+static inline void bar (B &b)
+{
+ b.foo ();
+}
+
+int main()
+{
+ D d;
+ bar (d);
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/pr46287-1.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/pr46287-1.C
@@ -0,0 +1,67 @@
+// Check that indirect calls to thunks do not lead to errors.
+// { dg-do run }
+// { dg-options "-O" }
+
+extern "C" void abort ();
+
+class A
+{
+public:
+ virtual void foo () {abort();}
+};
+
+class B : public A
+{
+public:
+ int z;
+ virtual void foo () {abort();}
+};
+
+class C : public A
+{
+public:
+ void *a[32];
+ unsigned long b;
+ long c[32];
+
+ virtual void foo () {abort();}
+};
+
+class D : public C, public B
+{
+public:
+ D () : C(), B()
+ {
+ int i;
+ for (i = 0; i < 32; i++)
+ {
+ a[i] = (void *) 0;
+ c[i] = 0;
+ }
+ b = 0xaaaa;
+ }
+
+ virtual void foo ();
+};
+
+inline void D::foo()
+{
+ if (b != 0xaaaa)
+ abort();
+}
+
+static inline void bar (B &b)
+{
+
+ b.foo ();
+}
+
+int main()
+{
+ int i;
+ D d;
+
+ for (i = 0; i < 5000; i++)
+ bar (d);
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/pr46287-2.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/pr46287-2.C
@@ -0,0 +1,68 @@
+// Check that indirect calls to thunks do not lead to errors.
+// { dg-do run }
+// { dg-options "-O -finline -finline-small-functions -finline-functions" }
+
+
+extern "C" void abort ();
+
+class A
+{
+public:
+ virtual void foo () {abort();}
+};
+
+class B : public A
+{
+public:
+ int z;
+ virtual void foo () {abort();}
+};
+
+class C : public A
+{
+public:
+ void *a[32];
+ unsigned long b;
+ long c[32];
+
+ virtual void foo () {abort();}
+};
+
+class D : public C, public B
+{
+public:
+ D () : C(), B()
+ {
+ int i;
+ for (i = 0; i < 32; i++)
+ {
+ a[i] = (void *) 0;
+ c[i] = 0;
+ }
+ b = 0xaaaa;
+ }
+
+ virtual void foo ();
+};
+
+void D::foo()
+{
+ if (b != 0xaaaa)
+ abort();
+}
+
+static inline void bar (B &b)
+{
+
+ b.foo ();
+}
+
+int main()
+{
+ int i;
+ D d;
+
+ for (i = 0; i < 5000; i++)
+ bar (d);
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/ipa/pr46287-3.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/ipa/pr46287-3.C
@@ -0,0 +1,67 @@
+// Check that indirect calls to thunks do not lead to errors.
+// { dg-do run }
+// { dg-options "-O -fipa-cp" }
+
+extern "C" void abort ();
+
+class A
+{
+public:
+ virtual void foo () {abort();}
+};
+
+class B : public A
+{
+public:
+ int z;
+ virtual void foo () {abort();}
+};
+
+class C : public A
+{
+public:
+ void *a[32];
+ unsigned long b;
+ long c[32];
+
+ virtual void foo () {abort();}
+};
+
+class D : public C, public B
+{
+public:
+ D () : C(), B()
+ {
+ int i;
+ for (i = 0; i < 32; i++)
+ {
+ a[i] = (void *) 0;
+ c[i] = 0;
+ }
+ b = 0xaaaa;
+ }
+
+ virtual void foo ();
+};
+
+void D::foo()
+{
+ if (b != 0xaaaa)
+ abort();
+}
+
+static void bar (B &b)
+{
+
+ b.foo ();
+}
+
+int main()
+{
+ int i;
+ D d;
+
+ for (i = 0; i < 5000; i++)
+ bar (d);
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/torture/covariant-1.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/torture/covariant-1.C
@@ -0,0 +1,33 @@
+// { dg-do run }
+
+extern "C" void abort ();
+
+class A {
+public:
+ virtual A* getThis() { return this; }
+};
+
+class B {
+int a;
+public:
+ virtual B* getThis() { return this; }
+};
+
+class AB : public A, public B {
+public:
+ virtual AB* getThis() { return this; }
+};
+
+int main ()
+{
+ AB ab;
+
+ A* a = &ab;
+ B* b = &ab;
+
+ if (a->getThis() != a
+ || b->getThis() != b)
+ abort ();
+
+ return 0;
+}
Index: icln/gcc/testsuite/g++.dg/torture/pr46287.C
===================================================================
--- /dev/null
+++ icln/gcc/testsuite/g++.dg/torture/pr46287.C
@@ -0,0 +1,66 @@
+// Check that indirect calls to thunks do not lead to errors.
+// { dg-do run }
+
+extern "C" void abort ();
+
+class A
+{
+public:
+ virtual void foo () {abort();}
+};
+
+class B : public A
+{
+public:
+ int z;
+ virtual void foo () {abort();}
+};
+
+class C : public A
+{
+public:
+ void *a[32];
+ unsigned long b;
+ long c[32];
+
+ virtual void foo () {abort();}
+};
+
+class D : public C, public B
+{
+public:
+ D () : C(), B()
+ {
+ int i;
+ for (i = 0; i < 32; i++)
+ {
+ a[i] = (void *) 0;
+ c[i] = 0;
+ }
+ b = 0xaaaa;
+ }
+
+ virtual void foo ();
+};
+
+void D::foo()
+{
+ if (b != 0xaaaa)
+ abort();
+}
+
+static inline void bar (B &b)
+{
+
+ b.foo ();
+}
+
+int main()
+{
+ int i;
+ D d;
+
+ for (i = 0; i < 5000; i++)
+ bar (d);
+ return 0;
+}
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 3/6] More robust compute_complex_ancestor_jump_func
2010-12-01 20:22 ` [PATCH, PR 45934 3/6] More robust compute_complex_ancestor_jump_func Martin Jambor
@ 2010-12-01 20:28 ` Richard Guenther
0 siblings, 0 replies; 29+ messages in thread
From: Richard Guenther @ 2010-12-01 20:28 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches, Richard Guenther, Jan Hubicka
On Wed, Dec 1, 2010 at 9:16 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> compute_complex_ancestor_jump_func tries to match a pattern like this:
>
> if (obj_2(D) != 0B)
> goto <bb 3>;
> else
> goto <bb 4>;
>
> <bb 3>:
> iftmp.1_3 = &obj_2(D)->D.1762;
>
> <bb 4>:
> # iftmp.1_1 = PHI <iftmp.1_3(3), 0B(2)>
> D.1879_6 = middleman_1 (iftmp.1_1, i_5(D));
> return D.1879_6; */
>
> At some point while working at the devirtualization issues I have seen
> this failing because the zero was the zeroth parameter in the phi
> node. I think it is necessary for some of the new testcases to work
> but I am not really sure any more, I only remembered when I was
> splitting the big patch into separate ones.
>
> I did not bootstrap this patch separately but it did pas bootstrap and
> testsuite on x86_64-linux and make check-c++ on i686 together with the
> rest.
Ok.
Thanks,
Richard.
> Thanks,
>
> Martin
>
>
> 2010-11-30 Martin Jambor <mjambor@suse.cz>
>
> * ipa-prop.c (compute_complex_ancestor_jump_func): Work also if the
> zero is the first phi parameter.
>
> Index: icln/gcc/ipa-prop.c
> ===================================================================
> --- icln.orig/gcc/ipa-prop.c
> +++ icln/gcc/ipa-prop.c
> @@ -461,11 +461,15 @@ compute_complex_ancestor_jump_func (stru
> tree tmp, parm, expr;
> int index, i;
>
> - if (gimple_phi_num_args (phi) != 2
> - || !integer_zerop (PHI_ARG_DEF (phi, 1)))
> + if (gimple_phi_num_args (phi) != 2)
> return;
>
> - tmp = PHI_ARG_DEF (phi, 0);
> + if (integer_zerop (PHI_ARG_DEF (phi, 1)))
> + tmp = PHI_ARG_DEF (phi, 0);
> + else if (integer_zerop (PHI_ARG_DEF (phi, 0)))
> + tmp = PHI_ARG_DEF (phi, 1);
> + else
> + return;
> if (TREE_CODE (tmp) != SSA_NAME
> || SSA_NAME_IS_DEFAULT_DEF (tmp)
> || !POINTER_TYPE_P (TREE_TYPE (tmp))
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 2/6] Remove devirtualizations that cannot be done
2010-12-01 20:23 ` [PATCH, PR 45934 2/6] Remove devirtualizations that cannot be done Martin Jambor
@ 2010-12-01 20:35 ` Richard Guenther
2010-12-02 10:46 ` Martin Jambor
0 siblings, 1 reply; 29+ messages in thread
From: Richard Guenther @ 2010-12-01 20:35 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches, Jan Hubicka
On Wed, 1 Dec 2010, Martin Jambor wrote:
> This patch removes parts of the current devirtualization machinery
> that cannot work because of potential dynamic type change which might
> be under way during sub-object construction and destruction. In the
> general case we simply cannot know this about global objects and we do
> not want to proceed with the necessary detection when folding. So
> devirtualization based on global decls, global IPA-CP constants is
> being removed and devirtualization in folding is dumbed down to never
> work on sub-objects.
>
> This patch removes the two testcases that test devirtualization based
> on global variables. After this patch, g++.dg/otr-fold-[12].C,
> g++.dg/tree-ssa/pr43411.C and g++.dg/tree-ssa/pr45605.C fail, this is
> addressed by the very last patch in the series.
>
> Bootstrapped and tested separately on x86-64-linux, I think I also did
> make check-c++ on i686 but I am no longer really sure, it certainly
> passed along with the rest.
>
Comments inline
> Thanks,
>
> Martin
>
>
> 2010-11-30 Martin Jambor <mjambor@suse.cz>
>
> PR tree-optimization/45934
> PR tree-optimization/46302
> * gimple-fold.c (get_base_binfo_for_type): Removed.
> (gimple_get_relevant_ref_binfo): Likewise.
> (gimple_fold_obj_type_ref_call): Dumb down to 4.5 functionality,
> removed parameter inplace, updated the caller.
> * gimple.h (gimple_get_relevant_ref_binfo): Remove declaration.
> * ipa-cp.c (ipcp_propagate_types): Do not derive types from constants.
> (ipcp_discover_new_direct_edges): Do not do devirtualization based on
> constants.
> * ipa-prop.c (mem_ref_offset): New variable base to store the result
> of get_ref_base_and_extent.
> (compute_known_type_jump_func): Use get_ref_base_and_extent and
> get_binfo_at_offset instead of gimple_get_relevant_ref_binfo.
> (ipa_analyze_node): Push and pop cfun, set current_function_decl.
> (update_jump_functions_after_inlining): Do not derive types from
> constants.
> (try_make_edge_direct_virtual_call): Likewise.
> * tree.c (get_binfo_at_offset): Get type from non-artificial fields.
> * testsuite/g++.dg/ipa/ipcp-ivi-1.C: Removed.
> * testsuite/g++.dg/ipa/ivinline-6.C: Likewise.
>
>
> Index: icln/gcc/gimple-fold.c
> ===================================================================
> --- icln.orig/gcc/gimple-fold.c
> +++ icln/gcc/gimple-fold.c
> @@ -1360,88 +1360,6 @@ gimple_fold_builtin (gimple stmt)
> return result;
> }
>
> -/* Search for a base binfo of BINFO that corresponds to TYPE and return it if
> - it is found or NULL_TREE if it is not. */
> -
> -static tree
> -get_base_binfo_for_type (tree binfo, tree type)
> -{
> - int i;
> - tree base_binfo;
> - tree res = NULL_TREE;
> -
> - for (i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
> - if (TREE_TYPE (base_binfo) == type)
> - {
> - gcc_assert (!res);
> - res = base_binfo;
> - }
> -
> - return res;
> -}
> -
> -/* Return a binfo describing the part of object referenced by expression REF.
> - Return NULL_TREE if it cannot be determined. REF can consist of a series of
> - COMPONENT_REFs of a declaration or of an INDIRECT_REF or it can also be just
> - a simple declaration, indirect reference or an SSA_NAME. If the function
> - discovers an INDIRECT_REF or an SSA_NAME, it will assume that the
> - encapsulating type is described by KNOWN_BINFO, if it is not NULL_TREE.
> - Otherwise the first non-artificial field declaration or the base declaration
> - will be examined to get the encapsulating type. */
> -
> -tree
> -gimple_get_relevant_ref_binfo (tree ref, tree known_binfo)
> -{
> - while (true)
> - {
> - if (TREE_CODE (ref) == COMPONENT_REF)
> - {
> - tree par_type;
> - tree binfo;
> - tree field = TREE_OPERAND (ref, 1);
> -
> - if (!DECL_ARTIFICIAL (field))
> - {
> - tree type = TREE_TYPE (field);
> - if (TREE_CODE (type) == RECORD_TYPE)
> - return TYPE_BINFO (type);
> - else
> - return NULL_TREE;
> - }
> -
> - par_type = TREE_TYPE (TREE_OPERAND (ref, 0));
> - binfo = TYPE_BINFO (par_type);
> - if (!binfo
> - || BINFO_N_BASE_BINFOS (binfo) == 0)
> - return NULL_TREE;
> -
> - /* Offset 0 indicates the primary base, whose vtable contents are
> - represented in the binfo for the derived class. */
> - if (int_bit_position (field) != 0)
> - {
> - tree d_binfo;
> -
> - /* Get descendant binfo. */
> - d_binfo = gimple_get_relevant_ref_binfo (TREE_OPERAND (ref, 0),
> - known_binfo);
> - if (!d_binfo)
> - return NULL_TREE;
> - return get_base_binfo_for_type (d_binfo, TREE_TYPE (field));
> - }
> -
> - ref = TREE_OPERAND (ref, 0);
> - }
> - else if (DECL_P (ref) && TREE_CODE (TREE_TYPE (ref)) == RECORD_TYPE)
> - return TYPE_BINFO (TREE_TYPE (ref));
> - else if (known_binfo
> - && (TREE_CODE (ref) == SSA_NAME
> - || TREE_CODE (ref) == MEM_REF))
> - return known_binfo;
> - else
> - return NULL_TREE;
> - }
> -}
> -
> /* Return a declaration of a function which an OBJ_TYPE_REF references. TOKEN
> is integer form of OBJ_TYPE_REF_TOKEN of the reference expression.
> KNOWN_BINFO carries the binfo describing the true type of
> @@ -1525,7 +1443,7 @@ gimple_adjust_this_by_delta (gimple_stmt
> INPLACE is false. Return true iff the statement was changed. */
>
> static bool
> -gimple_fold_obj_type_ref_call (gimple_stmt_iterator *gsi, bool inplace)
> +gimple_fold_obj_type_ref_call (gimple_stmt_iterator *gsi)
> {
> gimple stmt = gsi_stmt (*gsi);
> tree ref = gimple_call_fn (stmt);
> @@ -1533,27 +1451,21 @@ gimple_fold_obj_type_ref_call (gimple_st
> tree binfo, fndecl, delta;
> HOST_WIDE_INT token;
>
> - if (TREE_CODE (obj) == ADDR_EXPR)
> - obj = TREE_OPERAND (obj, 0);
> - else
> + if (TREE_CODE (obj) != ADDR_EXPR)
> return false;
> -
> - binfo = gimple_get_relevant_ref_binfo (obj, NULL_TREE);
> + obj = TREE_OPERAND (obj, 0);
> + if (!DECL_P (obj)
> + || TREE_CODE (TREE_TYPE (obj)) != RECORD_TYPE)
> + return false;
> + binfo = TYPE_BINFO (TREE_TYPE (obj));
> if (!binfo)
> return false;
> +
> token = tree_low_cst (OBJ_TYPE_REF_TOKEN (ref), 1);
> - fndecl = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta,
> - !DECL_P (obj));
> + fndecl = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta, false);
> if (!fndecl)
> return false;
> -
> - if (integer_nonzerop (delta))
> - {
> - if (inplace)
> - return false;
> - gimple_adjust_this_by_delta (gsi, delta);
> - }
> -
> + gcc_assert (integer_zerop (delta));
> gimple_call_set_fndecl (stmt, fndecl);
> return true;
> }
> @@ -1591,7 +1503,7 @@ gimple_fold_call (gimple_stmt_iterator *
> here where we can just smash the call operand. */
> callee = gimple_call_fn (stmt);
> if (TREE_CODE (callee) == OBJ_TYPE_REF)
> - return gimple_fold_obj_type_ref_call (gsi, inplace);
> + return gimple_fold_obj_type_ref_call (gsi);
> }
>
> return false;
> Index: icln/gcc/gimple.h
> ===================================================================
> --- icln.orig/gcc/gimple.h
> +++ icln/gcc/gimple.h
> @@ -892,7 +892,6 @@ unsigned get_gimple_rhs_num_ops (enum tr
> gimple gimple_alloc_stat (enum gimple_code, unsigned MEM_STAT_DECL);
> const char *gimple_decl_printable_name (tree, int);
> bool gimple_fold_call (gimple_stmt_iterator *gsi, bool inplace);
> -tree gimple_get_relevant_ref_binfo (tree ref, tree known_binfo);
> tree gimple_get_virt_mehtod_for_binfo (HOST_WIDE_INT, tree, tree *, bool);
> void gimple_adjust_this_by_delta (gimple_stmt_iterator *, tree);
> /* Returns true iff T is a valid GIMPLE statement. */
> Index: icln/gcc/ipa-cp.c
> ===================================================================
> --- icln.orig/gcc/ipa-cp.c
> +++ icln/gcc/ipa-cp.c
> @@ -781,26 +781,16 @@ ipcp_propagate_types (struct ipa_node_pa
> struct ipa_node_params *callee_info,
> struct ipa_jump_func *jf, int i)
> {
> - tree cst, binfo;
> -
> switch (jf->type)
> {
> case IPA_JF_UNKNOWN:
> case IPA_JF_CONST_MEMBER_PTR:
> + case IPA_JF_CONST:
> break;
>
> case IPA_JF_KNOWN_TYPE:
> return ipcp_add_param_type (callee_info, i, jf->value.base_binfo);
>
> - case IPA_JF_CONST:
> - cst = jf->value.constant;
> - if (TREE_CODE (cst) != ADDR_EXPR)
> - break;
> - binfo = gimple_get_relevant_ref_binfo (TREE_OPERAND (cst, 0), NULL_TREE);
> - if (!binfo)
> - break;
> - return ipcp_add_param_type (callee_info, i, binfo);
> -
> case IPA_JF_PASS_THROUGH:
> case IPA_JF_ANCESTOR:
> return ipcp_copy_types (caller_info, callee_info, i, jf);
> @@ -1292,35 +1282,13 @@ ipcp_discover_new_direct_edges (struct c
> for (ie = node->indirect_calls; ie; ie = next_ie)
> {
> struct cgraph_indirect_call_info *ici = ie->indirect_info;
> - tree target, delta = NULL_TREE;
>
> next_ie = ie->next_callee;
> - if (ici->param_index != index)
> + if (ici->param_index != index
> + || ici->polymorphic)
> continue;
>
> - if (ici->polymorphic)
> - {
> - tree binfo;
> - HOST_WIDE_INT token;
> -
> - if (TREE_CODE (cst) != ADDR_EXPR)
> - continue;
> -
> - binfo = gimple_get_relevant_ref_binfo (TREE_OPERAND (cst, 0),
> - NULL_TREE);
> - if (!binfo)
> - continue;
> - gcc_assert (ie->indirect_info->anc_offset == 0);
> - token = ie->indirect_info->otr_token;
> - target = gimple_get_virt_mehtod_for_binfo (token, binfo, &delta,
> - true);
> - if (!target)
> - continue;
> - }
> - else
> - target = cst;
> -
> - ipa_make_edge_direct_to_target (ie, target, delta);
> + ipa_make_edge_direct_to_target (ie, cst, NULL_TREE);
> }
> }
>
> Index: icln/gcc/ipa-prop.c
> ===================================================================
> --- icln.orig/gcc/ipa-prop.c
> +++ icln/gcc/ipa-prop.c
> @@ -362,7 +362,7 @@ compute_complex_assign_jump_func (struct
> gimple stmt, tree name)
> {
> HOST_WIDE_INT offset, size, max_size;
> - tree op1, op2, type;
> + tree op1, op2, base, type;
> int index;
>
> op1 = gimple_assign_rhs1 (stmt);
> @@ -404,20 +404,21 @@ compute_complex_assign_jump_func (struct
> type = TREE_TYPE (op1);
> if (TREE_CODE (type) != RECORD_TYPE)
> return;
> - op1 = get_ref_base_and_extent (op1, &offset, &size, &max_size);
> - if (TREE_CODE (op1) != MEM_REF
> + base = get_ref_base_and_extent (op1, &offset, &size, &max_size);
> + if (TREE_CODE (base) != MEM_REF
> /* If this is a varying address, punt. */
> || max_size == -1
> || max_size != size)
> return;
> - offset += mem_ref_offset (op1).low * BITS_PER_UNIT;
> - op1 = TREE_OPERAND (op1, 0);
> - if (TREE_CODE (op1) != SSA_NAME
> - || !SSA_NAME_IS_DEFAULT_DEF (op1)
> + offset += mem_ref_offset (base).low * BITS_PER_UNIT;
> + base = TREE_OPERAND (base, 0);
> + if (TREE_CODE (base) != SSA_NAME
> + || !SSA_NAME_IS_DEFAULT_DEF (base)
> || offset < 0)
> return;
>
> - index = ipa_get_param_decl_index (info, SSA_NAME_VAR (op1));
> + /* Dynamic types are changed only in constructors and destructors and */
> + index = ipa_get_param_decl_index (info, SSA_NAME_VAR (base));
> if (index >= 0)
> {
> jfunc->type = IPA_JF_ANCESTOR;
> @@ -530,13 +531,26 @@ compute_complex_ancestor_jump_func (stru
> static void
> compute_known_type_jump_func (tree op, struct ipa_jump_func *jfunc)
> {
> - tree binfo;
> + HOST_WIDE_INT offset, size, max_size;
> + tree base, binfo;
>
> - if (TREE_CODE (op) != ADDR_EXPR)
> + if (TREE_CODE (op) != ADDR_EXPR
> + || TREE_CODE (TREE_TYPE (TREE_TYPE (op))) != RECORD_TYPE)
> return;
>
> op = TREE_OPERAND (op, 0);
> - binfo = gimple_get_relevant_ref_binfo (op, NULL_TREE);
> + base = get_ref_base_and_extent (op, &offset, &size, &max_size);
> + if (!DECL_P (base)
> + || max_size == -1
> + || max_size != size
> + || TREE_CODE (TREE_TYPE (base)) != RECORD_TYPE
> + || is_global_var (base))
> + return;
> +
> + binfo = TYPE_BINFO (TREE_TYPE (base));
> + if (!binfo)
> + return;
> + binfo = get_binfo_at_offset (binfo, offset, TREE_TYPE (op));
> if (binfo)
> {
> jfunc->type = IPA_JF_KNOWN_TYPE;
> @@ -1346,6 +1360,9 @@ ipa_analyze_node (struct cgraph_node *no
> struct param_analysis_info *parms_info;
> int i, param_count;
>
> + push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> + current_function_decl = node->decl;
> +
Why do we need to push/pop cfun now?
> ipa_initialize_node_params (node);
>
> param_count = ipa_get_param_count (info);
> @@ -1358,6 +1375,9 @@ ipa_analyze_node (struct cgraph_node *no
> for (i = 0; i < param_count; i++)
> if (parms_info[i].visited_statements)
> BITMAP_FREE (parms_info[i].visited_statements);
> +
> + current_function_decl = NULL;
> + pop_cfun ();
> }
>
>
> @@ -1416,17 +1436,6 @@ update_jump_functions_after_inlining (st
> src = ipa_get_ith_jump_func (top, dst->value.ancestor.formal_id);
> if (src->type == IPA_JF_KNOWN_TYPE)
> combine_known_type_and_ancestor_jfs (src, dst);
> - else if (src->type == IPA_JF_CONST)
> - {
> - struct ipa_jump_func kt_func;
> -
> - kt_func.type = IPA_JF_UNKNOWN;
> - compute_known_type_jump_func (src->value.constant, &kt_func);
> - if (kt_func.type == IPA_JF_KNOWN_TYPE)
> - combine_known_type_and_ancestor_jfs (&kt_func, dst);
> - else
> - dst->type = IPA_JF_UNKNOWN;
> - }
> else if (src->type == IPA_JF_PASS_THROUGH
> && src->value.pass_through.operation == NOP_EXPR)
> dst->value.ancestor.formal_id = src->value.pass_through.formal_id;
> @@ -1539,15 +1548,6 @@ try_make_edge_direct_virtual_call (struc
>
> if (jfunc->type == IPA_JF_KNOWN_TYPE)
> binfo = jfunc->value.base_binfo;
> - else if (jfunc->type == IPA_JF_CONST)
> - {
> - tree cst = jfunc->value.constant;
> - if (TREE_CODE (cst) == ADDR_EXPR)
> - binfo = gimple_get_relevant_ref_binfo (TREE_OPERAND (cst, 0),
> - NULL_TREE);
> - else
> - return NULL;
> - }
> else
> return NULL;
>
> Index: icln/gcc/tree.c
> ===================================================================
> --- icln.orig/gcc/tree.c
> +++ icln/gcc/tree.c
> @@ -10950,8 +10950,7 @@ get_binfo_at_offset (tree binfo, HOST_WI
>
> if (type == expected_type)
> return binfo;
> - if (TREE_CODE (type) != RECORD_TYPE
> - || offset < 0)
> + if (offset < 0)
> return NULL_TREE;
>
> for (fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
Please keep the record-type check, using TYPE_FIELDS isn't valid
for other kinds.
Otherwise ok.
Thanks,
Richard.
> @@ -10964,12 +10963,18 @@ get_binfo_at_offset (tree binfo, HOST_WI
> if (pos <= offset && (pos + size) > offset)
> break;
> }
> - if (!fld || !DECL_ARTIFICIAL (fld))
> + if (!fld || TREE_CODE (TREE_TYPE (fld)) != RECORD_TYPE)
> return NULL_TREE;
>
> + if (!DECL_ARTIFICIAL (fld))
> + {
> + binfo = TYPE_BINFO (TREE_TYPE (fld));
> + if (!binfo)
> + return NULL_TREE;
> + }
> /* Offset 0 indicates the primary base, whose vtable contents are
> represented in the binfo for the derived class. */
> - if (offset != 0)
> + else if (offset != 0)
> {
> tree base_binfo, found_binfo = NULL_TREE;
> for (i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
> Index: icln/gcc/testsuite/g++.dg/ipa/ipcp-ivi-1.C
> ===================================================================
> --- icln.orig/gcc/testsuite/g++.dg/ipa/ipcp-ivi-1.C
> +++ /dev/null
> @@ -1,65 +0,0 @@
> -/* Verify that simple virtual calls are inlined even without early
> - inlining. */
> -/* { dg-do run } */
> -/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining" } */
> -
> -extern "C" void abort (void);
> -
> -class A
> -{
> -public:
> - int data;
> - virtual int foo (int i);
> -};
> -
> -class B : public A
> -{
> -public:
> - virtual int foo (int i);
> -};
> -
> -class C : public A
> -{
> -public:
> - virtual int foo (int i);
> -};
> -
> -int A::foo (int i)
> -{
> - return i + 1;
> -}
> -
> -int B::foo (int i)
> -{
> - return i + 2;
> -}
> -
> -int C::foo (int i)
> -{
> - return i + 3;
> -}
> -
> -int __attribute__ ((noinline)) middleman (class A *obj, int i)
> -{
> - return obj->foo (i);
> -}
> -
> -int __attribute__ ((noinline,noclone)) get_input(void)
> -{
> - return 1;
> -}
> -
> -class B b;
> -
> -int main (int argc, char *argv[])
> -{
> - int i;
> -
> - for (i = 0; i < get_input (); i++)
> - if (middleman (&b, get_input ()) != 3)
> - abort ();
> - return 0;
> -}
> -
> -/* { dg-final { scan-ipa-dump "B::foo\[^\\n\]*inline copy in int.*middleman" "inline" } } */
> -/* { dg-final { cleanup-ipa-dump "inline" } } */
> Index: icln/gcc/testsuite/g++.dg/ipa/ivinline-6.C
> ===================================================================
> --- icln.orig/gcc/testsuite/g++.dg/ipa/ivinline-6.C
> +++ /dev/null
> @@ -1,58 +0,0 @@
> -/* Verify that virtual call inlining works also when it has to get the
> - type from an ipa invariant and that even in this case it does not
> - pick a wrong method when there is a user defined ancestor in an
> - object. */
> -/* { dg-do run } */
> -/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining -fno-ipa-cp" } */
> -
> -extern "C" void abort (void);
> -
> -class A
> -{
> -public:
> - int data;
> - virtual int foo (int i);
> -};
> -
> -class B : public A
> -{
> -public:
> - class A confusion;
> - virtual int foo (int i);
> -};
> -
> -int A::foo (int i)
> -{
> - return i + 1;
> -}
> -
> -int B::foo (int i)
> -{
> - return i + 2;
> -}
> -
> -int middleman (class A *obj, int i)
> -{
> - return obj->foo (i);
> -}
> -
> -int __attribute__ ((noinline,noclone)) get_input(void)
> -{
> - return 1;
> -}
> -
> -class B b;
> -
> -int main (int argc, char *argv[])
> -{
> - int i, j = get_input ();
> -
> - for (i = 0; i < j; i++)
> - if ((middleman (&b, j) + 100 * middleman (&b.confusion, j)) != 203)
> - abort ();
> - return 0;
> -}
> -
> -/* { dg-final { scan-ipa-dump "A::foo\[^\\n\]*inline copy in int main" "inline" } } */
> -/* { dg-final { scan-ipa-dump "B::foo\[^\\n\]*inline copy in int main" "inline" } } */
> -/* { dg-final { cleanup-ipa-dump "inline" } } */
>
>
--
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks
2010-12-01 20:23 ` [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks Martin Jambor
@ 2010-12-01 20:58 ` Jan Hubicka
2010-12-03 13:01 ` Martin Jambor
2010-12-17 14:14 ` H.J. Lu
1 sibling, 1 reply; 29+ messages in thread
From: Jan Hubicka @ 2010-12-01 20:58 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches, Richard Guenther, Jan Hubicka
> 2010-11-08 Martin Jambor <mjambor@suse.cz>
>
> PR tree-optimization/46053
> PR middle-end/46287
> * cgraph.h (cgraph_indirect_call_info): New field.
> * gimple.h (gimple_fold_obj_type_ref): Declaration removed.
> (gimple_fold_call): Declare.
> (gimple_adjust_this_by_delta): Likewise.
> * cgraph.c (cgraph_make_edge_direct): New parameter delta. Updated
> all users.
> (cgraph_clone_edge): Create a copy of indirect_info also for direct
> edges.
> * cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Adjust this
> parameters.
> * gimple-fold.c (gimple_fold_obj_type_ref_known_binfo): Renamed to
> gimple_get_virt_mehtod_for_binfo, new parameter delta. Do not search
> through thunks, in fact bail out if we encounter one, check that
> BINFO_VIRTUALS is not NULL.
> (gimple_adjust_this_by_delta): New function.
> (gimple_fold_obj_type_ref): Removed.
> (gimple_fold_obj_type_ref_call): New function.
> (fold_gimple_call): Renamed to gimple_fold_call, made external.
> Updated users. Call gimple_fold_obj_type_ref_call instead of
> gimple_fold_obj_type_ref.
> * ipa-cp.c (ipcp_process_devirtualization_opportunities): Process
> thunk deltas.
> (ipcp_discover_new_direct_edges): Likewise.
> * ipa-prop.c (ipa_make_edge_direct_to_target): New parameter delta.
> Updated callers.
> (ipa_write_indirect_edge_info): Stream thunk_delta.
> (ipa_read_indirect_edge_info): Likewise.
> * tree-ssa-ccp.c (ccp_fold_stmt): Use gimple_fold_call instead of
> gimple_fold_obj_type_ref.
>
> * testsuite/g++.dg/ipa/pr46053.C: New test.
> * testsuite/g++.dg/ipa/pr46287-1.C: Likewise.
> * testsuite/g++.dg/ipa/pr46287-2.C: Likewise.
> * testsuite/g++.dg/ipa/pr46287-3.C: Likewise.
> * testsuite/g++.dg/torture/covariant-1.C: Likewise.
> * testsuite/g++.dg/torture/pr46287.C: Likewise.
>
> /* Make an indirect EDGE with an unknown callee an ordinary edge leading to
> - CALLEE. */
> + CALLEE. DELTA, if non-NULL, is an integer constant that is to be added to
> + the this pointer (first parameter). */
>
> void
> -cgraph_make_edge_direct (struct cgraph_edge *edge, struct cgraph_node *callee)
> +cgraph_make_edge_direct (struct cgraph_edge *edge, struct cgraph_node *callee,
> + tree delta)
> {
> edge->indirect_unknown_callee = 0;
> + edge->indirect_info->thunk_delta = delta;
I guess it is still fine as it is definite improvement over current situation, but
won't we need to handle all of cgraph_thunk_info here?
In thunk_info it is HOST_WIDE_INT, I would expect it to be here as well.
> + if (e->indirect_info && e->indirect_info->thunk_delta
> + && integer_nonzerop (e->indirect_info->thunk_delta))
> + {
> + if (cgraph_dump_file)
> + {
> + fprintf (cgraph_dump_file, " Thunk delta is ");
> + print_generic_expr (cgraph_dump_file,
> + e->indirect_info->thunk_delta, 0);
> + fprintf (cgraph_dump_file, "\n");
> + }
> + gsi = gsi_for_stmt (e->call_stmt);
> + gsi_computed = true;
> + gimple_adjust_this_by_delta (&gsi, e->indirect_info->thunk_delta);
> + e->indirect_info->thunk_delta = NULL_TREE;
> + }
Should've test here if this parameter was eliminated or not?
(i.e. first bit of e->callee->clone.combined_args_to_skip?
> Index: icln/gcc/ipa-prop.c
> ===================================================================
> --- icln.orig/gcc/ipa-prop.c
> +++ icln/gcc/ipa-prop.c
I would expect somewhere here to be code handling updating of operand
value for non-0 delta when doing ipa-cp propagation, but don't seem to
be able to find it?
The rest seems OK. I am most concerned that we implement just part of thunk
logic, but I see that you get deltas from BINFOs and the rest of adjustments
are not there?
Honza
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 2/6] Remove devirtualizations that cannot be done
2010-12-01 20:35 ` Richard Guenther
@ 2010-12-02 10:46 ` Martin Jambor
2010-12-02 12:14 ` Richard Guenther
0 siblings, 1 reply; 29+ messages in thread
From: Martin Jambor @ 2010-12-02 10:46 UTC (permalink / raw)
To: Richard Guenther; +Cc: GCC Patches, Jan Hubicka
Hi,
On Wed, Dec 01, 2010 at 09:35:05PM +0100, Richard Guenther wrote:
> On Wed, 1 Dec 2010, Martin Jambor wrote:
>
> > This patch removes parts of the current devirtualization machinery
> > that cannot work because of potential dynamic type change which might
> > be under way during sub-object construction and destruction. In the
> > general case we simply cannot know this about global objects and we do
> > not want to proceed with the necessary detection when folding. So
> > devirtualization based on global decls, global IPA-CP constants is
> > being removed and devirtualization in folding is dumbed down to never
> > work on sub-objects.
> >
> > This patch removes the two testcases that test devirtualization based
> > on global variables. After this patch, g++.dg/otr-fold-[12].C,
> > g++.dg/tree-ssa/pr43411.C and g++.dg/tree-ssa/pr45605.C fail, this is
> > addressed by the very last patch in the series.
> >
> > Bootstrapped and tested separately on x86-64-linux, I think I also did
> > make check-c++ on i686 but I am no longer really sure, it certainly
> > passed along with the rest.
> >
> >
> > 2010-11-30 Martin Jambor <mjambor@suse.cz>
> >
> > PR tree-optimization/45934
> > PR tree-optimization/46302
> > * gimple-fold.c (get_base_binfo_for_type): Removed.
> > (gimple_get_relevant_ref_binfo): Likewise.
> > (gimple_fold_obj_type_ref_call): Dumb down to 4.5 functionality,
> > removed parameter inplace, updated the caller.
> > * gimple.h (gimple_get_relevant_ref_binfo): Remove declaration.
> > * ipa-cp.c (ipcp_propagate_types): Do not derive types from constants.
> > (ipcp_discover_new_direct_edges): Do not do devirtualization based on
> > constants.
> > * ipa-prop.c (mem_ref_offset): New variable base to store the result
> > of get_ref_base_and_extent.
> > (compute_known_type_jump_func): Use get_ref_base_and_extent and
> > get_binfo_at_offset instead of gimple_get_relevant_ref_binfo.
> > (ipa_analyze_node): Push and pop cfun, set current_function_decl.
> > (update_jump_functions_after_inlining): Do not derive types from
> > constants.
> > (try_make_edge_direct_virtual_call): Likewise.
> > * tree.c (get_binfo_at_offset): Get type from non-artificial fields.
> > * testsuite/g++.dg/ipa/ipcp-ivi-1.C: Removed.
> > * testsuite/g++.dg/ipa/ivinline-6.C: Likewise.
> >
> >
> > Index: icln/gcc/ipa-prop.c
> > ===================================================================
> > --- icln.orig/gcc/ipa-prop.c
> > +++ icln/gcc/ipa-prop.c
> > @@ -362,7 +362,7 @@ compute_complex_assign_jump_func (struct
> > gimple stmt, tree name)
> > {
> > HOST_WIDE_INT offset, size, max_size;
> > - tree op1, op2, type;
> > + tree op1, op2, base, type;
> > int index;
> >
> > op1 = gimple_assign_rhs1 (stmt);
> > @@ -404,20 +404,21 @@ compute_complex_assign_jump_func (struct
> > type = TREE_TYPE (op1);
> > if (TREE_CODE (type) != RECORD_TYPE)
> > return;
> > - op1 = get_ref_base_and_extent (op1, &offset, &size, &max_size);
> > - if (TREE_CODE (op1) != MEM_REF
> > + base = get_ref_base_and_extent (op1, &offset, &size, &max_size);
> > + if (TREE_CODE (base) != MEM_REF
> > /* If this is a varying address, punt. */
> > || max_size == -1
> > || max_size != size)
> > return;
> > - offset += mem_ref_offset (op1).low * BITS_PER_UNIT;
> > - op1 = TREE_OPERAND (op1, 0);
> > - if (TREE_CODE (op1) != SSA_NAME
> > - || !SSA_NAME_IS_DEFAULT_DEF (op1)
> > + offset += mem_ref_offset (base).low * BITS_PER_UNIT;
> > + base = TREE_OPERAND (base, 0);
> > + if (TREE_CODE (base) != SSA_NAME
> > + || !SSA_NAME_IS_DEFAULT_DEF (base)
> > || offset < 0)
> > return;
> >
> > - index = ipa_get_param_decl_index (info, SSA_NAME_VAR (op1));
> > + /* Dynamic types are changed only in constructors and destructors and */
> > + index = ipa_get_param_decl_index (info, SSA_NAME_VAR (base));
> > if (index >= 0)
> > {
> > jfunc->type = IPA_JF_ANCESTOR;
> > @@ -530,13 +531,26 @@ compute_complex_ancestor_jump_func (stru
> > static void
> > compute_known_type_jump_func (tree op, struct ipa_jump_func *jfunc)
> > {
> > - tree binfo;
> > + HOST_WIDE_INT offset, size, max_size;
> > + tree base, binfo;
> >
> > - if (TREE_CODE (op) != ADDR_EXPR)
> > + if (TREE_CODE (op) != ADDR_EXPR
> > + || TREE_CODE (TREE_TYPE (TREE_TYPE (op))) != RECORD_TYPE)
> > return;
> >
> > op = TREE_OPERAND (op, 0);
> > - binfo = gimple_get_relevant_ref_binfo (op, NULL_TREE);
> > + base = get_ref_base_and_extent (op, &offset, &size, &max_size);
> > + if (!DECL_P (base)
> > + || max_size == -1
> > + || max_size != size
> > + || TREE_CODE (TREE_TYPE (base)) != RECORD_TYPE
> > + || is_global_var (base))
> > + return;
> > +
> > + binfo = TYPE_BINFO (TREE_TYPE (base));
> > + if (!binfo)
> > + return;
> > + binfo = get_binfo_at_offset (binfo, offset, TREE_TYPE (op));
> > if (binfo)
> > {
> > jfunc->type = IPA_JF_KNOWN_TYPE;
> > @@ -1346,6 +1360,9 @@ ipa_analyze_node (struct cgraph_node *no
> > struct param_analysis_info *parms_info;
> > int i, param_count;
> >
> > + push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> > + current_function_decl = node->decl;
> > +
>
> Why do we need to push/pop cfun now?
It is not, these two hunks should have been in the type change
detection patch, it is not necessary here but required for walking the
VDEFs, IIRC.
>
> > ipa_initialize_node_params (node);
> >
> > param_count = ipa_get_param_count (info);
> > @@ -1358,6 +1375,9 @@ ipa_analyze_node (struct cgraph_node *no
> > for (i = 0; i < param_count; i++)
> > if (parms_info[i].visited_statements)
> > BITMAP_FREE (parms_info[i].visited_statements);
> > +
> > + current_function_decl = NULL;
> > + pop_cfun ();
> > }
> >
> >
> > @@ -1416,17 +1436,6 @@ update_jump_functions_after_inlining (st
> > src = ipa_get_ith_jump_func (top, dst->value.ancestor.formal_id);
> > if (src->type == IPA_JF_KNOWN_TYPE)
> > combine_known_type_and_ancestor_jfs (src, dst);
> > - else if (src->type == IPA_JF_CONST)
> > - {
> > - struct ipa_jump_func kt_func;
> > -
> > - kt_func.type = IPA_JF_UNKNOWN;
> > - compute_known_type_jump_func (src->value.constant, &kt_func);
> > - if (kt_func.type == IPA_JF_KNOWN_TYPE)
> > - combine_known_type_and_ancestor_jfs (&kt_func, dst);
> > - else
> > - dst->type = IPA_JF_UNKNOWN;
> > - }
> > else if (src->type == IPA_JF_PASS_THROUGH
> > && src->value.pass_through.operation == NOP_EXPR)
> > dst->value.ancestor.formal_id = src->value.pass_through.formal_id;
> > @@ -1539,15 +1548,6 @@ try_make_edge_direct_virtual_call (struc
> >
> > if (jfunc->type == IPA_JF_KNOWN_TYPE)
> > binfo = jfunc->value.base_binfo;
> > - else if (jfunc->type == IPA_JF_CONST)
> > - {
> > - tree cst = jfunc->value.constant;
> > - if (TREE_CODE (cst) == ADDR_EXPR)
> > - binfo = gimple_get_relevant_ref_binfo (TREE_OPERAND (cst, 0),
> > - NULL_TREE);
> > - else
> > - return NULL;
> > - }
> > else
> > return NULL;
> >
> > Index: icln/gcc/tree.c
> > ===================================================================
> > --- icln.orig/gcc/tree.c
> > +++ icln/gcc/tree.c
> > @@ -10950,8 +10950,7 @@ get_binfo_at_offset (tree binfo, HOST_WI
> >
> > if (type == expected_type)
> > return binfo;
> > - if (TREE_CODE (type) != RECORD_TYPE
> > - || offset < 0)
> > + if (offset < 0)
> > return NULL_TREE;
> >
> > for (fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
>
> Please keep the record-type check, using TYPE_FIELDS isn't valid
> for other kinds.
I removed the check on purpose. The first type is extracted from a
binfo and therefore it really should be a record (although the
checking macros in tree.h also accept a union, which would lead to
interesting consequences... fortunately all callers do check for
record types before getting their binfos) and the check for recordness
of the type in subsequent iterations has been moved...
>
> Otherwise ok.
>
> Thanks,
> Richard.
>
> > @@ -10964,12 +10963,18 @@ get_binfo_at_offset (tree binfo, HOST_WI
> > if (pos <= offset && (pos + size) > offset)
> > break;
> > }
> > - if (!fld || !DECL_ARTIFICIAL (fld))
> > + if (!fld || TREE_CODE (TREE_TYPE (fld)) != RECORD_TYPE)
> > return NULL_TREE;
...here. So unless we are worried about binfos of unions, I think
removing the check is safe.
Is that OK? If I commit this on its own we would have failing
testcases so I do not intedn to do it. Or should I go ahead and open
a PR for them?
Thanks,
Martin
> >
> > + if (!DECL_ARTIFICIAL (fld))
> > + {
> > + binfo = TYPE_BINFO (TREE_TYPE (fld));
> > + if (!binfo)
> > + return NULL_TREE;
> > + }
> > /* Offset 0 indicates the primary base, whose vtable contents are
> > represented in the binfo for the derived class. */
> > - if (offset != 0)
> > + else if (offset != 0)
> > {
> > tree base_binfo, found_binfo = NULL_TREE;
> > for (i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
> > Index: icln/gcc/testsuite/g++.dg/ipa/ipcp-ivi-1.C
> > ===================================================================
> > --- icln.orig/gcc/testsuite/g++.dg/ipa/ipcp-ivi-1.C
> > +++ /dev/null
> > @@ -1,65 +0,0 @@
> > -/* Verify that simple virtual calls are inlined even without early
> > - inlining. */
> > -/* { dg-do run } */
> > -/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining" } */
> > -
> > -extern "C" void abort (void);
> > -
> > -class A
> > -{
> > -public:
> > - int data;
> > - virtual int foo (int i);
> > -};
> > -
> > -class B : public A
> > -{
> > -public:
> > - virtual int foo (int i);
> > -};
> > -
> > -class C : public A
> > -{
> > -public:
> > - virtual int foo (int i);
> > -};
> > -
> > -int A::foo (int i)
> > -{
> > - return i + 1;
> > -}
> > -
> > -int B::foo (int i)
> > -{
> > - return i + 2;
> > -}
> > -
> > -int C::foo (int i)
> > -{
> > - return i + 3;
> > -}
> > -
> > -int __attribute__ ((noinline)) middleman (class A *obj, int i)
> > -{
> > - return obj->foo (i);
> > -}
> > -
> > -int __attribute__ ((noinline,noclone)) get_input(void)
> > -{
> > - return 1;
> > -}
> > -
> > -class B b;
> > -
> > -int main (int argc, char *argv[])
> > -{
> > - int i;
> > -
> > - for (i = 0; i < get_input (); i++)
> > - if (middleman (&b, get_input ()) != 3)
> > - abort ();
> > - return 0;
> > -}
> > -
> > -/* { dg-final { scan-ipa-dump "B::foo\[^\\n\]*inline copy in int.*middleman" "inline" } } */
> > -/* { dg-final { cleanup-ipa-dump "inline" } } */
> > Index: icln/gcc/testsuite/g++.dg/ipa/ivinline-6.C
> > ===================================================================
> > --- icln.orig/gcc/testsuite/g++.dg/ipa/ivinline-6.C
> > +++ /dev/null
> > @@ -1,58 +0,0 @@
> > -/* Verify that virtual call inlining works also when it has to get the
> > - type from an ipa invariant and that even in this case it does not
> > - pick a wrong method when there is a user defined ancestor in an
> > - object. */
> > -/* { dg-do run } */
> > -/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining -fno-ipa-cp" } */
> > -
> > -extern "C" void abort (void);
> > -
> > -class A
> > -{
> > -public:
> > - int data;
> > - virtual int foo (int i);
> > -};
> > -
> > -class B : public A
> > -{
> > -public:
> > - class A confusion;
> > - virtual int foo (int i);
> > -};
> > -
> > -int A::foo (int i)
> > -{
> > - return i + 1;
> > -}
> > -
> > -int B::foo (int i)
> > -{
> > - return i + 2;
> > -}
> > -
> > -int middleman (class A *obj, int i)
> > -{
> > - return obj->foo (i);
> > -}
> > -
> > -int __attribute__ ((noinline,noclone)) get_input(void)
> > -{
> > - return 1;
> > -}
> > -
> > -class B b;
> > -
> > -int main (int argc, char *argv[])
> > -{
> > - int i, j = get_input ();
> > -
> > - for (i = 0; i < j; i++)
> > - if ((middleman (&b, j) + 100 * middleman (&b.confusion, j)) != 203)
> > - abort ();
> > - return 0;
> > -}
> > -
> > -/* { dg-final { scan-ipa-dump "A::foo\[^\\n\]*inline copy in int main" "inline" } } */
> > -/* { dg-final { scan-ipa-dump "B::foo\[^\\n\]*inline copy in int main" "inline" } } */
> > -/* { dg-final { cleanup-ipa-dump "inline" } } */
> >
> >
>
> --
> Richard Guenther <rguenther@suse.de>
> Novell / SUSE Labs
> SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 2/6] Remove devirtualizations that cannot be done
2010-12-02 10:46 ` Martin Jambor
@ 2010-12-02 12:14 ` Richard Guenther
0 siblings, 0 replies; 29+ messages in thread
From: Richard Guenther @ 2010-12-02 12:14 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches, Jan Hubicka
On Thu, 2 Dec 2010, Martin Jambor wrote:
> Hi,
>
> On Wed, Dec 01, 2010 at 09:35:05PM +0100, Richard Guenther wrote:
> > On Wed, 1 Dec 2010, Martin Jambor wrote:
> >
> > > This patch removes parts of the current devirtualization machinery
> > > that cannot work because of potential dynamic type change which might
> > > be under way during sub-object construction and destruction. In the
> > > general case we simply cannot know this about global objects and we do
> > > not want to proceed with the necessary detection when folding. So
> > > devirtualization based on global decls, global IPA-CP constants is
> > > being removed and devirtualization in folding is dumbed down to never
> > > work on sub-objects.
> > >
> > > This patch removes the two testcases that test devirtualization based
> > > on global variables. After this patch, g++.dg/otr-fold-[12].C,
> > > g++.dg/tree-ssa/pr43411.C and g++.dg/tree-ssa/pr45605.C fail, this is
> > > addressed by the very last patch in the series.
> > >
> > > Bootstrapped and tested separately on x86-64-linux, I think I also did
> > > make check-c++ on i686 but I am no longer really sure, it certainly
> > > passed along with the rest.
> > >
> > >
> > > 2010-11-30 Martin Jambor <mjambor@suse.cz>
> > >
> > > PR tree-optimization/45934
> > > PR tree-optimization/46302
> > > * gimple-fold.c (get_base_binfo_for_type): Removed.
> > > (gimple_get_relevant_ref_binfo): Likewise.
> > > (gimple_fold_obj_type_ref_call): Dumb down to 4.5 functionality,
> > > removed parameter inplace, updated the caller.
> > > * gimple.h (gimple_get_relevant_ref_binfo): Remove declaration.
> > > * ipa-cp.c (ipcp_propagate_types): Do not derive types from constants.
> > > (ipcp_discover_new_direct_edges): Do not do devirtualization based on
> > > constants.
> > > * ipa-prop.c (mem_ref_offset): New variable base to store the result
> > > of get_ref_base_and_extent.
> > > (compute_known_type_jump_func): Use get_ref_base_and_extent and
> > > get_binfo_at_offset instead of gimple_get_relevant_ref_binfo.
> > > (ipa_analyze_node): Push and pop cfun, set current_function_decl.
> > > (update_jump_functions_after_inlining): Do not derive types from
> > > constants.
> > > (try_make_edge_direct_virtual_call): Likewise.
> > > * tree.c (get_binfo_at_offset): Get type from non-artificial fields.
> > > * testsuite/g++.dg/ipa/ipcp-ivi-1.C: Removed.
> > > * testsuite/g++.dg/ipa/ivinline-6.C: Likewise.
> > >
> > >
> > > Index: icln/gcc/ipa-prop.c
> > > ===================================================================
> > > --- icln.orig/gcc/ipa-prop.c
> > > +++ icln/gcc/ipa-prop.c
> > > @@ -362,7 +362,7 @@ compute_complex_assign_jump_func (struct
> > > gimple stmt, tree name)
> > > {
> > > HOST_WIDE_INT offset, size, max_size;
> > > - tree op1, op2, type;
> > > + tree op1, op2, base, type;
> > > int index;
> > >
> > > op1 = gimple_assign_rhs1 (stmt);
> > > @@ -404,20 +404,21 @@ compute_complex_assign_jump_func (struct
> > > type = TREE_TYPE (op1);
> > > if (TREE_CODE (type) != RECORD_TYPE)
> > > return;
> > > - op1 = get_ref_base_and_extent (op1, &offset, &size, &max_size);
> > > - if (TREE_CODE (op1) != MEM_REF
> > > + base = get_ref_base_and_extent (op1, &offset, &size, &max_size);
> > > + if (TREE_CODE (base) != MEM_REF
> > > /* If this is a varying address, punt. */
> > > || max_size == -1
> > > || max_size != size)
> > > return;
> > > - offset += mem_ref_offset (op1).low * BITS_PER_UNIT;
> > > - op1 = TREE_OPERAND (op1, 0);
> > > - if (TREE_CODE (op1) != SSA_NAME
> > > - || !SSA_NAME_IS_DEFAULT_DEF (op1)
> > > + offset += mem_ref_offset (base).low * BITS_PER_UNIT;
> > > + base = TREE_OPERAND (base, 0);
> > > + if (TREE_CODE (base) != SSA_NAME
> > > + || !SSA_NAME_IS_DEFAULT_DEF (base)
> > > || offset < 0)
> > > return;
> > >
> > > - index = ipa_get_param_decl_index (info, SSA_NAME_VAR (op1));
> > > + /* Dynamic types are changed only in constructors and destructors and */
> > > + index = ipa_get_param_decl_index (info, SSA_NAME_VAR (base));
> > > if (index >= 0)
> > > {
> > > jfunc->type = IPA_JF_ANCESTOR;
> > > @@ -530,13 +531,26 @@ compute_complex_ancestor_jump_func (stru
> > > static void
> > > compute_known_type_jump_func (tree op, struct ipa_jump_func *jfunc)
> > > {
> > > - tree binfo;
> > > + HOST_WIDE_INT offset, size, max_size;
> > > + tree base, binfo;
> > >
> > > - if (TREE_CODE (op) != ADDR_EXPR)
> > > + if (TREE_CODE (op) != ADDR_EXPR
> > > + || TREE_CODE (TREE_TYPE (TREE_TYPE (op))) != RECORD_TYPE)
> > > return;
> > >
> > > op = TREE_OPERAND (op, 0);
> > > - binfo = gimple_get_relevant_ref_binfo (op, NULL_TREE);
> > > + base = get_ref_base_and_extent (op, &offset, &size, &max_size);
> > > + if (!DECL_P (base)
> > > + || max_size == -1
> > > + || max_size != size
> > > + || TREE_CODE (TREE_TYPE (base)) != RECORD_TYPE
> > > + || is_global_var (base))
> > > + return;
> > > +
> > > + binfo = TYPE_BINFO (TREE_TYPE (base));
> > > + if (!binfo)
> > > + return;
> > > + binfo = get_binfo_at_offset (binfo, offset, TREE_TYPE (op));
> > > if (binfo)
> > > {
> > > jfunc->type = IPA_JF_KNOWN_TYPE;
> > > @@ -1346,6 +1360,9 @@ ipa_analyze_node (struct cgraph_node *no
> > > struct param_analysis_info *parms_info;
> > > int i, param_count;
> > >
> > > + push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> > > + current_function_decl = node->decl;
> > > +
> >
> > Why do we need to push/pop cfun now?
>
> It is not, these two hunks should have been in the type change
> detection patch, it is not necessary here but required for walking the
> VDEFs, IIRC.
>
> >
> > > ipa_initialize_node_params (node);
> > >
> > > param_count = ipa_get_param_count (info);
> > > @@ -1358,6 +1375,9 @@ ipa_analyze_node (struct cgraph_node *no
> > > for (i = 0; i < param_count; i++)
> > > if (parms_info[i].visited_statements)
> > > BITMAP_FREE (parms_info[i].visited_statements);
> > > +
> > > + current_function_decl = NULL;
> > > + pop_cfun ();
> > > }
> > >
> > >
> > > @@ -1416,17 +1436,6 @@ update_jump_functions_after_inlining (st
> > > src = ipa_get_ith_jump_func (top, dst->value.ancestor.formal_id);
> > > if (src->type == IPA_JF_KNOWN_TYPE)
> > > combine_known_type_and_ancestor_jfs (src, dst);
> > > - else if (src->type == IPA_JF_CONST)
> > > - {
> > > - struct ipa_jump_func kt_func;
> > > -
> > > - kt_func.type = IPA_JF_UNKNOWN;
> > > - compute_known_type_jump_func (src->value.constant, &kt_func);
> > > - if (kt_func.type == IPA_JF_KNOWN_TYPE)
> > > - combine_known_type_and_ancestor_jfs (&kt_func, dst);
> > > - else
> > > - dst->type = IPA_JF_UNKNOWN;
> > > - }
> > > else if (src->type == IPA_JF_PASS_THROUGH
> > > && src->value.pass_through.operation == NOP_EXPR)
> > > dst->value.ancestor.formal_id = src->value.pass_through.formal_id;
> > > @@ -1539,15 +1548,6 @@ try_make_edge_direct_virtual_call (struc
> > >
> > > if (jfunc->type == IPA_JF_KNOWN_TYPE)
> > > binfo = jfunc->value.base_binfo;
> > > - else if (jfunc->type == IPA_JF_CONST)
> > > - {
> > > - tree cst = jfunc->value.constant;
> > > - if (TREE_CODE (cst) == ADDR_EXPR)
> > > - binfo = gimple_get_relevant_ref_binfo (TREE_OPERAND (cst, 0),
> > > - NULL_TREE);
> > > - else
> > > - return NULL;
> > > - }
> > > else
> > > return NULL;
> > >
> > > Index: icln/gcc/tree.c
> > > ===================================================================
> > > --- icln.orig/gcc/tree.c
> > > +++ icln/gcc/tree.c
> > > @@ -10950,8 +10950,7 @@ get_binfo_at_offset (tree binfo, HOST_WI
> > >
> > > if (type == expected_type)
> > > return binfo;
> > > - if (TREE_CODE (type) != RECORD_TYPE
> > > - || offset < 0)
> > > + if (offset < 0)
> > > return NULL_TREE;
> > >
> > > for (fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
> >
> > Please keep the record-type check, using TYPE_FIELDS isn't valid
> > for other kinds.
>
> I removed the check on purpose. The first type is extracted from a
> binfo and therefore it really should be a record (although the
> checking macros in tree.h also accept a union, which would lead to
> interesting consequences... fortunately all callers do check for
> record types before getting their binfos) and the check for recordness
> of the type in subsequent iterations has been moved...
>
> >
> > Otherwise ok.
> >
> > Thanks,
> > Richard.
> >
> > > @@ -10964,12 +10963,18 @@ get_binfo_at_offset (tree binfo, HOST_WI
> > > if (pos <= offset && (pos + size) > offset)
> > > break;
> > > }
> > > - if (!fld || !DECL_ARTIFICIAL (fld))
> > > + if (!fld || TREE_CODE (TREE_TYPE (fld)) != RECORD_TYPE)
> > > return NULL_TREE;
>
> ...here. So unless we are worried about binfos of unions, I think
> removing the check is safe.
>
> Is that OK? If I commit this on its own we would have failing
> testcases so I do not intedn to do it. Or should I go ahead and open
> a PR for them?
Ok. You could XFAIL them and un-XFAIL them when they are fixed again.
It would be nice to have the series bi-sectable (but of course the
intermediate steps should bootstrap & regtest ok).
Thanks,
Richard.
> Thanks,
>
> Martin
>
>
> > >
> > > + if (!DECL_ARTIFICIAL (fld))
> > > + {
> > > + binfo = TYPE_BINFO (TREE_TYPE (fld));
> > > + if (!binfo)
> > > + return NULL_TREE;
> > > + }
> > > /* Offset 0 indicates the primary base, whose vtable contents are
> > > represented in the binfo for the derived class. */
> > > - if (offset != 0)
> > > + else if (offset != 0)
> > > {
> > > tree base_binfo, found_binfo = NULL_TREE;
> > > for (i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
> > > Index: icln/gcc/testsuite/g++.dg/ipa/ipcp-ivi-1.C
> > > ===================================================================
> > > --- icln.orig/gcc/testsuite/g++.dg/ipa/ipcp-ivi-1.C
> > > +++ /dev/null
> > > @@ -1,65 +0,0 @@
> > > -/* Verify that simple virtual calls are inlined even without early
> > > - inlining. */
> > > -/* { dg-do run } */
> > > -/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining" } */
> > > -
> > > -extern "C" void abort (void);
> > > -
> > > -class A
> > > -{
> > > -public:
> > > - int data;
> > > - virtual int foo (int i);
> > > -};
> > > -
> > > -class B : public A
> > > -{
> > > -public:
> > > - virtual int foo (int i);
> > > -};
> > > -
> > > -class C : public A
> > > -{
> > > -public:
> > > - virtual int foo (int i);
> > > -};
> > > -
> > > -int A::foo (int i)
> > > -{
> > > - return i + 1;
> > > -}
> > > -
> > > -int B::foo (int i)
> > > -{
> > > - return i + 2;
> > > -}
> > > -
> > > -int C::foo (int i)
> > > -{
> > > - return i + 3;
> > > -}
> > > -
> > > -int __attribute__ ((noinline)) middleman (class A *obj, int i)
> > > -{
> > > - return obj->foo (i);
> > > -}
> > > -
> > > -int __attribute__ ((noinline,noclone)) get_input(void)
> > > -{
> > > - return 1;
> > > -}
> > > -
> > > -class B b;
> > > -
> > > -int main (int argc, char *argv[])
> > > -{
> > > - int i;
> > > -
> > > - for (i = 0; i < get_input (); i++)
> > > - if (middleman (&b, get_input ()) != 3)
> > > - abort ();
> > > - return 0;
> > > -}
> > > -
> > > -/* { dg-final { scan-ipa-dump "B::foo\[^\\n\]*inline copy in int.*middleman" "inline" } } */
> > > -/* { dg-final { cleanup-ipa-dump "inline" } } */
> > > Index: icln/gcc/testsuite/g++.dg/ipa/ivinline-6.C
> > > ===================================================================
> > > --- icln.orig/gcc/testsuite/g++.dg/ipa/ivinline-6.C
> > > +++ /dev/null
> > > @@ -1,58 +0,0 @@
> > > -/* Verify that virtual call inlining works also when it has to get the
> > > - type from an ipa invariant and that even in this case it does not
> > > - pick a wrong method when there is a user defined ancestor in an
> > > - object. */
> > > -/* { dg-do run } */
> > > -/* { dg-options "-O3 -fdump-ipa-inline -fno-early-inlining -fno-ipa-cp" } */
> > > -
> > > -extern "C" void abort (void);
> > > -
> > > -class A
> > > -{
> > > -public:
> > > - int data;
> > > - virtual int foo (int i);
> > > -};
> > > -
> > > -class B : public A
> > > -{
> > > -public:
> > > - class A confusion;
> > > - virtual int foo (int i);
> > > -};
> > > -
> > > -int A::foo (int i)
> > > -{
> > > - return i + 1;
> > > -}
> > > -
> > > -int B::foo (int i)
> > > -{
> > > - return i + 2;
> > > -}
> > > -
> > > -int middleman (class A *obj, int i)
> > > -{
> > > - return obj->foo (i);
> > > -}
> > > -
> > > -int __attribute__ ((noinline,noclone)) get_input(void)
> > > -{
> > > - return 1;
> > > -}
> > > -
> > > -class B b;
> > > -
> > > -int main (int argc, char *argv[])
> > > -{
> > > - int i, j = get_input ();
> > > -
> > > - for (i = 0; i < j; i++)
> > > - if ((middleman (&b, j) + 100 * middleman (&b.confusion, j)) != 203)
> > > - abort ();
> > > - return 0;
> > > -}
> > > -
> > > -/* { dg-final { scan-ipa-dump "A::foo\[^\\n\]*inline copy in int main" "inline" } } */
> > > -/* { dg-final { scan-ipa-dump "B::foo\[^\\n\]*inline copy in int main" "inline" } } */
> > > -/* { dg-final { cleanup-ipa-dump "inline" } } */
> > >
> > >
> >
> > --
> > Richard Guenther <rguenther@suse.de>
> > Novell / SUSE Labs
> > SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex
>
>
--
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 4/6] Dynamic type change detection
2010-12-01 20:23 ` [PATCH, PR 45934 4/6] Dynamic type change detection Martin Jambor
@ 2010-12-02 15:19 ` Richard Guenther
2010-12-02 16:17 ` Richard Guenther
2010-12-02 23:25 ` Jason Merrill
1 sibling, 1 reply; 29+ messages in thread
From: Richard Guenther @ 2010-12-02 15:19 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches, Jan Hubicka
On Wed, 1 Dec 2010, Martin Jambor wrote:
> This is the crux of the matter. I'll try to explain what and why I do
> in order to detect sub-object type changes during their construction
> first, below it there is the detection code as it looks like with the
> next patch applied and then there is of course the patch at the end.
>
> I have split the detection code into two patches, this one just
> detects that there is a type change in force and the subsequent one
> also tries to derive the new type. I did this in order to better
> structure the discussion about both and I also intend to commit the
> separately, easing potential bisecting (I hope there will be no reason
> for that of course). If it was absolutely necessary, we could
> postpone the next patch for stage1 but I hope to commit a variant of
> it soon.
>
> Because operations like placement new, memcpy and other byte-per-byte
> operations with objects that have virtual methods are deemed to
> produce code with undefined behavior and there are no unions of
> non-POD types, dynamic type change has a special meaning for
> devirtualization and only refers to what actual virtual method table a
> VMT pointer of an object points to. On the other hand the type in the
> sense of what you get from TREE_TYPE of the base of any given access
> to it is still the same and it is its static type. An important
> property of such objects is that their dynamic types (as given by the
> current VMT) can only be altered in constructors and destructors.
>
> This patch makes special assumptions about both constructors and
> destructors which are all the functions that are allowed to alter the
> dynamic types. It assumes that destructors begin with assignment into
> all VMT pointers and that constructors essentially look in the
> following way:
>
> 1) The very first thing they do is that they call constructors of the
> components (including ancestor sub-objects) that have them.
>
> 2) Then VMT pointers of this and all its ancestors is set to new
> values corresponding to the type corresponding to the constructor.
>
> 3) Only afterwards, other stuff such as the code written by the user
> is run. Only this may include calling virtual functions, directly
> or indirectly.
>
> There is no way to call a constructor of an ancestor sub-object in any
> other way (or any component for that matter but type is interesting
> only for ancestors).
>
> This means that we do not have to care whether constructors get the
> correct type information because they will always change it (in fact,
> if we define the type to be given by the VMT pointer, it is
> undefined).
>
> The most important fact to derive from the above is that if, for some
> statement in the section 3, we try to detect whether the dynamic type
> has changed, we can safely ignore all calls as we examine the function
> body backwards until we reach statements in section 2 because these
> calls cannot be constructors or destructors (if the input is not
> bogus) and so do not change the dynamic type. We then must detect
> that statements in section 2 change the dynamic type and can try to
> derive the new type. That is enough and we can stop, we will never
> see the calls into constructors of sub-objects in this code.
> Therefore we can safely ignore all call statements that we traverse,
> possibly except builtins which we consider as changing the type in an
> unknown way and which previous optimization passes might have
> introduced.
>
> Constructors of ancestors may be early inlined but that does not
> change the above division. Function splitting may lead to inlining of
> only a portion of a constructor but it can only inline the beginning
> of the constructor, not some part in the middle and so the division
> again holds. Moreover, the inlined portion will essentially be
> divided into three such sections too (perhaps some of the latter ones
> in another function which means that the type we pass to it either
> does not matter or is known) and therefore the same arguments will
> also hold for the associated sub-object.
>
> Exceptions can change this flow and lead to invocations of destructors
> of the completely constructed sub-objects in places where the method
> based on examining all possible previous statements might fail.
> However, because destructors begin with assignment to all VMT
> pointers, the type information passed to them is never used anyway.
> Apart from the destructors, user code cannot handle exceptions that
> escape an ancestor constructor in any of the constructors of the
> descendants and so we will never encounter calls to other functions in
> these areas.
>
> Therefore we do the following. Whenever we are about to construct a
> jump function that carries type information (known_type, ancestor or
> pass_through) or note that a virtual call is based on the type of a
> parameter, we invoke a function to check whether the dynamic type has
> changed and see whether we can deduce the new type. This function
> traverses VDEFs from the call site and examines individual statements
> and their potential or known effect on the VMT pointers in question.
>
> When we do this, we employ the TBAA disambiguation using the static
> type of the (sub-)object for reasons stated above when defining what
> we mean by dynamic type change. I believe we can even use types
> derived from pointers because of what conversions are allowed by the
> C++ standard.
>
> When examining the statements, we default to assume VMT pointers are
> changed in unpredictable ways. There are, however, two exceptions.
> The first are calls to non-builtins which we consider harmless for the
> reasons above. The second one are assignments to COMPONENT_REFs where
> we check whether the FIELD_DECL has the virtual flag set or not. If
> it is not set, we assume this is not a store to the VMT pointer. In
> the future, we might want to add more such cases (like ignoring
> ARRAY_REFs too, for example) but so far I am happy with the two.
>
> If the assignment is a COMPONENT_REF to a field with the virtual flag
> set and the base is the same decl or SSA name as the object we are
> examining, we try to deduce the new type from the right hand side.
>
> Tho code that does both detection of dynamic type changes and tries to
> extract the new dynamic type in some cases is the following:
>
> ----------------------------------------------------------------------
>
> /* Structure to be passed in between detect_type_change_anc and
> check_stmt_for_type_change. */
>
> struct type_change_info
> {
> /* The declaration or SSA_NAME pointer of the base that we are checking for
> type change. */
> tree object;
> /* If we actually can tell the type that the object has changed to, it is
> stored in this field. Otherwise it remains NULL_TREE. */
> tree known_current_type;
> /* Set to true if dynamic type change has been detected. */
> bool type_maybe_changed;
> };
>
> /* Return true if STMT can modify a virtual method table pointer. The function
> assumes it will never be called on constructor or destructor calls (since
> the search for dynamic type change will always end before reaching the
> former and lifetime of the object is over anyway after the latter. */
>
> static bool
> stmt_may_be_vtbl_ptr_store (gimple stmt)
> {
> if (is_gimple_call (stmt))
> {
> tree fndecl = gimple_call_fndecl (stmt);
> if (!fndecl || !DECL_BUILT_IN (fndecl))
> return false;
I'm not totally convinced by your arguments above (mainly the
partial inlining and EH ones). But let those aside, why are
all builtins possibly changing the vtable (but not indirect
calls to builtins (huh)). I know I made the memcpy argument,
but if it is valid then any call can make a memcpy call and
so all calls would be possibly changing the vtable pointer.
Thus, please remove the code and instead do
if (is_gimple_call (stmt))
return false;
and add a big comment as to why, possibly just cut&pasted from
your mail.
> }
> else if (is_gimple_assign (stmt))
> {
> tree lhs = gimple_assign_lhs (stmt);
>
> if (TREE_CODE (lhs) == COMPONENT_REF
> && !DECL_VIRTUAL_P (TREE_OPERAND (lhs, 1)))
> return false;
> /* In the future we might want to use get_base_ref_and_offset to find
> if there is a field corresponding to the offset and if so, proceed
> almost like if it was a component ref. */
> }
> return true;
Otherwise I guess I'm happy with this as initial implementation.
I could now argue that any aggregate store could also copy
a vtable pointer though, like when we'd detect that we are
doing copy construction and copying every field, including
the vtable pointer we could optimize it that way.
As I know we don't do that at the moment I'm fine if you add
a FIXME comment with respect to that. Or, alternatively,
only return false if !AGGREGATE_TYPE_P (TREE_TYPE (lhs))
which would me make feel 120% comfortable.
> }
>
> /* If STMT can be proved to be an assignment to the virtual method table
> pointer of ANALYZED_OBJ and the type associated witht the new table
> identified, return the type. Otherwise return NULL_TREE. */
>
> static tree
> extr_type_from_vtbl_ptr_store (gimple stmt, tree analyzed_obj)
> {
> tree lhs, t, obj;
>
> if (!is_gimple_assign (stmt))
> return NULL_TREE;
>
> lhs = gimple_assign_lhs (stmt);
>
> if (TREE_CODE (lhs) != COMPONENT_REF)
> return NULL_TREE;
> obj = lhs;
>
> if (!DECL_VIRTUAL_P (TREE_OPERAND (lhs, 1)))
> return NULL_TREE;
>
> do
> {
> obj = TREE_OPERAND (obj, 0);
> } while (TREE_CODE (obj) == COMPONENT_REF);
while goes to the next line
> if (TREE_CODE (obj) == MEM_REF)
> obj = TREE_OPERAND (obj, 0);
> if (TREE_CODE (obj) == ADDR_EXPR)
> obj = TREE_OPERAND (obj, 0);
> if (obj != analyzed_obj)
> return NULL_TREE;
>
excess vertical space
> t = gimple_assign_rhs1 (stmt);
> if (TREE_CODE (t) != ADDR_EXPR)
> return NULL_TREE;
> t = get_base_address (TREE_OPERAND (t, 0));
> if (!t || TREE_CODE (t) != VAR_DECL || !DECL_VIRTUAL_P (t))
> return NULL_TREE;
>
> return DECL_CONTEXT (t);
Can you document the special semantics of a vtables DECL_CONTEXT
in tree.h please?
> }
>
> /* Callbeck of walk_aliased_vdefs and a helper function for
> detect_type_change_anc to check whether a particular statement may modify
> the virtual table pointer, and if possible also determine the new type of
> the (sub-)object. It stores its result into DATA, which points to a
> type_change_info structure. */
>
> static bool
> check_stmt_for_type_change (ao_ref *ao ATTRIBUTE_UNUSED, tree vdef, void *data)
> {
> gimple stmt = SSA_NAME_DEF_STMT (vdef);
> struct type_change_info *tci = (struct type_change_info *) data;
>
> if (stmt_may_be_vtbl_ptr_store (stmt))
> {
> tci->type_maybe_changed = true;
> tci->known_current_type = extr_type_from_vtbl_ptr_store (stmt,
> tci->object);
Now we get to the EH concern. Please change that to
tree type;
tci->type_maybe_changed = true;
type = extr_type_from_vtbl_ptr_store (stmt, tci->object);
if (tci->known_current_type
&& type != tci->known_current_type)
tci->known_current_type = (tree) (intptr_t) -1;
if you ever come along a PHI node while walking and detect two different
vtable pointer stores before reaching a common dominator punt. You
say this won't happen, but I say be safe, not sorry.
You of course need to handle the -1 value in the consumers as
also unknown (or add another flag, whatever you prefer).
> return true;
> }
> else
> return false;
> }
>
> /* Detect whether the dynamic type of ARG 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. If
> ANC_OFFSET is non-zero, look for binfo of its ancestor of that type at that
> offset. ARG is supposed to be a dereferenced pointer or a declaration or a
> series of component_refs of either. */
>
> static bool
> detect_type_change_anc (tree arg, gimple call, struct ipa_jump_func *jfunc,
> HOST_WIDE_INT anc_offset)
> {
> struct type_change_info tci;
> tree obj;
> ao_ref ar;
So this function is for pass-by-value of objects while the one below
is for pass-by-reference? Or is arg always a pointer but it is
dereferenced in the callee here as opposed to the other function?
> /* Const calls cannot call virtual methods through VMT and so type changes do
> not matter. */
> if (!gimple_vuse (call))
> return false;
>
> obj = arg;
> while (handled_component_p (obj))
> obj = TREE_OPERAND (obj, 0);
> if (TREE_CODE (obj) == MEM_REF)
> obj = TREE_OPERAND (obj, 0);
> if (TREE_CODE (obj) == ADDR_EXPR)
> obj = TREE_OPERAND (obj, 0);
As of the question above I'm not sure but this doesn't seem to handle
an array of objects correctly (we lose information as to what sub-object
we refer to). That is, I could do
A a[2];
~a[1];
foo (&a[0]);
and you'd wrongly infer for example the vtable for a pure-virtual
base (thus, I get the usual problem of pure-virtual call runtime
error when refering to a destructed object)?
At least you seem to assume that from the base of the address/object
you can only refer to a single object?
> tci.object = obj;
> tci.known_current_type = NULL_TREE;
> tci.type_maybe_changed = false;
>
> ao_ref_init (&ar, arg);
> walk_aliased_vdefs (&ar, gimple_vuse (call), check_stmt_for_type_change,
> &tci, NULL);
> if (!tci.type_maybe_changed)
> return false;
>
> if (!tci.known_current_type)
> jfunc->type = IPA_JF_UNKNOWN;
> else if (anc_offset != 0)
> {
> tree new_binfo = get_binfo_at_offset (TYPE_BINFO (tci.known_current_type),
> anc_offset, TREE_TYPE (arg));
>
> if (new_binfo)
> {
> jfunc->type = IPA_JF_KNOWN_TYPE;
> jfunc->value.base_binfo = new_binfo;
> }
> else
> jfunc->type = IPA_JF_UNKNOWN;
> }
> else
> {
> /* If an offset of a searched-for sub-object actually happens to be zero,
> we do not have to worry about non-artificialness. Either there are no
> virtual methods anyway or there is a VMT pointer at offset zero and so
> artificial sub-objects start at higher offsets. */
> jfunc->type = IPA_JF_KNOWN_TYPE;
> jfunc->value.base_binfo = TYPE_BINFO (tci.known_current_type);
> }
>
> return true;
> }
>
> /* Like detect_type_change_anc but ARG is supposed to be a non-dereferenced
> pointer SSA name. */
>
> static bool
> detect_type_change (tree arg, gimple call, struct ipa_jump_func *jfunc)
> {
> if (!POINTER_TYPE_P (TREE_TYPE (arg))
> || TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) != RECORD_TYPE)
> return false;
> gcc_checking_assert (TREE_CODE (arg) != ADDR_EXPR);
Hm. I suppose the interfacing to this and the above function can
be improved to not need to build a mem-ref here - esp. as you seem
to handle &a explicitly in the caller.
More comments below at the callers (eventually)
> arg = build_simple_mem_ref (arg);
> /* Building a MEM_REF just for the sake of ao_ref_init is probably
> wasteful here, I'll be happy to discuss alternatives. */
>
> return detect_type_change_anc (arg, call, jfunc, 0);
> }
>
> ----------------------------------------------------------------------
>
> The patch implementing just the detection of changes with all the
> callers is below, the rest is in the next patch.
>
> I have bootstrapped and tested this patch separately (i.e. with the
> previous ones but not the subsequent ones) on x86-64-linux and it has
> also passed make check-c++ on i686.
>
> Thanks for any comments,
>
> Martin
>
>
> 2010-11-29 Martin Jambor <mjambor@suse.cz>
>
> PR tree-optimization/45934
> PR tree-optimization/46302
> * ipa-prop.c (type_change_info): New type.
> (stmt_may_be_vtbl_ptr_store): New function.
> (check_stmt_for_type_change): Likewise.
> (detect_type_change_1): Likewise.
> (detect_type_change): Likewise.
> (compute_complex_assign_jump_func): Check for dynamic type change.
> (compute_complex_ancestor_jump_func): Likewise.
> (compute_known_type_jump_func): Likewise.
> (compute_scalar_jump_functions): Likewise.
> (ipa_analyze_virtual_call_uses): Likewise.
>
> * testsuite/g++.dg/ipa/devirt-c-1.C: New test.
> * testsuite/g++.dg/ipa/devirt-c-2.C: Likewise.
> * testsuite/g++.dg/ipa/devirt-c-3.C: Likewise.
> * testsuite/g++.dg/ipa/devirt-c-4.C: Likewise.
> * testsuite/g++.dg/ipa/devirt-c-5.C: Likewise.
> * testsuite/g++.dg/ipa/devirt-d-1.C: Likewise.
> * testsuite/g++.dg/torture/pr45934.C: Likewise.
>
>
> Index: icln/gcc/ipa-prop.c
> ===================================================================
> --- icln.orig/gcc/ipa-prop.c
> +++ icln/gcc/ipa-prop.c
> @@ -350,6 +350,111 @@ ipa_print_all_jump_functions (FILE *f)
> }
> }
>
> +/* Structure to be passed in between detect_type_change_anc and
> + check_stmt_for_type_change. */
> +
> +struct type_change_info
> +{
> + /* Set to true if dynamic type change has been detected. */
> + bool type_maybe_changed;
> +};
> +
> +/* Return true if STMT can modify a virtual method table pointer. The function
> + assumes it will never be called on constructor or destructor calls (since
> + the search for dynamic type change will always end before reaching the
> + former and lifetime of the object is over anyway after the latter. */
> +
> +static bool
> +stmt_may_be_vtbl_ptr_store (gimple stmt)
> +{
> + if (is_gimple_call (stmt))
> + {
> + tree fndecl = gimple_call_fndecl (stmt);
> + if (!fndecl || !DECL_BUILT_IN (fndecl))
> + return false;
> + }
> + else if (is_gimple_assign (stmt))
> + {
> + tree lhs = gimple_assign_lhs (stmt);
> +
> + if (TREE_CODE (lhs) == COMPONENT_REF
> + && !DECL_VIRTUAL_P (TREE_OPERAND (lhs, 1)))
> + return false;
> + /* In the future we might want to use get_base_ref_and_offset to find
> + if there is a field corresponding to the offset and if so, proceed
> + almost like if it was a component ref. */
> + }
> + return true;
> +}
> +
> +/* Callbeck of walk_aliased_vdefs and a helper function for
> + detect_type_change_anc to check whether a particular statement may modify
> + the virtual table pointer, and if possible also determine the new type of
> + the (sub-)object. It stores its result into DATA, which points to a
> + type_change_info structure. */
> +
> +static bool
> +check_stmt_for_type_change (ao_ref *ao ATTRIBUTE_UNUSED, tree vdef, void *data)
> +{
> + gimple stmt = SSA_NAME_DEF_STMT (vdef);
> + struct type_change_info *tci = (struct type_change_info *) data;
> +
> + if (stmt_may_be_vtbl_ptr_store (stmt))
> + {
> + tci->type_maybe_changed = true;
> + return true;
> + }
> + else
> + return false;
> +}
> +
> +/* Detect whether the dynamic type of ARG 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. If
> + ANC_OFFSET is non-zero, look for binfo of its ancestor of that type at that
> + offset. ARG is supposed to be a dereferenced pointer or a declaration or a
> + series of component_refs of either. */
> +
> +static bool
> +detect_type_change_anc (tree arg, gimple call, struct ipa_jump_func *jfunc,
> + HOST_WIDE_INT anc_offset ATTRIBUTE_UNUSED)
> +{
> + struct type_change_info tci;
> + ao_ref ar;
> +
> + /* Const calls cannot call virtual methods through VMT and so type changes do
> + not matter. */
> + if (!gimple_vuse (call))
> + return false;
> +
> + tci.type_maybe_changed = false;
> +
> + ao_ref_init (&ar, arg);
> + walk_aliased_vdefs (&ar, gimple_vuse (call), check_stmt_for_type_change,
> + &tci, NULL);
> + if (!tci.type_maybe_changed)
> + return false;
> +
> + jfunc->type = IPA_JF_UNKNOWN;
> + return true;
> +}
> +
> +/* Like detect_type_change_anc but ARG is supposed to be a non-dereferenced
> + pointer SSA name. */
> +
> +static bool
> +detect_type_change (tree arg, gimple call, struct ipa_jump_func *jfunc)
> +{
> + if (!POINTER_TYPE_P (TREE_TYPE (arg))
> + || TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) != RECORD_TYPE)
> + return false;
> + gcc_checking_assert (TREE_CODE (arg) != ADDR_EXPR);
> + arg = build_simple_mem_ref (arg);
> +
> + return detect_type_change_anc (arg, call, jfunc, 0);
> +}
> +
> +
> /* Given that an actual argument is an SSA_NAME (given in NAME) and is a result
> of an assignment statement STMT, try to find out whether NAME can be
> described by a (possibly polynomial) pass-through jump-function or an
> @@ -359,10 +464,10 @@ ipa_print_all_jump_functions (FILE *f)
> static void
> compute_complex_assign_jump_func (struct ipa_node_params *info,
> struct ipa_jump_func *jfunc,
> - gimple stmt, tree name)
> + gimple call, gimple stmt, tree name)
> {
> HOST_WIDE_INT offset, size, max_size;
> - tree op1, op2, base, type;
> + tree op1, op2, base;
> int index;
>
> op1 = gimple_assign_rhs1 (stmt);
> @@ -388,7 +493,8 @@ compute_complex_assign_jump_func (struct
> jfunc->value.pass_through.operation = gimple_assign_rhs_code (stmt);
> jfunc->value.pass_through.operand = op2;
> }
> - else if (gimple_assign_unary_nop_p (stmt))
> + else if (gimple_assign_unary_nop_p (stmt)
> + && !detect_type_change (op1, call, jfunc))
Why's it only necessary to check for type changes when the statement
is a (pointer) conversion?
> {
> jfunc->type = IPA_JF_PASS_THROUGH;
> jfunc->value.pass_through.formal_id = index;
> @@ -399,10 +505,8 @@ compute_complex_assign_jump_func (struct
>
> if (TREE_CODE (op1) != ADDR_EXPR)
> return;
> -
Like pass the ADDR_EXPR (it seems that pass-by-value isn't handled)
down to a single detect_type_change function. You'd then use
ao_ref_init_from_ptr_and_size which is more correct anyway
(just pass NULL_TREE to the size, or if you know that the object
starts at the given address and the vtable pointer is at offset
zero pass TYPE_SIZE_UNIT (ptr_type_node)).
> op1 = TREE_OPERAND (op1, 0);
> - type = TREE_TYPE (op1);
> - if (TREE_CODE (type) != RECORD_TYPE)
> + if (TREE_CODE (TREE_TYPE (op1)) != RECORD_TYPE)
> return;
> base = get_ref_base_and_extent (op1, &offset, &size, &max_size);
> if (TREE_CODE (base) != MEM_REF
> @@ -419,12 +523,13 @@ compute_complex_assign_jump_func (struct
>
> /* Dynamic types are changed only in constructors and destructors and */
> index = ipa_get_param_decl_index (info, SSA_NAME_VAR (base));
> - if (index >= 0)
> + if (index >= 0
> + && !detect_type_change_anc (op1, call, jfunc, offset))
> {
> jfunc->type = IPA_JF_ANCESTOR;
> jfunc->value.ancestor.formal_id = index;
> jfunc->value.ancestor.offset = offset;
> - jfunc->value.ancestor.type = type;
> + jfunc->value.ancestor.type = TREE_TYPE (op1);
> }
> }
>
> @@ -453,12 +558,12 @@ compute_complex_assign_jump_func (struct
> static void
> compute_complex_ancestor_jump_func (struct ipa_node_params *info,
> struct ipa_jump_func *jfunc,
> - gimple phi)
> + gimple call, gimple phi)
> {
> HOST_WIDE_INT offset, size, max_size;
> gimple assign, cond;
> basic_block phi_bb, assign_bb, cond_bb;
> - tree tmp, parm, expr;
> + tree tmp, parm, expr, obj;
> int index, i;
>
> if (gimple_phi_num_args (phi) != 2)
> @@ -486,6 +591,7 @@ compute_complex_ancestor_jump_func (stru
> if (TREE_CODE (expr) != ADDR_EXPR)
> return;
> expr = TREE_OPERAND (expr, 0);
> + obj = expr;
> expr = get_ref_base_and_extent (expr, &offset, &size, &max_size);
>
> if (TREE_CODE (expr) != MEM_REF
> @@ -513,7 +619,6 @@ compute_complex_ancestor_jump_func (stru
> || !integer_zerop (gimple_cond_rhs (cond)))
> return;
>
> -
> phi_bb = gimple_bb (phi);
> for (i = 0; i < 2; i++)
> {
> @@ -522,10 +627,13 @@ compute_complex_ancestor_jump_func (stru
> return;
> }
>
> - jfunc->type = IPA_JF_ANCESTOR;
> - jfunc->value.ancestor.formal_id = index;
> - jfunc->value.ancestor.offset = offset;
> - jfunc->value.ancestor.type = TREE_TYPE (TREE_TYPE (tmp));
> + if (!detect_type_change_anc (obj, call, jfunc, offset))
works here as well.
> + {
> + jfunc->type = IPA_JF_ANCESTOR;
> + jfunc->value.ancestor.formal_id = index;
> + jfunc->value.ancestor.offset = offset;
> + jfunc->value.ancestor.type = TREE_TYPE (obj);;
> + }
> }
>
> /* Given OP whch is passed as an actual argument to a called function,
> @@ -533,7 +641,8 @@ compute_complex_ancestor_jump_func (stru
> and if so, create one and store it to JFUNC. */
>
> static void
> -compute_known_type_jump_func (tree op, struct ipa_jump_func *jfunc)
> +compute_known_type_jump_func (tree op, struct ipa_jump_func *jfunc,
> + gimple call)
> {
> HOST_WIDE_INT offset, size, max_size;
> tree base, binfo;
> @@ -551,6 +660,9 @@ compute_known_type_jump_func (tree op, s
> || is_global_var (base))
> return;
>
> + if (detect_type_change_anc (op, call, jfunc, offset))
> + return;
and here.
Thanks,
Richard.
> binfo = TYPE_BINFO (TREE_TYPE (base));
> if (!binfo)
> return;
> @@ -592,7 +704,8 @@ compute_scalar_jump_functions (struct ip
> {
> int index = ipa_get_param_decl_index (info, SSA_NAME_VAR (arg));
>
> - if (index >= 0)
> + if (index >= 0
> + && !detect_type_change (arg, call, &functions[num]))
> {
> functions[num].type = IPA_JF_PASS_THROUGH;
> functions[num].value.pass_through.formal_id = index;
> @@ -604,14 +717,14 @@ compute_scalar_jump_functions (struct ip
> gimple stmt = SSA_NAME_DEF_STMT (arg);
> if (is_gimple_assign (stmt))
> compute_complex_assign_jump_func (info, &functions[num],
> - stmt, arg);
> + call, stmt, arg);
> else if (gimple_code (stmt) == GIMPLE_PHI)
> compute_complex_ancestor_jump_func (info, &functions[num],
> - stmt);
> + call, stmt);
> }
> }
> else
> - compute_known_type_jump_func (arg, &functions[num]);
> + compute_known_type_jump_func (arg, &functions[num], call);
> }
> }
>
> @@ -1218,6 +1331,7 @@ ipa_analyze_virtual_call_uses (struct cg
> struct ipa_node_params *info, gimple call,
> tree target)
> {
> + struct ipa_jump_func jfunc;
> tree obj = OBJ_TYPE_REF_OBJECT (target);
> tree var;
> int index;
> @@ -1241,7 +1355,8 @@ ipa_analyze_virtual_call_uses (struct cg
> var = SSA_NAME_VAR (obj);
> index = ipa_get_param_decl_index (info, var);
>
> - if (index >= 0)
> + if (index >= 0
> + && !detect_type_change (obj, call, &jfunc))
> ipa_note_param_call (node, index, call, true);
> }
>
> Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-1.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-1.C
> @@ -0,0 +1,71 @@
> +/* Verify that ipa-cp correctly detects the dynamic type of an object
> + under construction when doing devirtualization. */
> +/* { dg-do run } */
> +/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */
> +
> +extern "C" void abort (void);
> +
> +class A
> +{
> +public:
> + int data;
> + A();
> + virtual int foo (int i);
> +};
> +
> +class B : public A
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +class C : public A
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +int A::foo (int i)
> +{
> + return i + 1;
> +}
> +
> +int B::foo (int i)
> +{
> + return i + 2;
> +}
> +
> +int C::foo (int i)
> +{
> + return i + 3;
> +}
> +
> +static int middleman (class A *obj, int i)
> +{
> + return obj->foo (i);
> +}
> +
> +int __attribute__ ((noinline,noclone)) get_input(void)
> +{
> + return 1;
> +}
> +
> +A::A ()
> +{
> + if (middleman (this, get_input ()) != 2)
> + abort ();
> +}
> +
> +static void bah ()
> +{
> + class B b;
> +}
> +
> +int main (int argc, char *argv[])
> +{
> + int i;
> +
> + for (i = 0; i < 10; i++)
> + bah ();
> + return 0;
> +}
> Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-2.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-2.C
> @@ -0,0 +1,79 @@
> +/* Verify that ipa-cp correctly detects the dynamic type of an object
> + under construction when doing devirtualization. */
> +/* { dg-do run } */
> +/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */
> +
> +extern "C" void abort (void);
> +
> +class Distraction
> +{
> +public:
> + float f;
> + double d;
> + Distraction ()
> + {
> + f = 8.3;
> + d = 10.2;
> + }
> + virtual float bar (float z);
> +};
> +
> +class A
> +{
> +public:
> + int data;
> + A();
> + virtual int foo (int i);
> +};
> +
> +class B : public Distraction, public A
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +float Distraction::bar (float z)
> +{
> + f += z;
> + return f/2;
> +}
> +
> +int A::foo (int i)
> +{
> + return i + 1;
> +}
> +
> +int B::foo (int i)
> +{
> + return i + 2;
> +}
> +
> +int __attribute__ ((noinline,noclone)) get_input(void)
> +{
> + return 1;
> +}
> +
> +static int middleman (class A *obj, int i)
> +{
> + return obj->foo (i);
> +}
> +
> +A::A()
> +{
> + if (middleman (this, get_input ()) != 2)
> + abort ();
> +}
> +
> +static void bah ()
> +{
> + class B b;
> +}
> +
> +int main (int argc, char *argv[])
> +{
> + int i;
> +
> + for (i = 0; i < 10; i++)
> + bah ();
> + return 0;
> +}
> Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-3.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-3.C
> @@ -0,0 +1,80 @@
> +/* Verify that ipa-cp correctly detects the dynamic type of an object
> + under construction when doing devirtualization. */
> +/* { dg-do run } */
> +/* { dg-options "-O3 -fno-inline" } */
> +
> +extern "C" void abort (void);
> +
> +class Distraction
> +{
> +public:
> + float f;
> + double d;
> + Distraction ()
> + {
> + f = 8.3;
> + d = 10.2;
> + }
> + virtual float bar (float z);
> +};
> +
> +class A
> +{
> +public:
> + int data;
> + A();
> + virtual int foo (int i);
> +};
> +
> +class B : public Distraction, public A
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +float Distraction::bar (float z)
> +{
> + f += z;
> + return f/2;
> +}
> +
> +int A::foo (int i)
> +{
> + return i + 1;
> +}
> +
> +int B::foo (int i)
> +{
> + return i + 2;
> +}
> +
> +int __attribute__ ((noinline,noclone)) get_input(void)
> +{
> + return 1;
> +}
> +
> +static int __attribute__ ((noinline))
> +middleman (class A *obj, int i)
> +{
> + return obj->foo (i);
> +}
> +
> +inline __attribute__ ((always_inline)) A::A()
> +{
> + if (middleman (this, get_input ()) != 2)
> + abort ();
> +}
> +
> +static void bah ()
> +{
> + class B b;
> +}
> +
> +int main (int argc, char *argv[])
> +{
> + int i;
> +
> + for (i = 0; i < 10; i++)
> + bah ();
> + return 0;
> +}
> Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-4.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-4.C
> @@ -0,0 +1,110 @@
> +/* Verify that ipa-cp correctly detects the dynamic type of an object
> + under construction when doing devirtualization. */
> +/* { dg-do run } */
> +/* { dg-options "-O3 -fno-inline" } */
> +
> +extern "C" void abort (void);
> +
> +class Distraction
> +{
> +public:
> + float f;
> + double d;
> + Distraction ()
> + {
> + f = 8.3;
> + d = 10.2;
> + }
> + virtual float bar (float z);
> +};
> +
> +class A
> +{
> +public:
> + int data;
> + A();
> + virtual int foo (int i);
> +};
> +
> +class B : public Distraction, public A
> +{
> +public:
> + B();
> + virtual int foo (int i);
> +};
> +
> +class C : public B
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +
> +float Distraction::bar (float z)
> +{
> + f += z;
> + return f/2;
> +}
> +
> +int A::foo (int i)
> +{
> + return i + 1;
> +}
> +
> +int B::foo (int i)
> +{
> + return i + 2;
> +}
> +
> +int C::foo (int i)
> +{
> + return i + 3;
> +}
> +
> +int __attribute__ ((noinline,noclone)) get_input(void)
> +{
> + return 1;
> +}
> +
> +static int __attribute__ ((noinline))
> +middleman (class A *obj, int i)
> +{
> + return obj->foo (i);
> +}
> +
> +static void __attribute__ ((noinline))
> +sth2 (A *a)
> +{
> + if (a->foo (get_input ()) != 3)
> + abort ();
> +}
> +
> +inline void __attribute__ ((always_inline)) sth1 (B *b)
> +{
> + sth2 (b);
> +}
> +
> +inline __attribute__ ((always_inline)) A::A()
> +{
> + if (middleman (this, get_input ()) != 2)
> + abort ();
> +}
> +
> +B::B() : Distraction(), A()
> +{
> + sth1 (this);
> +}
> +
> +static void bah ()
> +{
> + class C c;
> +}
> +
> +int main (int argc, char *argv[])
> +{
> + int i;
> +
> + for (i = 0; i < 10; i++)
> + bah ();
> + return 0;
> +}
> Index: icln/gcc/testsuite/g++.dg/ipa/devirt-d-1.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/ipa/devirt-d-1.C
> @@ -0,0 +1,71 @@
> +/* Verify that ipa-cp correctly detects the dynamic type of an object
> + under destruction when doing devirtualization. */
> +/* { dg-do run } */
> +/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */
> +
> +extern "C" void abort (void);
> +
> +class A
> +{
> +public:
> + int data;
> + ~A();
> + virtual int foo (int i);
> +};
> +
> +class B : public A
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +class C : public A
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +int A::foo (int i)
> +{
> + return i + 1;
> +}
> +
> +int B::foo (int i)
> +{
> + return i + 2;
> +}
> +
> +int C::foo (int i)
> +{
> + return i + 3;
> +}
> +
> +static int middleman (class A *obj, int i)
> +{
> + return obj->foo (i);
> +}
> +
> +int __attribute__ ((noinline,noclone)) get_input(void)
> +{
> + return 1;
> +}
> +
> +A::~A ()
> +{
> + if (middleman (this, get_input ()) != 2)
> + abort ();
> +}
> +
> +static void bah ()
> +{
> + class B b;
> +}
> +
> +int main (int argc, char *argv[])
> +{
> + int i;
> +
> + for (i = 0; i < 10; i++)
> + bah ();
> + return 0;
> +}
> Index: icln/gcc/testsuite/g++.dg/torture/pr45934.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/torture/pr45934.C
> @@ -0,0 +1,23 @@
> +/* { dg-do run } */
> +
> +extern "C" void abort ();
> +
> +struct B *b;
> +
> +struct B
> +{
> + virtual void f () { }
> + ~B() { b->f(); }
> +};
> +
> +struct D : public B
> +{
> + virtual void f () { abort (); }
> +};
> +
> +int main ()
> +{
> + D d;
> + b = &d;
> + return 0;
> +}
> Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-5.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-5.C
> @@ -0,0 +1,79 @@
> +/* Verify that ipa-cp correctly detects the dynamic type of an object
> + under construction when doing devirtualization. */
> +/* { dg-do run } */
> +/* { dg-options "-O3 -fno-early-inlining -fno-inline" } */
> +
> +extern "C" void abort (void);
> +
> +class B;
> +
> +class A
> +{
> +public:
> + int data;
> + A();
> + A(B *b);
> + virtual int foo (int i);
> +};
> +
> +class B : public A
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +class C : public A
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +int A::foo (int i)
> +{
> + return i + 1;
> +}
> +
> +int B::foo (int i)
> +{
> + return i + 2;
> +}
> +
> +int C::foo (int i)
> +{
> + return i + 3;
> +}
> +
> +static int middleman (class A *obj, int i)
> +{
> + return obj->foo (i);
> +}
> +
> +int __attribute__ ((noinline,noclone)) get_input(void)
> +{
> + return 1;
> +}
> +
> +A::A ()
> +{
> +}
> +
> +A::A (B *b)
> +{
> + if (middleman (b, get_input ()) != 3)
> + abort ();
> +}
> +
> +static void bah ()
> +{
> + B b;
> + A a(&b);
> +}
> +
> +int main (int argc, char *argv[])
> +{
> + int i;
> +
> + for (i = 0; i < 10; i++)
> + bah ();
> + return 0;
> +}
> Index: icln/gcc/testsuite/g++.dg/ipa/devirt-c-6.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/ipa/devirt-c-6.C
> @@ -0,0 +1,72 @@
> +/* Verify that ipa-cp correctly detects the dynamic type of an object
> + under construction when doing devirtualization. */
> +/* { dg-do run } */
> +/* { dg-options "-O3 -fno-inline" } */
> +
> +extern "C" void abort (void);
> +
> +class A
> +{
> +public:
> + int data;
> + A();
> + virtual int foo (int i);
> +};
> +
> +class B : public A
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +class C : public A
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +int A::foo (int i)
> +{
> + return i + 1;
> +}
> +
> +int B::foo (int i)
> +{
> + return i + 2;
> +}
> +
> +int C::foo (int i)
> +{
> + return i + 3;
> +}
> +
> +static inline int __attribute__ ((always_inline))
> +middleman (class A *obj, int i)
> +{
> + return obj->foo (i);
> +}
> +
> +int __attribute__ ((noinline,noclone)) get_input(void)
> +{
> + return 1;
> +}
> +
> +__attribute__ ((noinline)) A::A ()
> +{
> + if (middleman (this, get_input ()) != 2)
> + abort ();
> +}
> +
> +static void bah ()
> +{
> + class B b;
> +}
> +
> +int main (int argc, char *argv[])
> +{
> + int i;
> +
> + for (i = 0; i < 10; i++)
> + bah ();
> + return 0;
> +}
>
>
--
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 6/6] Intraprocedural type-based devirtualization
2010-12-01 20:23 ` [PATCH, PR 45934 6/6] Intraprocedural type-based devirtualization Martin Jambor
@ 2010-12-02 15:40 ` Richard Guenther
0 siblings, 0 replies; 29+ messages in thread
From: Richard Guenther @ 2010-12-02 15:40 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches, Jan Hubicka
On Wed, 1 Dec 2010, Martin Jambor wrote:
> This patch is in some ways still in the RFC shape because it certainly
> depends very much on how the previous ones will look like in the end
> and I'd like to discuss where this functionality should be placed. On
> the other hand, putting it into the cgraph rebuilding pass seems the
> best solution so far to me. It certainly looks better then when I
> tried to cram it into ipa-prop analysis phase, if not for any other
> reasons then because cfg cleanup might be necessary.
>
> Another reason is that I still want to try some more test-cases.
> However, I did not want to wait with the whole series any longer and
> so I am sending it out with the rest now.
>
> This patch compensates for not devirtualizing when folding. It looks
> for OBJ_TYPE_REF calls, and tries to deduce their type by watching out
> for dynamic type changes. If the type is known and the call can be
> converted to a direct one with that knowledge, it is done so.
>
> This patch is necessary for (modified variants of) testcases
> g++.dg/otr-fold-[12].C, g++.dg/tree-ssa/pr43411.C and
> g++.dg/tree-ssa/pr45605.C to pass again. Without it, early inlining
> can be detrimental to devirtualization by making it a purely
> intraprocedural issue and slow down simple testcases three-fold.
>
> Despite being an RFC patch, it also passes bootstrap and testing on
> x86_64-linux and make check-c++ on i686. I'm looking forward to all
> comments and suggestions.
>
> Thanks,
>
> Martin
>
>
>
> 2010-12-01 Martin Jambor <mjambor@suse.cz>
>
> * ipa-prop.c (get_ancestor_base_and_offset): New function.
> (compute_complex_assign_jump_func): Use it, check that op2 is NULL
> in the ancestor case.
> (compute_complex_ancestor_jump_func): Likewise.
> (ipa_try_devirtualize_immediately): New fuction.
> * cgraphbuild.c (rebuild_cgraph_edges): Renamed to
> rebuild_cgraph_edges_1, new parameter devirtualize, call
> ipa_try_devirtualize_immediately if it is true.
> (rebuild_cgraph_edges): New function.
> (rebuild_cgraph_edges_devirtualize): Likewise.
> (pass_rebuild_cgraph_edges_and_devirt): New variable.
> * passes.c (init_optimization_passes): Use it.
> * tree-pass.h (pass_rebuild_cgraph_edges_and_devirt): Declare.
> * ipa-prop.h (ipa_try_devirtualize_immediately): Declare.
>
> * testsuite/g++.dg/otr-fold-1.C: Moved to...
> * testsuite/g++.dg/ipa/imm-devirt-1.C: ...here, compiling with -O2.
> * testsuite/g++.dg/otr-fold-2.C: Moved to...
> * testsuite/g++.dg/ipa/imm-devirt-2.C: ...here, compiling with -O2.
> * testsuite/g++.dg/tree-ssa/pr45605.C: Compile with O2, scan optimized
> dump.
>
>
> Index: icln/gcc/ipa-prop.c
> ===================================================================
> --- icln.orig/gcc/ipa-prop.c
> +++ icln/gcc/ipa-prop.c
> @@ -539,6 +539,24 @@ detect_type_change (tree arg, gimple cal
> return detect_type_change_anc (arg, call, jfunc, 0);
> }
>
> +/* !!! Doc */
> +
> +static tree
> +get_ancestor_base_and_offset (tree op, HOST_WIDE_INT *offset)
> +{
> + HOST_WIDE_INT size, max_size;
> + tree base;
> +
> + base = get_ref_base_and_extent (op, offset, &size, &max_size);
> + if (TREE_CODE (base) != MEM_REF
> + /* If this is a varying address, punt. */
> + || max_size == -1
> + || max_size != size)
> + return NULL_TREE;
> + *offset += mem_ref_offset (base).low * BITS_PER_UNIT;
> + return TREE_OPERAND (base, 0);
> +}
> +
>
How's this different from get_addr_base_and_unit_offset? You'd have
the pointer-dereference check and the BITS_PER_UNIT multiplication
at the caller. At least I'd use get_addr_base_and_unit_offset above
as it is cheaper than get_ref_base_and_extent:
tree base = get_addr_base_and_unit_offset (op, offset);
if (!base
|| TREE_CODE (base) != MEM_REF)
return NULL_TREE;
*offset += mem_ref_offset (base).low;
*offset *= BITS_PER_UNIT;
return TREE_OPERAND (base, 0);
(in fact that looks more like a true get_addr_base_and_unit_offset
while the existing one is misnamed and should probably be
called get_ref_base[_and_unit_offset]).
> /* Given that an actual argument is an SSA_NAME (given in NAME) and is a result
> of an assignment statement STMT, try to find out whether NAME can be
> @@ -551,7 +569,7 @@ compute_complex_assign_jump_func (struct
> struct ipa_jump_func *jfunc,
> gimple call, gimple stmt, tree name)
> {
> - HOST_WIDE_INT offset, size, max_size;
> + HOST_WIDE_INT offset;
> tree op1, op2, base;
> int index;
>
> @@ -588,20 +606,14 @@ compute_complex_assign_jump_func (struct
> return;
> }
>
> - if (TREE_CODE (op1) != ADDR_EXPR)
> + if (op2 || TREE_CODE (op1) != ADDR_EXPR)
> return;
> op1 = TREE_OPERAND (op1, 0);
> if (TREE_CODE (TREE_TYPE (op1)) != RECORD_TYPE)
> return;
> - base = get_ref_base_and_extent (op1, &offset, &size, &max_size);
> - if (TREE_CODE (base) != MEM_REF
> - /* If this is a varying address, punt. */
> - || max_size == -1
> - || max_size != size)
> - return;
> - offset += mem_ref_offset (base).low * BITS_PER_UNIT;
> - base = TREE_OPERAND (base, 0);
> - if (TREE_CODE (base) != SSA_NAME
> + base = get_ancestor_base_and_offset (op1, &offset);
> + if (!base
> + || TREE_CODE (base) != SSA_NAME
> || !SSA_NAME_IS_DEFAULT_DEF (base)
> || offset < 0)
> return;
> @@ -645,10 +657,10 @@ compute_complex_ancestor_jump_func (stru
> struct ipa_jump_func *jfunc,
> gimple call, gimple phi)
> {
> - HOST_WIDE_INT offset, size, max_size;
> + HOST_WIDE_INT offset;
> gimple assign, cond;
> basic_block phi_bb, assign_bb, cond_bb;
> - tree tmp, parm, expr, obj;
> + tree tmp, parm, expr;
> int index, i;
>
> if (gimple_phi_num_args (phi) != 2)
> @@ -676,17 +688,9 @@ compute_complex_ancestor_jump_func (stru
> if (TREE_CODE (expr) != ADDR_EXPR)
> return;
> expr = TREE_OPERAND (expr, 0);
> - obj = expr;
> - expr = get_ref_base_and_extent (expr, &offset, &size, &max_size);
> -
> - if (TREE_CODE (expr) != MEM_REF
> - /* If this is a varying address, punt. */
> - || max_size == -1
> - || max_size != size)
> - return;
> - offset += mem_ref_offset (expr).low * BITS_PER_UNIT;
> - parm = TREE_OPERAND (expr, 0);
> - if (TREE_CODE (parm) != SSA_NAME
> + parm = get_ancestor_base_and_offset (expr, &offset);
> + if (!parm
> + || TREE_CODE (parm) != SSA_NAME
> || !SSA_NAME_IS_DEFAULT_DEF (parm)
> || offset < 0)
> return;
> @@ -712,12 +716,12 @@ compute_complex_ancestor_jump_func (stru
> return;
> }
>
> - if (!detect_type_change_anc (obj, call, jfunc, offset))
> + if (!detect_type_change_anc (expr, call, jfunc, offset))
> {
> jfunc->type = IPA_JF_ANCESTOR;
> jfunc->value.ancestor.formal_id = index;
> jfunc->value.ancestor.offset = offset;
> - jfunc->value.ancestor.type = TREE_TYPE (obj);;
> + jfunc->value.ancestor.type = TREE_TYPE (expr);;
> }
> }
>
> @@ -1407,6 +1411,77 @@ ipa_analyze_indirect_call_uses (struct c
> return;
> }
>
> +/* If a call to an OBJ_TYPE_REF given by GSI can be turned into a direct one
> + according to the type of its object right away and if so, do it and return
> + true. If CFG is changed in the process, *CFG_CHANGED is set to true. */
> +
> +tree
> +ipa_try_devirtualize_immediately (gimple_stmt_iterator *gsi, bool *cfg_changed)
> +{
> + struct ipa_jump_func jfunc;
> + tree obj, fndecl, delta, token;
> + gimple call = gsi_stmt (*gsi);
> + tree target = gimple_call_fn (call);
> +
> + if (TREE_CODE (target) != OBJ_TYPE_REF)
> + return NULL_TREE;
fold-stmt will still fold calls with OBJ_TYPE_REF_EXPR being a
&function-decl, right? (But I can't find that it does so)
De-virtualization for that case seems wasteful, so maybe assert
that OBJ_TYPE_REF_EXPR is an SSA name here (or bail out).
> + jfunc.type = IPA_JF_UNKNOWN;
> + obj = OBJ_TYPE_REF_OBJECT (target);
> + if (TREE_CODE (obj) == SSA_NAME)
> + {
> + HOST_WIDE_INT offset;
> + tree op, base;
> + gimple stmt = SSA_NAME_DEF_STMT (obj);
> + if (!stmt || !is_gimple_assign (stmt) || gimple_assign_rhs2 (stmt))
> + return NULL_TREE;
> + op = gimple_assign_rhs1 (stmt);
> + if (TREE_CODE (op) != ADDR_EXPR)
> + return NULL_TREE;
> + op = TREE_OPERAND (op, 0);
> + if (TREE_CODE (TREE_TYPE (op)) != RECORD_TYPE)
> + return NULL_TREE;
> + base = get_ancestor_base_and_offset (op, &offset);
> + if (!base || offset < 0)
> + return NULL_TREE;
> + detect_type_change_anc (op, call, &jfunc, offset);
> + }
> + else
> + compute_known_type_jump_func (obj, &jfunc, call);
> + if (jfunc.type != IPA_JF_KNOWN_TYPE)
> + return NULL_TREE;
> +
> + token = OBJ_TYPE_REF_TOKEN (target);
> + fndecl = gimple_get_virt_mehtod_for_binfo (tree_low_cst (token, 1),
Check for host_integerp (token, 1) first or use TREE_INT_CST_LOW
if you know that's always the case.
Otherwise this looks mostly ok, I'll defer to Honza if this is a good
place to do this (I'd say inlining itself would be a good place as well,
before or during early inlining).
Thanks,
Richard.
> + jfunc.value.base_binfo,
> + &delta, true);
> + if (!fndecl)
> + return NULL_TREE;
> + if (dump_file)
> + {
> + fprintf (dump_file, "ipa-prop: Immediately devirtualizing call ");
> + print_gimple_expr (dump_file, call, 0, 0);
> + }
> +
> + if (integer_nonzerop (delta))
> + gimple_adjust_this_by_delta (gsi, delta);
> + gimple_call_set_fndecl (call, fndecl);
> + update_stmt (call);
> + if (maybe_clean_eh_stmt (call)
> + && gimple_purge_dead_eh_edges (gimple_bb (call)))
> + *cfg_changed = true;
> +
> + if (dump_file)
> + {
> + fprintf (dump_file, "\n into ");
> + print_gimple_expr (dump_file, call, 0, 0);
> + fprintf (dump_file, "\n with delta ");
> + print_generic_expr (dump_file, delta, 0);
> + fprintf (dump_file, "\n");
> + }
> +
> + return fndecl;
> +}
> +
> /* Analyze a CALL to an OBJ_TYPE_REF which is passed in TARGET and if the
> object referenced in the expression is a formal parameter of the caller
> (described by INFO), create a call note for the statement. */
> Index: icln/gcc/testsuite/g++.dg/ipa/imm-devirt-1.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/ipa/imm-devirt-1.C
> @@ -0,0 +1,76 @@
> +/* Verify that virtual calls are folded even when a typecast to an
> + ancestor is involved along the way. */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fdump-tree-optimized-slim" } */
> +
> +extern "C" void abort (void);
> +
> +class Distraction
> +{
> +public:
> + float f;
> + double d;
> + Distraction ()
> + {
> + f = 8.3;
> + d = 10.2;
> + }
> + virtual float bar (float z);
> +};
> +
> +class A
> +{
> +public:
> + int data;
> + virtual int foo (int i);
> +};
> +
> +
> +class B : public Distraction, public A
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +float Distraction::bar (float z)
> +{
> + f += z;
> + return f/2;
> +}
> +
> +int __attribute__ ((noinline)) A::foo (int i)
> +{
> + return i + 1;
> +}
> +
> +int __attribute__ ((noinline)) B::foo (int i)
> +{
> + return i + 2;
> +}
> +
> +int __attribute__ ((noinline,noclone)) get_input(void)
> +{
> + return 1;
> +}
> +
> +static inline int middleman_1 (class A *obj, int i)
> +{
> + return obj->foo (i);
> +}
> +
> +static inline int middleman_2 (class B *obj, int i)
> +{
> + return middleman_1 (obj, i);
> +}
> +
> +int main (int argc, char *argv[])
> +{
> + class B b;
> +
> + if (middleman_2 (&b, get_input ()) != 3)
> + abort ();
> + return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump "= B::.*foo" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
> Index: icln/gcc/testsuite/g++.dg/ipa/imm-devirt-2.C
> ===================================================================
> --- /dev/null
> +++ icln/gcc/testsuite/g++.dg/ipa/imm-devirt-2.C
> @@ -0,0 +1,88 @@
> +/* Verify that virtual calls are folded even when a typecast to an
> + ancestor is involved along the way. */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -fdump-tree-optimized-slim" } */
> +
> +extern "C" void abort (void);
> +
> +class Distraction
> +{
> +public:
> + float f;
> + double d;
> + Distraction ()
> + {
> + f = 8.3;
> + d = 10.2;
> + }
> + virtual float bar (float z);
> +};
> +
> +class A
> +{
> +public:
> + int data;
> + virtual int foo (int i);
> +};
> +
> +class A_2 : public A
> +{
> +public:
> + int data_2;
> + virtual int baz (int i);
> +};
> +
> +
> +class B : public Distraction, public A_2
> +{
> +public:
> + virtual int foo (int i);
> +};
> +
> +float __attribute__ ((noinline)) Distraction::bar (float z)
> +{
> + f += z;
> + return f/2;
> +}
> +
> +int __attribute__ ((noinline)) A::foo (int i)
> +{
> + return i + 1;
> +}
> +
> +int __attribute__ ((noinline)) A_2::baz (int i)
> +{
> + return i * 15;
> +}
> +
> +int __attribute__ ((noinline)) B::foo (int i)
> +{
> + return i + 2;
> +}
> +
> +int __attribute__ ((noinline,noclone)) get_input(void)
> +{
> + return 1;
> +}
> +
> +static inline int middleman_1 (class A *obj, int i)
> +{
> + return obj->foo (i);
> +}
> +
> +static inline int middleman_2 (class A *obj, int i)
> +{
> + return middleman_1 (obj, i);
> +}
> +
> +int main (int argc, char *argv[])
> +{
> + class B b;
> +
> + if (middleman_2 (&b, get_input ()) != 3)
> + abort ();
> + return 0;
> +}
> +
> +/* { dg-final { scan-tree-dump "= B::.*foo" "optimized" } } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
> Index: icln/gcc/testsuite/g++.dg/otr-fold-1.C
> ===================================================================
> --- icln.orig/gcc/testsuite/g++.dg/otr-fold-1.C
> +++ /dev/null
> @@ -1,76 +0,0 @@
> -/* Verify that virtual calls are folded even when a typecast to an
> - ancestor is involved along the way. */
> -/* { dg-do run } */
> -/* { dg-options "-O -fdump-tree-optimized-slim" } */
> -
> -extern "C" void abort (void);
> -
> -class Distraction
> -{
> -public:
> - float f;
> - double d;
> - Distraction ()
> - {
> - f = 8.3;
> - d = 10.2;
> - }
> - virtual float bar (float z);
> -};
> -
> -class A
> -{
> -public:
> - int data;
> - virtual int foo (int i);
> -};
> -
> -
> -class B : public Distraction, public A
> -{
> -public:
> - virtual int foo (int i);
> -};
> -
> -float Distraction::bar (float z)
> -{
> - f += z;
> - return f/2;
> -}
> -
> -int A::foo (int i)
> -{
> - return i + 1;
> -}
> -
> -int B::foo (int i)
> -{
> - return i + 2;
> -}
> -
> -int __attribute__ ((noinline,noclone)) get_input(void)
> -{
> - return 1;
> -}
> -
> -static inline int middleman_1 (class A *obj, int i)
> -{
> - return obj->foo (i);
> -}
> -
> -static inline int middleman_2 (class B *obj, int i)
> -{
> - return middleman_1 (obj, i);
> -}
> -
> -int main (int argc, char *argv[])
> -{
> - class B b;
> -
> - if (middleman_2 (&b, get_input ()) != 3)
> - abort ();
> - return 0;
> -}
> -
> -/* { dg-final { scan-tree-dump "= B::.*foo" "optimized" } } */
> -/* { dg-final { cleanup-tree-dump "optimized" } } */
> Index: icln/gcc/testsuite/g++.dg/otr-fold-2.C
> ===================================================================
> --- icln.orig/gcc/testsuite/g++.dg/otr-fold-2.C
> +++ /dev/null
> @@ -1,88 +0,0 @@
> -/* Verify that virtual calls are folded even when a typecast to an
> - ancestor is involved along the way. */
> -/* { dg-do run } */
> -/* { dg-options "-O -fdump-tree-optimized-slim" } */
> -
> -extern "C" void abort (void);
> -
> -class Distraction
> -{
> -public:
> - float f;
> - double d;
> - Distraction ()
> - {
> - f = 8.3;
> - d = 10.2;
> - }
> - virtual float bar (float z);
> -};
> -
> -class A
> -{
> -public:
> - int data;
> - virtual int foo (int i);
> -};
> -
> -class A_2 : public A
> -{
> -public:
> - int data_2;
> - virtual int baz (int i);
> -};
> -
> -
> -class B : public Distraction, public A_2
> -{
> -public:
> - virtual int foo (int i);
> -};
> -
> -float Distraction::bar (float z)
> -{
> - f += z;
> - return f/2;
> -}
> -
> -int A::foo (int i)
> -{
> - return i + 1;
> -}
> -
> -int A_2::baz (int i)
> -{
> - return i * 15;
> -}
> -
> -int B::foo (int i)
> -{
> - return i + 2;
> -}
> -
> -int __attribute__ ((noinline,noclone)) get_input(void)
> -{
> - return 1;
> -}
> -
> -static inline int middleman_1 (class A *obj, int i)
> -{
> - return obj->foo (i);
> -}
> -
> -static inline int middleman_2 (class A *obj, int i)
> -{
> - return middleman_1 (obj, i);
> -}
> -
> -int main (int argc, char *argv[])
> -{
> - class B b;
> -
> - if (middleman_2 (&b, get_input ()) != 3)
> - abort ();
> - return 0;
> -}
> -
> -/* { dg-final { scan-tree-dump "= B::.*foo" "optimized" } } */
> -/* { dg-final { cleanup-tree-dump "optimized" } } */
> Index: icln/gcc/testsuite/g++.dg/tree-ssa/pr45605.C
> ===================================================================
> --- icln.orig/gcc/testsuite/g++.dg/tree-ssa/pr45605.C
> +++ icln/gcc/testsuite/g++.dg/tree-ssa/pr45605.C
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-options "-O1 -fdump-tree-ssa" } */
> +/* { dg-options "-O2 -fdump-tree-optimized" } */
> extern "C" void abort();
> bool destructor_called = false;
>
> @@ -33,5 +33,5 @@ int main() {
>
>
> /* We should devirtualize call to D::Run */
> -/* { dg-final { scan-tree-dump-times "D::Run \\(" 1 "ssa"} } */
> -/* { dg-final { cleanup-tree-dump "ssa" } } */
> +/* { dg-final { scan-tree-dump-times "D::Run \\(" 1 "optimized"} } */
> +/* { dg-final { cleanup-tree-dump "optimized" } } */
> Index: icln/gcc/cgraphbuild.c
> ===================================================================
> --- icln.orig/gcc/cgraphbuild.c
> +++ icln/gcc/cgraphbuild.c
> @@ -33,6 +33,9 @@ along with GCC; see the file COPYING3.
> #include "tree-pass.h"
> #include "ipa-utils.h"
> #include "except.h"
> +#include "ipa-prop.h"
> +
> +/* !!! Add ipa-prop.h to dependencies in Makefile.in */
>
> /* Context of record_reference. */
> struct record_reference_ctx
> @@ -429,12 +432,13 @@ record_references_in_initializer (tree d
> /* Rebuild cgraph edges for current function node. This needs to be run after
> passes that don't update the cgraph. */
>
> -unsigned int
> -rebuild_cgraph_edges (void)
> +static unsigned int
> +rebuild_cgraph_edges_1 (bool devirtualize)
> {
> basic_block bb;
> struct cgraph_node *node = cgraph_node (current_function_decl);
> gimple_stmt_iterator gsi;
> + bool cfg_changed = false;
>
> cgraph_node_remove_callees (node);
> ipa_remove_all_references (&node->ref_list);
> @@ -453,6 +457,8 @@ rebuild_cgraph_edges (void)
> int freq = compute_call_stmt_bb_frequency (current_function_decl,
> bb);
> decl = gimple_call_fndecl (stmt);
> + if (!decl && devirtualize)
> + decl = ipa_try_devirtualize_immediately (&gsi, &cfg_changed);
> if (decl)
> cgraph_create_edge (node, cgraph_node (decl), stmt,
> bb->count, freq,
> @@ -474,7 +480,26 @@ rebuild_cgraph_edges (void)
> record_eh_tables (node, cfun);
> gcc_assert (!node->global.inlined_to);
>
> - return 0;
> + if (cfg_changed)
> + return TODO_cleanup_cfg;
> + else
> + return 0;
> +}
> +
> +/* !!!Doc */
> +
> +unsigned int
> +rebuild_cgraph_edges (void)
> +{
> + return rebuild_cgraph_edges_1 (false);
> +}
> +
> +/* !!!Doc */
> +
> +static unsigned int
> +rebuild_cgraph_edges_devirtualize (void)
> +{
> + return rebuild_cgraph_edges_1 (true);
> }
>
> /* Rebuild cgraph edges for current function node. This needs to be run after
> @@ -518,6 +543,25 @@ struct gimple_opt_pass pass_rebuild_cgra
> NULL, /* sub */
> NULL, /* next */
> 0, /* static_pass_number */
> + TV_CGRAPH, /* tv_id */
> + PROP_cfg, /* properties_required */
> + 0, /* properties_provided */
> + 0, /* properties_destroyed */
> + 0, /* todo_flags_start */
> + 0, /* todo_flags_finish */
> + }
> +};
> +
> +struct gimple_opt_pass pass_rebuild_cgraph_edges_and_devirt =
> +{
> + {
> + GIMPLE_PASS,
> + "*rebuild_cgraph_edges_d", /* name */
> + NULL, /* gate */
> + rebuild_cgraph_edges_devirtualize, /* execute */
> + NULL, /* sub */
> + NULL, /* next */
> + 0, /* static_pass_number */
> TV_CGRAPH, /* tv_id */
> PROP_cfg, /* properties_required */
> 0, /* properties_provided */
> Index: icln/gcc/ipa-prop.h
> ===================================================================
> --- icln.orig/gcc/ipa-prop.h
> +++ icln/gcc/ipa-prop.h
> @@ -429,6 +429,9 @@ void ipa_initialize_node_params (struct
> bool ipa_propagate_indirect_call_infos (struct cgraph_edge *cs,
> VEC (cgraph_edge_p, heap) **new_edges);
>
> +/* Intraprocedural devirtualization. */
> +tree ipa_try_devirtualize_immediately (gimple_stmt_iterator *, bool *);
> +
> /* Indirect edge and binfo processing. */
> struct cgraph_edge *ipa_make_edge_direct_to_target (struct cgraph_edge *, tree,
> tree);
> Index: icln/gcc/passes.c
> ===================================================================
> --- icln.orig/gcc/passes.c
> +++ icln/gcc/passes.c
> @@ -791,7 +791,7 @@ init_optimization_passes (void)
> NEXT_PASS (pass_split_functions);
> }
> NEXT_PASS (pass_release_ssa_names);
> - NEXT_PASS (pass_rebuild_cgraph_edges);
> + NEXT_PASS (pass_rebuild_cgraph_edges_and_devirt);
> NEXT_PASS (pass_inline_parameters);
> }
> NEXT_PASS (pass_ipa_tree_profile);
> Index: icln/gcc/tree-pass.h
> ===================================================================
> --- icln.orig/gcc/tree-pass.h
> +++ icln/gcc/tree-pass.h
> @@ -436,6 +436,7 @@ extern struct gimple_opt_pass pass_uncpr
> extern struct gimple_opt_pass pass_return_slot;
> extern struct gimple_opt_pass pass_reassoc;
> extern struct gimple_opt_pass pass_rebuild_cgraph_edges;
> +extern struct gimple_opt_pass pass_rebuild_cgraph_edges_and_devirt;
> extern struct gimple_opt_pass pass_remove_cgraph_callee_edges;
> extern struct gimple_opt_pass pass_build_cgraph_edges;
> extern struct gimple_opt_pass pass_local_pure_const;
>
>
--
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 4/6] Dynamic type change detection
2010-12-02 15:19 ` Richard Guenther
@ 2010-12-02 16:17 ` Richard Guenther
2010-12-09 11:30 ` Martin Jambor
0 siblings, 1 reply; 29+ messages in thread
From: Richard Guenther @ 2010-12-02 16:17 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches, Jan Hubicka
On Thu, 2 Dec 2010, Richard Guenther wrote:
> On Wed, 1 Dec 2010, Martin Jambor wrote:
>
> > This is the crux of the matter. I'll try to explain what and why I do
> > in order to detect sub-object type changes during their construction
> > first, below it there is the detection code as it looks like with the
> > next patch applied and then there is of course the patch at the end.
> >
> > I have split the detection code into two patches, this one just
> > detects that there is a type change in force and the subsequent one
> > also tries to derive the new type. I did this in order to better
> > structure the discussion about both and I also intend to commit the
> > separately, easing potential bisecting (I hope there will be no reason
> > for that of course). If it was absolutely necessary, we could
> > postpone the next patch for stage1 but I hope to commit a variant of
> > it soon.
> >
> > Because operations like placement new, memcpy and other byte-per-byte
> > operations with objects that have virtual methods are deemed to
> > produce code with undefined behavior and there are no unions of
> > non-POD types, dynamic type change has a special meaning for
> > devirtualization and only refers to what actual virtual method table a
> > VMT pointer of an object points to. On the other hand the type in the
> > sense of what you get from TREE_TYPE of the base of any given access
> > to it is still the same and it is its static type. An important
> > property of such objects is that their dynamic types (as given by the
> > current VMT) can only be altered in constructors and destructors.
> >
> > This patch makes special assumptions about both constructors and
> > destructors which are all the functions that are allowed to alter the
> > dynamic types. It assumes that destructors begin with assignment into
> > all VMT pointers and that constructors essentially look in the
> > following way:
> >
> > 1) The very first thing they do is that they call constructors of the
> > components (including ancestor sub-objects) that have them.
> >
> > 2) Then VMT pointers of this and all its ancestors is set to new
> > values corresponding to the type corresponding to the constructor.
> >
> > 3) Only afterwards, other stuff such as the code written by the user
> > is run. Only this may include calling virtual functions, directly
> > or indirectly.
> >
> > There is no way to call a constructor of an ancestor sub-object in any
> > other way (or any component for that matter but type is interesting
> > only for ancestors).
> >
> > This means that we do not have to care whether constructors get the
> > correct type information because they will always change it (in fact,
> > if we define the type to be given by the VMT pointer, it is
> > undefined).
You can still reach wrong conclusions for the dynamic type of *p
in the following testcase:
extern "C" void abort (void);
extern "C" void *malloc(__SIZE_TYPE__);
inline void* operator new(__SIZE_TYPE__, void* __p) throw() { return __p;
}
int x;
class A {
public:
virtual ~A() { }
};
class B : public A {
public:
virtual ~B() { if (x == 1) abort (); x = 1; }
};
void __attribute__((noinline,noclone)) foo (void *p)
{
B *b = reinterpret_cast<B *>(p);
b->~B();
new (p) A;
}
int main()
{
void *p = __builtin_malloc (sizeof (B));
new (p) B;
foo(p);
reinterpret_cast<A *>(p)->~A();
return 0;
}
if you ignore the call to foo() then you see a vtable pointer store
to make *p a B, but in reality at the time of the virtual call to
the destructor it is an A.
Similar, if you add a char data[256]; member to A then a
{
B x;
new (&x.data) A;
reinterpret_cast<A *>(&x.data)->~A;
}
might possibly mis-lead you as to the type of B at destruction time
(this example is more similar to the array issue I raised).
Richard.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 4/6] Dynamic type change detection
2010-12-01 20:23 ` [PATCH, PR 45934 4/6] Dynamic type change detection Martin Jambor
2010-12-02 15:19 ` Richard Guenther
@ 2010-12-02 23:25 ` Jason Merrill
2010-12-03 13:45 ` Martin Jambor
1 sibling, 1 reply; 29+ messages in thread
From: Jason Merrill @ 2010-12-02 23:25 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches, Richard Guenther, Jan Hubicka
On 12/01/2010 03:16 PM, Martin Jambor wrote:
> and there are no unions of non-POD types
This is actually no longer true in C++0x.
> dynamic types. It assumes that destructors begin with assignment into
> all VMT pointers and that constructors essentially look in the
> following way:
>
> 1) The very first thing they do is that they call constructors of the
> components (including ancestor sub-objects) that have them.
> 2) Then VMT pointers of this and all its ancestors is set to new
> values corresponding to the type corresponding to the constructor.
Slight clarification: this happens after calling constructors for base
sub-objects and before calling constructors for member sub-objects.
The rest of the writeup all sounds correct.
Jason
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks
2010-12-01 20:58 ` Jan Hubicka
@ 2010-12-03 13:01 ` Martin Jambor
2010-12-14 17:39 ` Jan Hubicka
0 siblings, 1 reply; 29+ messages in thread
From: Martin Jambor @ 2010-12-03 13:01 UTC (permalink / raw)
To: Jan Hubicka; +Cc: GCC Patches
Hi,
On Wed, Dec 01, 2010 at 09:58:08PM +0100, Jan Hubicka wrote:
> > 2010-11-08 Martin Jambor <mjambor@suse.cz>
> >
> > PR tree-optimization/46053
> > PR middle-end/46287
> > * cgraph.h (cgraph_indirect_call_info): New field.
> > * gimple.h (gimple_fold_obj_type_ref): Declaration removed.
> > (gimple_fold_call): Declare.
> > (gimple_adjust_this_by_delta): Likewise.
> > * cgraph.c (cgraph_make_edge_direct): New parameter delta. Updated
> > all users.
> > (cgraph_clone_edge): Create a copy of indirect_info also for direct
> > edges.
> > * cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Adjust this
> > parameters.
> > * gimple-fold.c (gimple_fold_obj_type_ref_known_binfo): Renamed to
> > gimple_get_virt_mehtod_for_binfo, new parameter delta. Do not search
> > through thunks, in fact bail out if we encounter one, check that
> > BINFO_VIRTUALS is not NULL.
> > (gimple_adjust_this_by_delta): New function.
> > (gimple_fold_obj_type_ref): Removed.
> > (gimple_fold_obj_type_ref_call): New function.
> > (fold_gimple_call): Renamed to gimple_fold_call, made external.
> > Updated users. Call gimple_fold_obj_type_ref_call instead of
> > gimple_fold_obj_type_ref.
> > * ipa-cp.c (ipcp_process_devirtualization_opportunities): Process
> > thunk deltas.
> > (ipcp_discover_new_direct_edges): Likewise.
> > * ipa-prop.c (ipa_make_edge_direct_to_target): New parameter delta.
> > Updated callers.
> > (ipa_write_indirect_edge_info): Stream thunk_delta.
> > (ipa_read_indirect_edge_info): Likewise.
> > * tree-ssa-ccp.c (ccp_fold_stmt): Use gimple_fold_call instead of
> > gimple_fold_obj_type_ref.
> >
> > * testsuite/g++.dg/ipa/pr46053.C: New test.
> > * testsuite/g++.dg/ipa/pr46287-1.C: Likewise.
> > * testsuite/g++.dg/ipa/pr46287-2.C: Likewise.
> > * testsuite/g++.dg/ipa/pr46287-3.C: Likewise.
> > * testsuite/g++.dg/torture/covariant-1.C: Likewise.
> > * testsuite/g++.dg/torture/pr46287.C: Likewise.
> >
>
> > /* Make an indirect EDGE with an unknown callee an ordinary edge leading to
> > - CALLEE. */
> > + CALLEE. DELTA, if non-NULL, is an integer constant that is to be added to
> > + the this pointer (first parameter). */
> >
> > void
> > -cgraph_make_edge_direct (struct cgraph_edge *edge, struct cgraph_node *callee)
> > +cgraph_make_edge_direct (struct cgraph_edge *edge, struct cgraph_node *callee,
> > + tree delta)
> > {
> > edge->indirect_unknown_callee = 0;
> > + edge->indirect_info->thunk_delta = delta;
>
> I guess it is still fine as it is definite improvement over current
> situation, but won't we need to handle all of cgraph_thunk_info
> here?
Eventually we will, certainly before we start propagating constants to
zeroth arguments of OBJ_TYPE_REFs. And not only here but also at
other places (e.g. cgraph_set_call_stmt and company... and of course
there is the weird cgraph_create_edge_including_clones I wrote you
about earlier).
> In thunk_info it is HOST_WIDE_INT, I would expect it to be here as well.
I extract it from BINFOs as tree constants and I need to use it in
POINTER_PLUSes as tree constants so there is probably no point in
striing it as H_W_I.
>
> > + if (e->indirect_info && e->indirect_info->thunk_delta
> > + && integer_nonzerop (e->indirect_info->thunk_delta))
> > + {
> > + if (cgraph_dump_file)
> > + {
> > + fprintf (cgraph_dump_file, " Thunk delta is ");
> > + print_generic_expr (cgraph_dump_file,
> > + e->indirect_info->thunk_delta, 0);
> > + fprintf (cgraph_dump_file, "\n");
> > + }
> > + gsi = gsi_for_stmt (e->call_stmt);
> > + gsi_computed = true;
> > + gimple_adjust_this_by_delta (&gsi, e->indirect_info->thunk_delta);
> > + e->indirect_info->thunk_delta = NULL_TREE;
> > + }
>
> Should've test here if this parameter was eliminated or not?
> (i.e. first bit of e->callee->clone.combined_args_to_skip?
It is not strictly necessary because I do the callee modification
before removing parameters but on the second thought, yes, we should
avoid creating unnecessary stuff when it is this easy. So I will add
the check and the condition will be:
if (e->indirect_info && e->indirect_info->thunk_delta
&& integer_nonzerop (e->indirect_info->thunk_delta)
&& (!e->callee->clone.combined_args_to_skip
|| !bitmap_bit_p (e->callee->clone.combined_args_to_skip, 0)))
>
> > Index: icln/gcc/ipa-prop.c
> > ===================================================================
> > --- icln.orig/gcc/ipa-prop.c
> > +++ icln/gcc/ipa-prop.c
>
> I would expect somewhere here to be code handling updating of operand
> value for non-0 delta when doing ipa-cp propagation, but don't seem to
> be able to find it?
What do you mean? IPA-CP stores the delta into the indirect info
structure of the corresponding edge and then the code in
gimple_adjust_this_by_delta performs the adjustment.
>
> The rest seems OK. I am most concerned that we implement just part of thunk
> logic, but I see that you get deltas from BINFOs and the rest of adjustments
> are not there?
>
Correct, for simple this adjusting thunks, BINFOs contain the decl of
the real function and the this delta separately, for more complex
thunks it stores the decl of an underlying thunk + a this delta which
would otherwise be adjusted in this thunk.
And yes, we will certainly have to re-think how to represent thunks in
the cgraph in a more general way.
Anyway, is the patch OK with the above change then?
Thanks,
Martin
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 4/6] Dynamic type change detection
2010-12-02 23:25 ` Jason Merrill
@ 2010-12-03 13:45 ` Martin Jambor
2010-12-03 14:34 ` Gabriel Dos Reis
2010-12-03 16:07 ` Jason Merrill
0 siblings, 2 replies; 29+ messages in thread
From: Martin Jambor @ 2010-12-03 13:45 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches, Richard Guenther, Jan Hubicka
Hi,
On Thu, Dec 02, 2010 at 06:25:28PM -0500, Jason Merrill wrote:
> On 12/01/2010 03:16 PM, Martin Jambor wrote:
> >and there are no unions of non-POD types
>
> This is actually no longer true in C++0x.
OK... but from what I have googled I understand that types with
virtual functions are still explicitely not allowed to be fields of
a union, right?
>
> >dynamic types. It assumes that destructors begin with assignment into
> >all VMT pointers and that constructors essentially look in the
> >following way:
> >
> >1) The very first thing they do is that they call constructors of the
> > components (including ancestor sub-objects) that have them.
>
> >2) Then VMT pointers of this and all its ancestors is set to new
> > values corresponding to the type corresponding to the constructor.
>
> Slight clarification: this happens after calling constructors for
> base sub-objects and before calling constructors for member
> sub-objects.
OK, I really only care about the sub-objects representing ancestors
(virtual or otherwise) as type of the user-defined member sub-objects
is not derived from the type of whatever other object they are
embedded in. In any event, passing the correct type to any
constructor is not necessary.
>
> The rest of the writeup all sounds correct.
>
Great, thanks a lot,
Martin
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 4/6] Dynamic type change detection
2010-12-03 13:45 ` Martin Jambor
@ 2010-12-03 14:34 ` Gabriel Dos Reis
2010-12-03 16:07 ` Jason Merrill
1 sibling, 0 replies; 29+ messages in thread
From: Gabriel Dos Reis @ 2010-12-03 14:34 UTC (permalink / raw)
To: Jason Merrill, GCC Patches, Richard Guenther, Jan Hubicka
On Fri, Dec 3, 2010 at 7:45 AM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> On Thu, Dec 02, 2010 at 06:25:28PM -0500, Jason Merrill wrote:
>> On 12/01/2010 03:16 PM, Martin Jambor wrote:
>> >and there are no unions of non-POD types
>>
>> This is actually no longer true in C++0x.
>
> OK... but from what I have googled I understand that types with
> virtual functions are still explicitely not allowed to be fields of
> a union, right?
I cannot see that restriction from the current draft.
http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2010/n3225.pdf
-- Gaby
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 4/6] Dynamic type change detection
2010-12-03 13:45 ` Martin Jambor
2010-12-03 14:34 ` Gabriel Dos Reis
@ 2010-12-03 16:07 ` Jason Merrill
2010-12-03 16:09 ` Richard Guenther
2010-12-03 16:21 ` Gabriel Dos Reis
1 sibling, 2 replies; 29+ messages in thread
From: Jason Merrill @ 2010-12-03 16:07 UTC (permalink / raw)
To: GCC Patches, Richard Guenther, Jan Hubicka
On 12/03/2010 08:45 AM, Martin Jambor wrote:
> On Thu, Dec 02, 2010 at 06:25:28PM -0500, Jason Merrill wrote:
>> On 12/01/2010 03:16 PM, Martin Jambor wrote:
>>> and there are no unions of non-POD types
>>
>> This is actually no longer true in C++0x.
>
> OK... but from what I have googled I understand that types with
> virtual functions are still explicitly not allowed to be fields of
> a union, right?
They are allowed now. 9.5/4 of N3126:
[ Example: Consider an object u of a union type U having non-static data
members m of type M and n of type N. If M has a non-trivial destructor
and N has a non-trivial constructor (for instance, if they declare or
inherit virtual functions), the active member of u can be safely
switched from m to n using the destructor and placement new operator as
follows:
u.m.~M();
new (&u.n) N;
-- end example ]
I wouldn't worry about this for now, though. I'll add it to the pile of
union language issues.
Jason
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 4/6] Dynamic type change detection
2010-12-03 16:07 ` Jason Merrill
@ 2010-12-03 16:09 ` Richard Guenther
2010-12-03 16:21 ` Gabriel Dos Reis
1 sibling, 0 replies; 29+ messages in thread
From: Richard Guenther @ 2010-12-03 16:09 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches, Jan Hubicka
On Fri, 3 Dec 2010, Jason Merrill wrote:
> On 12/03/2010 08:45 AM, Martin Jambor wrote:
> > On Thu, Dec 02, 2010 at 06:25:28PM -0500, Jason Merrill wrote:
> > > On 12/01/2010 03:16 PM, Martin Jambor wrote:
> > > > and there are no unions of non-POD types
> > >
> > > This is actually no longer true in C++0x.
> >
> > OK... but from what I have googled I understand that types with
> > virtual functions are still explicitly not allowed to be fields of
> > a union, right?
>
> They are allowed now. 9.5/4 of N3126:
>
> [ Example: Consider an object u of a union type U having non-static data
> members m of type M and n of type N. If M has a non-trivial destructor and N
> has a non-trivial constructor (for instance, if they declare or inherit
> virtual functions), the active member of u can be safely switched from m to n
> using the destructor and placement new operator as follows:
> u.m.~M();
> new (&u.n) N;
> -- end example ]
>
> I wouldn't worry about this for now, though. I'll add it to the pile of union
> language issues.
You have to handle similar situations with allocated storage and
arrays of chars anyway, so I don't think unions should
complicate anything.
Richard.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 4/6] Dynamic type change detection
2010-12-03 16:07 ` Jason Merrill
2010-12-03 16:09 ` Richard Guenther
@ 2010-12-03 16:21 ` Gabriel Dos Reis
2010-12-04 23:14 ` Jason Merrill
1 sibling, 1 reply; 29+ messages in thread
From: Gabriel Dos Reis @ 2010-12-03 16:21 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches, Richard Guenther, Jan Hubicka
On Fri, Dec 3, 2010 at 10:07 AM, Jason Merrill <jason@redhat.com> wrote:
> On 12/03/2010 08:45 AM, Martin Jambor wrote:
>>
>> On Thu, Dec 02, 2010 at 06:25:28PM -0500, Jason Merrill wrote:
>>>
>>> On 12/01/2010 03:16 PM, Martin Jambor wrote:
>>>>
>>>> and there are no unions of non-POD types
>>>
>>> This is actually no longer true in C++0x.
>>
>> OK... but from what I have googled I understand that types with
>> virtual functions are still explicitly not allowed to be fields of
>> a union, right?
>
> They are allowed now. 9.5/4 of N3126:
>
> [ Example: Consider an object u of a union type U having non-static data
> members m of type M and n of type N. If M has a non-trivial destructor and N
> has a non-trivial constructor (for instance, if they declare or inherit
> virtual functions), the active member of u can be safely switched from m to
> n using the destructor and placement new operator as follows:
> u.m.~M();
> new (&u.n) N;
> -- end example ]
>
> I wouldn't worry about this for now, though. I'll add it to the pile of
> union language issues.
Are you anticipating banning them?
I would not think they pose any problem -- copy and assignment
has to be user provided anyway, and assignment to a non-POD
data can be done only to the active member (since it is a
member function call)
-- Gaby
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 4/6] Dynamic type change detection
2010-12-03 16:21 ` Gabriel Dos Reis
@ 2010-12-04 23:14 ` Jason Merrill
0 siblings, 0 replies; 29+ messages in thread
From: Jason Merrill @ 2010-12-04 23:14 UTC (permalink / raw)
To: Gabriel Dos Reis; +Cc: GCC Patches, Richard Guenther, Jan Hubicka
On 12/03/2010 11:21 AM, Gabriel Dos Reis wrote:
> On Fri, Dec 3, 2010 at 10:07 AM, Jason Merrill<jason@redhat.com> wrote:
>> On 12/03/2010 08:45 AM, Martin Jambor wrote:
>>> OK... but from what I have googled I understand that types with
>>> virtual functions are still explicitly not allowed to be fields of
>>> a union, right?
>>
>> They are allowed now. 9.5/4 of N3126:
>>[...]
>> I wouldn't worry about this for now, though. I'll add it to the pile of
>> union language issues.
>
> Are you anticipating banning them?
No, just need to add this to the list of cases to be considered when
fixing the C/C++ union aliasing rules.
Jason
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 4/6] Dynamic type change detection
2010-12-02 16:17 ` Richard Guenther
@ 2010-12-09 11:30 ` Martin Jambor
0 siblings, 0 replies; 29+ messages in thread
From: Martin Jambor @ 2010-12-09 11:30 UTC (permalink / raw)
To: Richard Guenther; +Cc: GCC Patches, Jan Hubicka
Hi,
On Thu, Dec 02, 2010 at 05:17:25PM +0100, Richard Guenther wrote:
> On Thu, 2 Dec 2010, Richard Guenther wrote:
>
> > On Wed, 1 Dec 2010, Martin Jambor wrote:
> >
> > > This means that we do not have to care whether constructors get the
> > > correct type information because they will always change it (in fact,
> > > if we define the type to be given by the VMT pointer, it is
> > > undefined).
>
> You can still reach wrong conclusions for the dynamic type of *p
> in the following testcase:
>
> extern "C" void abort (void);
> extern "C" void *malloc(__SIZE_TYPE__);
> inline void* operator new(__SIZE_TYPE__, void* __p) throw() { return __p;
> }
> int x;
> class A {
> public:
> virtual ~A() { }
> };
> class B : public A {
> public:
> virtual ~B() { if (x == 1) abort (); x = 1; }
> };
> void __attribute__((noinline,noclone)) foo (void *p)
> {
> B *b = reinterpret_cast<B *>(p);
> b->~B();
> new (p) A;
> }
> int main()
> {
> void *p = __builtin_malloc (sizeof (B));
> new (p) B;
> foo(p);
> reinterpret_cast<A *>(p)->~A();
> return 0;
> }
>
> if you ignore the call to foo() then you see a vtable pointer store
> to make *p a B, but in reality at the time of the virtual call to
> the destructor it is an A.
This exactly the reason why I only devirtualize when I now that the
object has been automatically allocated (well, after the second patch
removes devirtualization according to global variables).
If we split main from the above testcase into three functions, one of
the jump function would look wrong but would not be used because it
will be combined with an UNKNOWN jump function coming from the caller
so it would not matter.
>
> Similar, if you add a char data[256]; member to A then a
>
> {
> B x;
> new (&x.data) A;
> reinterpret_cast<A *>(&x.data)->~A;
> }
>
> might possibly mis-lead you as to the type of B at destruction time
> (this example is more similar to the array issue I raised).
>
With a bit of tweaking I was able to make a failing testcase out of
this. More on this when I reply to the other email.
Thanks,
Martin
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks
2010-12-03 13:01 ` Martin Jambor
@ 2010-12-14 17:39 ` Jan Hubicka
2010-12-15 15:15 ` Martin Jambor
0 siblings, 1 reply; 29+ messages in thread
From: Jan Hubicka @ 2010-12-14 17:39 UTC (permalink / raw)
To: Jan Hubicka, GCC Patches
> Hi,
>
> > I guess it is still fine as it is definite improvement over current
> > situation, but won't we need to handle all of cgraph_thunk_info
> > here?
>
> Eventually we will, certainly before we start propagating constants to
> zeroth arguments of OBJ_TYPE_REFs. And not only here but also at
I tought we do that via constant folding already.
> other places (e.g. cgraph_set_call_stmt and company... and of course
> there is the weird cgraph_create_edge_including_clones I wrote you
> about earlier).
Hmm...
>
> > In thunk_info it is HOST_WIDE_INT, I would expect it to be here as well.
>
> I extract it from BINFOs as tree constants and I need to use it in
> POINTER_PLUSes as tree constants so there is probably no point in
> striing it as H_W_I.
Well, it is similar for the thunk code. In a way I preffer constants to be constants
as they are easier to handle in GGC/LTO etc. but I do not care too much.
> > Should've test here if this parameter was eliminated or not?
> > (i.e. first bit of e->callee->clone.combined_args_to_skip?
>
> It is not strictly necessary because I do the callee modification
> before removing parameters but on the second thought, yes, we should
> avoid creating unnecessary stuff when it is this easy. So I will add
> the check and the condition will be:
>
> if (e->indirect_info && e->indirect_info->thunk_delta
> && integer_nonzerop (e->indirect_info->thunk_delta)
> && (!e->callee->clone.combined_args_to_skip
> || !bitmap_bit_p (e->callee->clone.combined_args_to_skip, 0)))
OK...
>
> >
> > > Index: icln/gcc/ipa-prop.c
> > > ===================================================================
> > > --- icln.orig/gcc/ipa-prop.c
> > > +++ icln/gcc/ipa-prop.c
> >
> > I would expect somewhere here to be code handling updating of operand
> > value for non-0 delta when doing ipa-cp propagation, but don't seem to
> > be able to find it?
>
> What do you mean? IPA-CP stores the delta into the indirect info
> structure of the corresponding edge and then the code in
> gimple_adjust_this_by_delta performs the adjustment.
Will then IPA-CP handle correctly call to thunk that calls to real function
that just passes pointer to other function?
>
> >
> > The rest seems OK. I am most concerned that we implement just part of thunk
> > logic, but I see that you get deltas from BINFOs and the rest of adjustments
> > are not there?
> >
>
> Correct, for simple this adjusting thunks, BINFOs contain the decl of
> the real function and the this delta separately, for more complex
> thunks it stores the decl of an underlying thunk + a this delta which
> would otherwise be adjusted in this thunk.
>
> And yes, we will certainly have to re-think how to represent thunks in
> the cgraph in a more general way.
>
> Anyway, is the patch OK with the above change then?
OK.
Honza
>
> Thanks,
>
> Martin
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks
2010-12-14 17:39 ` Jan Hubicka
@ 2010-12-15 15:15 ` Martin Jambor
2010-12-15 15:46 ` Jan Hubicka
0 siblings, 1 reply; 29+ messages in thread
From: Martin Jambor @ 2010-12-15 15:15 UTC (permalink / raw)
To: Jan Hubicka; +Cc: GCC Patches
Hi,
On Tue, Dec 14, 2010 at 06:14:05PM +0100, Jan Hubicka wrote:
> > Hi,
> >
> > > I guess it is still fine as it is definite improvement over current
> > > situation, but won't we need to handle all of cgraph_thunk_info
> > > here?
> >
> > Eventually we will, certainly before we start propagating constants to
> > zeroth arguments of OBJ_TYPE_REFs. And not only here but also at
>
> I tought we do that via constant folding already.
No, things would break just as they did break in the testcases if IPA
saw calls to thunk decls.
>
> > other places (e.g. cgraph_set_call_stmt and company... and of course
> > there is the weird cgraph_create_edge_including_clones I wrote you
> > about earlier).
>
> Hmm...
> >
> > > In thunk_info it is HOST_WIDE_INT, I would expect it to be here as well.
> >
> > I extract it from BINFOs as tree constants and I need to use it in
> > POINTER_PLUSes as tree constants so there is probably no point in
> > striing it as H_W_I.
>
> Well, it is similar for the thunk code. In a way I preffer constants
> to be constants as they are easier to handle in GGC/LTO etc. but I
> do not care too much.
> > > Should've test here if this parameter was eliminated or not?
> > > (i.e. first bit of e->callee->clone.combined_args_to_skip?
> >
> > It is not strictly necessary because I do the callee modification
> > before removing parameters but on the second thought, yes, we should
> > avoid creating unnecessary stuff when it is this easy. So I will add
> > the check and the condition will be:
> >
> > if (e->indirect_info && e->indirect_info->thunk_delta
> > && integer_nonzerop (e->indirect_info->thunk_delta)
> > && (!e->callee->clone.combined_args_to_skip
> > || !bitmap_bit_p (e->callee->clone.combined_args_to_skip, 0)))
>
> OK...
> >
> > >
> > > > Index: icln/gcc/ipa-prop.c
> > > > ===================================================================
> > > > --- icln.orig/gcc/ipa-prop.c
> > > > +++ icln/gcc/ipa-prop.c
> > >
> > > I would expect somewhere here to be code handling updating of operand
> > > value for non-0 delta when doing ipa-cp propagation, but don't seem to
> > > be able to find it?
> >
> > What do you mean? IPA-CP stores the delta into the indirect info
> > structure of the corresponding edge and then the code in
> > gimple_adjust_this_by_delta performs the adjustment.
>
> Will then IPA-CP handle correctly call to thunk that calls to real function
> that just passes pointer to other function?
IPA-CP never propagates constants along edges that it has made direct
(becase these edges did not participate in the analysis), if that is
the concern. I agree that the final mechanism should be more general
and robust (and I guess we should meet for lunch early in January to
discuss it) but I certainly think this is good enough for 4.6.
> >
> > >
> > > The rest seems OK. I am most concerned that we implement just part of thunk
> > > logic, but I see that you get deltas from BINFOs and the rest of adjustments
> > > are not there?
> > >
> >
> > Correct, for simple this adjusting thunks, BINFOs contain the decl of
> > the real function and the this delta separately, for more complex
> > thunks it stores the decl of an underlying thunk + a this delta which
> > would otherwise be adjusted in this thunk.
> >
> > And yes, we will certainly have to re-think how to represent thunks in
> > the cgraph in a more general way.
> >
> > Anyway, is the patch OK with the above change then?
>
> OK.
Thanks a lot, I committed it as revision 167855.
Martin
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks
2010-12-15 15:15 ` Martin Jambor
@ 2010-12-15 15:46 ` Jan Hubicka
2010-12-15 16:52 ` Martin Jambor
0 siblings, 1 reply; 29+ messages in thread
From: Jan Hubicka @ 2010-12-15 15:46 UTC (permalink / raw)
To: Jan Hubicka, GCC Patches
> Hi,
>
> On Tue, Dec 14, 2010 at 06:14:05PM +0100, Jan Hubicka wrote:
> > > Hi,
> > >
> > > > I guess it is still fine as it is definite improvement over current
> > > > situation, but won't we need to handle all of cgraph_thunk_info
> > > > here?
> > >
> > > Eventually we will, certainly before we start propagating constants to
> > > zeroth arguments of OBJ_TYPE_REFs. And not only here but also at
> >
> > I tought we do that via constant folding already.
>
> No, things would break just as they did break in the testcases if IPA
> saw calls to thunk decls.
Well, I really think it is just matter of producing proper testcase. Those
functions are in vtables and thus the constant folding should get to them.
>
> IPA-CP never propagates constants along edges that it has made direct
> (becase these edges did not participate in the analysis), if that is
> the concern. I agree that the final mechanism should be more general
> and robust (and I guess we should meet for lunch early in January to
> discuss it) but I certainly think this is good enough for 4.6.
Well, this is somewhat fragile - the jump functions should be valid.
Hmm, so if early optimization devirutalize into thunk, we won't see this problem?
Can't we get problem when ipa-cp make some call direct and we propagate type though
it for furhter devirutalizatoin in inliner?
Honza
>
> > >
> > > >
> > > > The rest seems OK. I am most concerned that we implement just part of thunk
> > > > logic, but I see that you get deltas from BINFOs and the rest of adjustments
> > > > are not there?
> > > >
> > >
> > > Correct, for simple this adjusting thunks, BINFOs contain the decl of
> > > the real function and the this delta separately, for more complex
> > > thunks it stores the decl of an underlying thunk + a this delta which
> > > would otherwise be adjusted in this thunk.
> > >
> > > And yes, we will certainly have to re-think how to represent thunks in
> > > the cgraph in a more general way.
> > >
> > > Anyway, is the patch OK with the above change then?
> >
> > OK.
>
> Thanks a lot, I committed it as revision 167855.
>
> Martin
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks
2010-12-15 15:46 ` Jan Hubicka
@ 2010-12-15 16:52 ` Martin Jambor
0 siblings, 0 replies; 29+ messages in thread
From: Martin Jambor @ 2010-12-15 16:52 UTC (permalink / raw)
To: Jan Hubicka; +Cc: GCC Patches
Hi,
On Wed, Dec 15, 2010 at 03:38:08PM +0100, Jan Hubicka wrote:
> > Hi,
> >
> > On Tue, Dec 14, 2010 at 06:14:05PM +0100, Jan Hubicka wrote:
> > > > Hi,
> > > >
> > > > > I guess it is still fine as it is definite improvement over current
> > > > > situation, but won't we need to handle all of cgraph_thunk_info
> > > > > here?
> > > >
> > > > Eventually we will, certainly before we start propagating constants to
> > > > zeroth arguments of OBJ_TYPE_REFs. And not only here but also at
> > >
> > > I tought we do that via constant folding already.
> >
> > No, things would break just as they did break in the testcases if IPA
> > saw calls to thunk decls.
>
> Well, I really think it is just matter of producing proper testcase. Those
> functions are in vtables and thus the constant folding should get to them.
I'll prepare a small patch for cgraph_rebuild that will ICE if that
happens (when checking is enabled).
> >
> > IPA-CP never propagates constants along edges that it has made direct
> > (becase these edges did not participate in the analysis), if that is
> > the concern. I agree that the final mechanism should be more general
> > and robust (and I guess we should meet for lunch early in January to
> > discuss it) but I certainly think this is good enough for 4.6.
>
> Well, this is somewhat fragile - the jump functions should be
> valid.
Jump functions are always valid. This is because the previously
unknown edges did not participate in the lattice meet operations, and
so we cannot redirect them to the modified clone.
> Hmm, so if early optimization devirutalize into thunk, we won't see
> this problem?
We will.
> Can't we get problem when ipa-cp make some call direct and we
> propagate type through it for furhter devirutalizatoin in inliner?
Actually, we might miss those devirtualizations.
I know this is rather fragile in the sense that it depends on the fact
we do not perform certain optimizations, but exactly so is not doing
anything which is what we had until now. And a proper framework to
address this will be rather intrusive.
Martin
>
> Honza
> >
> > > >
> > > > >
> > > > > The rest seems OK. I am most concerned that we implement just part of thunk
> > > > > logic, but I see that you get deltas from BINFOs and the rest of adjustments
> > > > > are not there?
> > > > >
> > > >
> > > > Correct, for simple this adjusting thunks, BINFOs contain the decl of
> > > > the real function and the this delta separately, for more complex
> > > > thunks it stores the decl of an underlying thunk + a this delta which
> > > > would otherwise be adjusted in this thunk.
> > > >
> > > > And yes, we will certainly have to re-think how to represent thunks in
> > > > the cgraph in a more general way.
> > > >
> > > > Anyway, is the patch OK with the above change then?
> > >
> > > OK.
> >
> > Thanks a lot, I committed it as revision 167855.
> >
> > Martin
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks
2010-12-01 20:23 ` [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks Martin Jambor
2010-12-01 20:58 ` Jan Hubicka
@ 2010-12-17 14:14 ` H.J. Lu
1 sibling, 0 replies; 29+ messages in thread
From: H.J. Lu @ 2010-12-17 14:14 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches, Richard Guenther, Jan Hubicka
On Wed, Dec 1, 2010 at 12:16 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> this is a yet another ping for the patch below. It has already been
> approved by Richi but still awaits Honza's assessment.
>
> Re-bootstrapped and re-tested on x86-64-linux this Monday.
>
> Thanks,
>
> Martin
>
> ----------------------------------------
>
> Hi,
>
> we currently cannot really represent direct calls to thunks in the
> call graph (i.e. in the edges) and this means we can omit the
> necessary thunk adjustments when using that edge in IPA
> transformations. This then causes problems filed as PR 46053 and PR
> 46287. In 4.5 there were no such calls so it did not matter but with
> devirtualization we currently have them and we either need to handle
> them or avoid them. The patch below does both.
>
> Certainly, soon we will need to be able to represent this information
> on the edges in the full generality. In particular we will have to
> once we start devirtualizing by folding loads from the VMT. However,
> dealing with result adjusting and virtual_offset adjusting thunks will
> require more changes than I am comfortable to push in stage three,
> mainly because they are nested but there will also be other unforseen
> problems too.
>
> Therefore, I currently handle the simple this adjustment thunks and
> punt when a more complex one is discovered. Calls to this adjusting
> thunks are then converted to a POINTER_PLUS in the caller and a call
> to the ordinary function declaration. This way, we still don't have
> any direct calls to thunks in the IL. These adjustments in callees
> are also performed during IPA inline and IPA-CP devirtualizations.
>
> I do not perform the thunk check if the second argument of a folded
> OBJ_TYPE_REF is an ADDR_EXPR of a simple declaration (as opposed to a
> COMPONENT_REF) so that g++.dg/opt/devirt1.C passes. If I did, the
> thunk test would not be possible because there is no call graph node
> for the called method (which is not defined in this compilation unit).
> In this case there should be no thunks involved ( it is not a call to
> an ancestor's virtual method) and it is what previous versions of gcc
> do so we should not regress.
>
> Bootstrapped and tested on x86-64-linux without any issues, a very
> similar patch has also passed make check-c++ on i686. OK for trunk?
>
> Thanks,
>
> Martin
>
>
>
>
> 2010-11-08 Martin Jambor <mjambor@suse.cz>
>
> PR tree-optimization/46053
> PR middle-end/46287
> * cgraph.h (cgraph_indirect_call_info): New field.
> * gimple.h (gimple_fold_obj_type_ref): Declaration removed.
> (gimple_fold_call): Declare.
> (gimple_adjust_this_by_delta): Likewise.
> * cgraph.c (cgraph_make_edge_direct): New parameter delta. Updated
> all users.
> (cgraph_clone_edge): Create a copy of indirect_info also for direct
> edges.
> * cgraphunit.c (cgraph_redirect_edge_call_stmt_to_callee): Adjust this
> parameters.
> * gimple-fold.c (gimple_fold_obj_type_ref_known_binfo): Renamed to
> gimple_get_virt_mehtod_for_binfo, new parameter delta. Do not search
> through thunks, in fact bail out if we encounter one, check that
> BINFO_VIRTUALS is not NULL.
> (gimple_adjust_this_by_delta): New function.
> (gimple_fold_obj_type_ref): Removed.
> (gimple_fold_obj_type_ref_call): New function.
> (fold_gimple_call): Renamed to gimple_fold_call, made external.
> Updated users. Call gimple_fold_obj_type_ref_call instead of
> gimple_fold_obj_type_ref.
> * ipa-cp.c (ipcp_process_devirtualization_opportunities): Process
> thunk deltas.
> (ipcp_discover_new_direct_edges): Likewise.
> * ipa-prop.c (ipa_make_edge_direct_to_target): New parameter delta.
> Updated callers.
> (ipa_write_indirect_edge_info): Stream thunk_delta.
> (ipa_read_indirect_edge_info): Likewise.
> * tree-ssa-ccp.c (ccp_fold_stmt): Use gimple_fold_call instead of
> gimple_fold_obj_type_ref.
>
> * testsuite/g++.dg/ipa/pr46053.C: New test.
> * testsuite/g++.dg/ipa/pr46287-1.C: Likewise.
> * testsuite/g++.dg/ipa/pr46287-2.C: Likewise.
> * testsuite/g++.dg/ipa/pr46287-3.C: Likewise.
> * testsuite/g++.dg/torture/covariant-1.C: Likewise.
> * testsuite/g++.dg/torture/pr46287.C: Likewise.
>
This caused:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46987
H.J.
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2010-12-17 13:31 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-12-01 20:22 [PATCH, PR 45934 0/6] Devirtualization aware of dynamic type changes Martin Jambor
2010-12-01 20:22 ` [PATCH, PR 45934 3/6] More robust compute_complex_ancestor_jump_func Martin Jambor
2010-12-01 20:28 ` Richard Guenther
2010-12-01 20:23 ` [PATCH, PR 45934 1/6] [PR 46287] Do not generate direct calls to thunks Martin Jambor
2010-12-01 20:58 ` Jan Hubicka
2010-12-03 13:01 ` Martin Jambor
2010-12-14 17:39 ` Jan Hubicka
2010-12-15 15:15 ` Martin Jambor
2010-12-15 15:46 ` Jan Hubicka
2010-12-15 16:52 ` Martin Jambor
2010-12-17 14:14 ` H.J. Lu
2010-12-01 20:23 ` [PATCH, PR 45934 6/6] Intraprocedural type-based devirtualization Martin Jambor
2010-12-02 15:40 ` Richard Guenther
2010-12-01 20:23 ` [PATCH, PR 45934 4/6] Dynamic type change detection Martin Jambor
2010-12-02 15:19 ` Richard Guenther
2010-12-02 16:17 ` Richard Guenther
2010-12-09 11:30 ` Martin Jambor
2010-12-02 23:25 ` Jason Merrill
2010-12-03 13:45 ` Martin Jambor
2010-12-03 14:34 ` Gabriel Dos Reis
2010-12-03 16:07 ` Jason Merrill
2010-12-03 16:09 ` Richard Guenther
2010-12-03 16:21 ` Gabriel Dos Reis
2010-12-04 23:14 ` Jason Merrill
2010-12-01 20:23 ` [PATCH, PR 45934 5/6] Identify the new dynamic type after change Martin Jambor
2010-12-01 20:23 ` [PATCH, PR 45934 2/6] Remove devirtualizations that cannot be done Martin Jambor
2010-12-01 20:35 ` Richard Guenther
2010-12-02 10:46 ` Martin Jambor
2010-12-02 12:14 ` Richard Guenther
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).