[PATCH v4][C][ADA] use function descriptors instead of trampolines in C

Martin Sebor msebor@gmail.com
Sat Dec 15 01:20:00 GMT 2018


On 12/14/18 4:36 PM, Jeff Law wrote:
> On 12/14/18 3:05 AM, Uecker, Martin wrote:
>>
>> Am Donnerstag, den 13.12.2018, 16:35 -0700 schrieb Jeff Law:
>>> On 12/12/18 11:12 AM, Uecker, Martin wrote:
>> ...
>>>>>> 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.
>> The test is new (2019-11-29 Martin Sebor), but one could
>> argue that we could simply remove this specific test as 'aligned'
>> is only required to increase alignment. Martin?
> The test is meant to test that we do the right thing consistently.  If
> we're failing with your patch, then that needs to be addressed.

I haven't been paying attention here and so I don't know how the test
fails after the change.  It's meant to verify that attribute aligned
successfully reduces the alignment of functions that have not been
previously declared with one all the way down to the supported minimum
(which is 1 on i386).  I agree with Jeff that removing the test would
not be right unless the failure is due to some bad assumptions on my
part.  If it's the built-in that fails that could be due to a bug in
it (it's very new).

Martin

> 
> I read your note as the test would fail if you dropped the
> CUSTOM_FUNCTION_DESCRIPTORS macro, not that it was failing with your
> patch as-is.
> 
> 
> 
>>
>>>> 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?
>> This is fine with me. So just confirm: I should install the
>> patch despite the regression?
> We need to address the regression.  Simply removing the test is probably
> not the way to go.
> 
> jeff
> 



More information about the Gcc-patches mailing list