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
