Hi,
On Tue, Sep 25 2018, Eric Botcazou wrote:
> Hi,
>
> this extends the support for thunks present in the middle-end to accomodate
> the Ada language, which can generate more diverse thunks than C++. The main
> couple of enhancements are:
> 1. Indirect offsets used to implement thunks for self-referential types,
> 2. Local thunks used to implement thunks for types defined locally.
>
> It's done entirely in the middle-end, i.e. the interface with the back-end is
> not changed, the rationale being that the vast majority of thunks generated
> in
> Ada are the same as those generated in C++.
>
> This should be transparent for C++, except for the calls.c change which
> causes
> thunks not generated by the back-end to use a tailcall even at -O0 when
> that's
> possible, since that's what most of the back-ends do too.
>
> Tested x86-64/Linux, OK for the mainline?
I think you should also handle duplicate_thunk_for_node in
cgraphclones.c. That function clones thunks when the function they
belong to gets cloned. If you think you don't need to handle it for one
reason or another, at the very least put an assert there that
indirect_offset is zero.
Also...
> Index: cgraphunit.c
> ===================================================================
> --- cgraphunit.c (revision 264525)
> +++ cgraphunit.c (working copy)
> @@ -623,20 +623,18 @@ cgraph_node::analyze (void)
> callees->can_throw_external = !TREE_NOTHROW (t->decl);
> /* Target code in expand_thunk may need the thunk's target
> to be analyzed, so recurse here. */
> - if (!t->analyzed)
> + if (!t->analyzed && t->definition)
> t->analyze ();
> if (t->alias)
> {
> t = t->get_alias_target ();
> - if (!t->analyzed)
> + if (!t->analyzed && t->definition)
> t->analyze ();
> }
> - if (!expand_thunk (false, false))
> - {
> - thunk.alias = NULL;
> - return;
> - }
does this mean you can have thunks in one compilation unit that belong
to a function in another compilation unit? Interesting...
> + bool ret = expand_thunk (false, false);
> thunk.alias = NULL;
> + if (!ret)
> + return;
> }
> if (alias)
> resolve_alias (cgraph_node::get (alias_target), transparent_alias);
> @@ -1609,15 +1607,16 @@ init_lowered_empty_function (tree decl,
> return bb;
> }
>
> -/* Adjust PTR by the constant FIXED_OFFSET, and by the vtable
> - offset indicated by VIRTUAL_OFFSET, if that is
> - non-null. THIS_ADJUSTING is nonzero for a this adjusting thunk and
> - zero for a result adjusting thunk. */
> +/* Adjust PTR by the constant FIXED_OFFSET, by the vtable offset indicated by
> + VIRTUAL_OFFSET, and by the indirect offset indicated by INDIRECT_OFFSET,
> if
> + it is non-null. THIS_ADJUSTING is nonzero for a this adjusting thunk and
> zero
> + for a result adjusting thunk. */
>
> tree
> thunk_adjust (gimple_stmt_iterator * bsi,
> tree ptr, bool this_adjusting,
> - HOST_WIDE_INT fixed_offset, tree virtual_offset)
> + HOST_WIDE_INT fixed_offset, tree virtual_offset,
> + HOST_WIDE_INT indirect_offset)
> {
> gassign *stmt;
> tree ret;
> @@ -1632,6 +1631,16 @@ thunk_adjust (gimple_stmt_iterator * bsi
> gsi_insert_after (bsi, stmt, GSI_NEW_STMT);
> }
>
> + if (!vtable_entry_type && (virtual_offset || indirect_offset != 0))
> + {
> + tree vfunc_type = make_node (FUNCTION_TYPE);
> + TREE_TYPE (vfunc_type) = integer_type_node;
> + TYPE_ARG_TYPES (vfunc_type) = NULL_TREE;
> + layout_type (vfunc_type);
> +
> + vtable_entry_type = build_pointer_type (vfunc_type);
> + }
> +
> /* If there's a virtual offset, look up that value in the vtable and
> adjust the pointer again. */
> if (virtual_offset)
> @@ -1640,16 +1649,6 @@ thunk_adjust (gimple_stmt_iterator * bsi
> tree vtabletmp2;
> tree vtabletmp3;
>
> - if (!vtable_entry_type)
> - {
> - tree vfunc_type = make_node (FUNCTION_TYPE);
> - TREE_TYPE (vfunc_type) = integer_type_node;
> - TYPE_ARG_TYPES (vfunc_type) = NULL_TREE;
> - layout_type (vfunc_type);
> -
> - vtable_entry_type = build_pointer_type (vfunc_type);
> - }
> -
> vtabletmp =
> create_tmp_reg (build_pointer_type
> (build_pointer_type (vtable_entry_type)), "vptr");
> @@ -1687,6 +1686,41 @@ thunk_adjust (gimple_stmt_iterator * bsi
> GSI_CONTINUE_LINKING);
> }
>
> + /* Likewise for an offset that is stored in the object that contains the
> + vtable. */
> + if (indirect_offset != 0)
> + {
> + tree offset_ptr, offset_tree;
> +
> + /* Get the address of the offset. */
> + offset_ptr
> + = create_tmp_reg (build_pointer_type
> + (build_pointer_type (vtable_entry_type)),
> + "offset_ptr");
> + stmt = gimple_build_assign (offset_ptr,
> + build1 (NOP_EXPR, TREE_TYPE (offset_ptr),
> + ptr));
> + gsi_insert_after (bsi, stmt, GSI_NEW_STMT);
> +
> + stmt = gimple_build_assign
> + (offset_ptr,
> + fold_build_pointer_plus_hwi_loc (input_location, offset_ptr,
> + indirect_offset));
> + gsi_insert_after (bsi, stmt, GSI_NEW_STMT);
> +
> + /* Get the offset itself. */
> + offset_tree = create_tmp_reg (TREE_TYPE (TREE_TYPE (offset_ptr)),
> + "offset");
> + stmt = gimple_build_assign (offset_tree,
> + build_simple_mem_ref (offset_ptr));
> + gsi_insert_after (bsi, stmt, GSI_NEW_STMT);
> +
If I read this correctly (and the offset is an int), nowadays you might
just want to do simple
stmt = gimple_build_assign (offset_tree,
fold_build2 (MEM_REF,
integer_type_node,
ptr,
build_int_cst
(build_pointer_type (integer_type_node),
indirect_offset)));
instead of all of the above? You might then also leave the creation of
vtable_entry_type where it is now.
Thanks,
Martin
