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).
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
