This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH v4][C][ADA] use function descriptors instead of trampolines in C
- From: Jeff Law <law at redhat dot com>
- To: "Uecker, Martin" <Martin dot Uecker at med dot uni-goettingen dot de>, "joseph at codesourcery dot com" <joseph at codesourcery dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, "ebotcazou at adacore dot com" <ebotcazou at adacore dot com>
- Date: Thu, 13 Dec 2018 16:35:22 -0700
- Subject: Re: [PATCH v4][C][ADA] use function descriptors instead of trampolines in C
- References: <1534005653.22677.9.camel@med.uni-goettingen.de> <1534609978.14596.2.camel@med.uni-goettingen.de> <1534774021.5798.6.camel@med.uni-goettingen.de> <alpine.DEB.2.21.1808202227370.646@digraph.polyomino.org.uk> <1534832264.15600.1.camel@med.uni-goettingen.de> <alpine.DEB.2.21.1808212132350.13329@digraph.polyomino.org.uk> <1534918133.4891.5.camel@med.uni-goettingen.de> <alpine.DEB.2.21.1808221541430.26950@digraph.polyomino.org.uk> <1541364511.30044.4.camel@med.uni-goettingen.de> <d6b29467-0d1f-ac82-faf5-7e15c469bbca@redhat.com> <1544638361.17067.3.camel@med.uni-goettingen.de>
On 12/12/18 11:12 AM, Uecker, Martin wrote:
>
> Hi Jeff,
>
> thank you. I fixed all the minor issues, but see below.
>
>
> Am Montag, den 03.12.2018, 14:56 -0700 schrieb Jeff Law:
>> On 11/4/18 1:48 PM, Uecker, Martin wrote:
>>> Hi Joseph,
>>>
>>> here is a new version of this patch which adds a warning
>>> for targets which do not support -fno-trampolines and
>>> only runs the test case on architectures where this is
>>> supported. It seems that documentation for this general
>>> feature has improved in the meantime so I only mention
>>> C as supported.
>>>
>>>
>>> Best,
>>> Martin
>>>
>>> diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
>>> index ce2d43f989e..b79f2373c63 100644
>>> --- a/gcc/ada/gcc-interface/trans.c
>>> +++ b/gcc/ada/gcc-interface/trans.c
>>> @@ -1753,7 +1753,8 @@ Attribute_to_gnu (Node_Id gnat_node, tree *gnu_result_type_p, int
>>> attribute)
>>> if ((attribute == Attr_Access
>>> || attribute == Attr_Unrestricted_Access)
>>> && targetm.calls.custom_function_descriptors > 0
>>> - && Can_Use_Internal_Rep (Etype (gnat_node)))
>>> + && Can_Use_Internal_Rep (Etype (gnat_node))
>>> + && (flag_trampolines != 1))
>>> FUNC_ADDR_BY_DESCRIPTOR (gnu_expr) = 1;
>>
>> You've got an extraneous set of parenthesis around your flag_trampolines
>> check. Please remove them.
>>
>>
>>>
>>> /* Otherwise, we need to check that we are not violating the
>>> @@ -4330,7 +4331,8 @@ Call_to_gnu (Node_Id gnat_node, tree *gnu_result_type_p, tree gnu_target,
>>> /* If the access type doesn't require foreign-compatible representation,
>>> be prepared for descriptors. */
>>> if (targetm.calls.custom_function_descriptors > 0
>>> - && Can_Use_Internal_Rep (Etype (Prefix (Name (gnat_node)))))
>>> + && Can_Use_Internal_Rep (Etype (Prefix (Name (gnat_node))))
>>> + && (flag_trampolines != 1))
>>
>> Similarly here.
>>
>>
>>> diff --git a/gcc/c/c-objc-common.h b/gcc/c/c-objc-common.h
>>> index 78e768c2366..ef039560eb9 100644
>>> --- a/gcc/c/c-objc-common.h
>>> +++ b/gcc/c/c-objc-common.h
>>> @@ -110,4 +110,7 @@ along with GCC; see the file COPYING3. If not see
>>>
>>> #undef LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P
>>> #define LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P c_vla_unspec_p
>>> +
>>> +#undef LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS
>>> +#define LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS true
>>> #endif /* GCC_C_OBJC_COMMON */
>>
>> I wonder if we even need the lang hook anymore. ISTM that a front-end
>> that wants to use the function descriptors can just set
>> FUNC_ADDR_BY_DESCRIPTOR and we'd use the function descriptor, else we'll
>> use the trampoline. Thoughts?
>
> The lang hook also affects the minimum alignment for function
> pointers via the FUNCTION_ALIGNMENT macro (gcc/default.h). This does
> not appear to change the default alignment on any architecture, but
> it causes a failure in i386/gcc.target/i386/attr-aligned.c when
> requesting a smaller alignment which is then silently ignored.
Ugh. I didn't see that.
>
> I am not sure what the best approach is, but my preference
> would be to remove the lang hook and the FUNCTION_ALIGNMENT
> logic which will also fix the test case (the requested
> alignment will be applied).
>
> I would then instead add a warning (or error?) which triggers
> only with -fno-trampolines if the user requests an alignment
> which is too small for this mechanism to work.
>
> Does this sound reasonable?
So I'm thinking we should wrap the existing patch as-is for the trunk
(we're well into stage3 after all). So leave the hook as-is for gcc-9.
We can then tackle removal of the hook, including twiddling
FUNCTION_ALIGNMENT for gcc-10.
Does that sound reasonable to you?
jeff