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
