Here is the new patch that walks op looking for the reference to addr.
Passes internal benchmarks and regression tests. Ok for google/4_9?
Thanks,
Teresa
2014-10-27 Teresa Johnson <tejohn...@google.com>
Google ref b/18110567.
* cgraphbuild.c (fixup_all_refs_1): New function.
(fixup_all_refs): Ditto.
(fixup_ref): Walk tree to find and replace ref.
Index: cgraphbuild.c
===================================================================
--- cgraphbuild.c (revision 216735)
+++ cgraphbuild.c (working copy)
@@ -665,6 +665,45 @@ record_references_in_initializer (tree decl, bool
pointer_set_destroy (visited_nodes);
}
+typedef struct _fixup_decl_info {
+ tree orig_decl;
+ tree new_decl;
+} fixup_decl_info;
+
+/* Check the tree at TP to see if it contains the original decl stored in
+ DATA and if so replace it with the new decl. If original decl is
+ found set WALK_SUBTREES to 0 so the subtree under TP is not traversed.
+ Returns the updated parent tree T or NULL if no update performed. */
+
+static tree
+fixup_all_refs_1 (tree *tp, int *walk_subtrees, void *data)
+{
+ tree t = *tp;
+ fixup_decl_info *info = (fixup_decl_info *) data;
+
+ /* The original function decl is always the first tree operand. */
+ if (TREE_OPERAND (t,0) == info->orig_decl)
+ {
+ TREE_OPERAND (t,0) = info->new_decl;
+ *walk_subtrees = 0;
+ return t;
+ }
+ return NULL_TREE;
+}
+
+/* Walk the whole tree rooted at TP and invoke fixup_all_refs_1 to
+ replace any references to the original decl with the new decl
+ stored in INFO. */
+
+static inline void
+fixup_all_refs (tree *tp, fixup_decl_info *info)
+{
+ tree t = walk_tree (tp, fixup_all_refs_1, info, NULL);
+ /* This is invoked when we found the original decl, so we expect
+ to have replaced a reference. */
+ gcc_assert (t != NULL_TREE);
+}
+
/* Update any function decl references in base ADDR of operand OP to refer to
the resolved node. */
@@ -674,13 +713,16 @@ fixup_ref (gimple, tree addr, tree op)
addr = get_base_address (addr);
if (addr && TREE_CODE (addr) == FUNCTION_DECL)
{
- gcc_assert (TREE_CODE (op) == ADDR_EXPR);
- gcc_assert (TREE_OPERAND (op,0) == addr);
struct cgraph_node *real_callee;
real_callee = cgraph_lipo_get_resolved_node (addr);
if (addr == real_callee->decl)
return false;
- TREE_OPERAND (op,0) = real_callee->decl;
+ /* We need to locate and update the tree operand within OP
+ that contains ADDR and update it to the real callee's decl. */
+ fixup_decl_info info;
+ info.orig_decl = addr;
+ info.new_decl = real_callee->decl;
+ fixup_all_refs (&op, &info);
}
return false;
}
On Fri, Oct 24, 2014 at 11:46 AM, Teresa Johnson <tejohn...@google.com> wrote:
> On Fri, Oct 24, 2014 at 10:55 AM, Xinliang David Li <davi...@google.com>
> wrote:
>> When orgin_addr == addr, is there a guarantee that this assert:
>>
>> gcc_assert (TREE_OPERAND (op,0) == addr);
>>
>> is always true?
>
> It should be, that is the assumption of the code that we are trying to
> enforce with the assert.
>
> Teresa
>
>>
>> David
>>
>>
>>
>> On Fri, Oct 24, 2014 at 10:21 AM, Teresa Johnson <tejohn...@google.com>
>> wrote:
>>> This patch makes a fix to the reference fixups performed after LIPO
>>> node resolution, to better handle the case where we are updating the
>>> base address of a reference.
>>>
>>> Fixes google benchmark and passes regression tests. Ok for google/4_9?
>>>
>>> Thanks,
>>> Teresa
>>>
>>> 2014-10-24 Teresa Johnson <tejohn...@google.com>
>>>
>>> Google ref b/18110567.
>>> * cgraphbuild.c (get_base_address_expr): New function.
>>> (fixup_ref): Update the op expression for new base address.
>>>
>>> Index: cgraphbuild.c
>>> ===================================================================
>>> --- cgraphbuild.c (revision 216667)
>>> +++ cgraphbuild.c (working copy)
>>> @@ -665,13 +665,35 @@ record_references_in_initializer (tree decl, bool
>>> pointer_set_destroy (visited_nodes);
>>> }
>>>
>>> +/* Similar to get_base_address but returns the ADDR_EXPR pointing
>>> + to the base address corresponding to T. */
>>> +
>>> +static tree
>>> +get_base_address_expr (tree t)
>>> +{
>>> + while (handled_component_p (t))
>>> + t = TREE_OPERAND (t, 0);
>>> +
>>> + if ((TREE_CODE (t) == MEM_REF
>>> + || TREE_CODE (t) == TARGET_MEM_REF)
>>> + && TREE_CODE (TREE_OPERAND (t, 0)) == ADDR_EXPR)
>>> + return TREE_OPERAND (t, 0);
>>> +
>>> + return NULL_TREE;
>>> +}
>>> +
>>> /* Update any function decl references in base ADDR of operand OP to refer
>>> to
>>> the resolved node. */
>>>
>>> static bool
>>> fixup_ref (gimple, tree addr, tree op)
>>> {
>>> + tree orig_addr = addr;
>>> addr = get_base_address (addr);
>>> + /* If the address was changed, update the operand OP to be the
>>> + ADDR_EXPR pointing to the new base address. */
>>> + if (orig_addr != addr)
>>> + op = get_base_address_expr (orig_addr);
>>> if (addr && TREE_CODE (addr) == FUNCTION_DECL)
>>> {
>>> gcc_assert (TREE_CODE (op) == ADDR_EXPR);
>>>
>>>
>>> --
>>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
--
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
