Am Montag, den 17.12.2018, 10:31 -0700 schrieb Martin Sebor: > On 12/16/18 6:45 AM, Uecker, Martin wrote: > > Am Freitag, den 14.12.2018, 18:20 -0700 schrieb Martin Sebor: > > > 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). > > > > There is a choice to be made: > > > > Either we activate the lang hook for C, then the minimum alignment for > > functions on is not 1 anymore, because we need one bit to identify the > > descriptors. From a correcntess point of view, this is OK as 'alignas' > > is only required to increase alignment. > > alignas doesn't apply to functions. Changing function alignment > is a feature unique to attribute aligned. The attribute can be > used to decrease the alignment of functions that have not yet > been explicitly declared with one. GCC has historically allowed > this, and based on my tests, the i386 target has always allowed > functions to be completely unaligned (either by using attribute > aligned (1) when supported or via -Os/-falign-functions=1). > The purpose of the test is to verify that this works. > > > It is also not really regression > > in my opinion, as it is nowhere documented that one can reduce alignment > > to '1'. The test also has just been added a couple of days ago (if I am > > not mistaken). For these reasons, I think it would be OK to remove the test. > > This wasn't clearly documented until recently. The test was added > to verify that GCC behaves as intended and clarified in the manual. > The latest manual mentions that: > > The attribute cannot be used to decrease the alignment of > a function previously declared with a more restrictive alignment; > only to increase it. Attempts to do otherwise are diagnosed. > Some targets specify a minimum default alignment for functions > that is greater than 1. On such targets, specifying a less > restrictive alignment is silently ignored. > > The update to the text was discussed here: > https://gcc.gnu.org/ml/gcc/2018-11/msg00127.html > > If the i386 target should increase the minimum supported alignment > up from 1 under some conditions the test might need to be adjusted > (but it should not be removed).
Thank you for clarifying. Based on the discussion I think we can't change the minimum alignment (at least without the command-line flag). Best, Martin > > > > The other option is to decide that having an alignment of only '1' > > is a valuable feature and should be preserved on those platforms which > > support it. I have no idea what this could be good for, but maybe > > there are use cases. In this case, it makes of course sense to keep > > the test. We should then remove the lang hook (as it could never be > > activated for most languages) and instead live with the fact that > > '-fno-trampoline' and using alignof(1) on functions are simply > > incompatible. A warning could be added if the compiler sees > > alignof(1) when -fno-trampoline is active. > > > > > > I am happy with both choices. > > > > > > Best, > > Martin > > > > > >
