Hi Martin,
> The tests for the new __builtin_has_attribute function have been
> failing on a number of targets because of a couple of assumptions
> that only hold on some.
>
> First, they expect that it's safe to apply attribute aligned with
> a smaller alignment than the target provides when GCC rejects such
> arguments. The tests pass on i86 and elsewhere but fail on
> strictly aligned targets like aarch64 or sparc. After some testing
> and thinking I don't think this is helpful -- I believe it's better
> to instead silently accept attributes that ask for a less restrictive
> alignment than the function ultimately ends up with (see * below).
> This is what testing shows Clang does on those targets. The attached
> patch implements this change.
>
> Second, the tests assume that the priority forms of the constructor
> and destructor attributes are universally supported. That's also
> not the case, even though the manual doesn't mention that. To
> avoid these failures the attached patch moves the priority forms
> of the attribute constructor and destructor tests into its own
> file that's compiled only for init_priority targets.
>
> Finally, I noticed that attribute aligned accepts zero as
> an argument even though it's not a power of two as the manual
> documents as a precondition (zero is treated the same as
> the attribute without an argument). A zero argument is likely
> to be a mistake, especially when the zero comes from macro
> expansion, that users might want to know about. Clang rejects
> a zero with an error but I think a warning is more in line with
> established GCC practice, so the patch also implements that.
>
> Besides x86_64-linux, I tested this change with cross-compilers
> for aarch64-linux-elf, powerpc64le-linux, and sparc-solaris2.11.
> I added tests for the changed aligned attribute for those targets
> To make the gcc.dg/builtin-has-attribute.c test pass with
> the cross-compilers I changed from a runtime test into a compile
> only one.
>
> Martin
>
> PS I'm not happy about duplicating the same test across all those
> targets. It would be much nicer to have a single test somewhere
> in dg.exp #include a target-specific header with macros describing
> the target-specific parameters.
why so complicated? Just have a single attr-aligned.c test, restricted
to the target cpus it supports via dg directives and with
MINALIGN/MAXALIGN definitions controlled by appropriate target macros?
Besides, the new gcc.target/sparc/attr-aligned.c test currently FAILs on
64-bit sparc:
FAIL: gcc.target/sparc/attr-aligned.c (test for excess errors)
Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/gcc.target/sparc/attr-aligned.c:29:1:
error: static assertion failed: "alignof (f_) == MAXALIGN"
alignof (f_) is 16 for sparcv9. The following patch fixes this, tested
on sparc-sun-solaris2.11 with -m32 and -m64.
Ok for mainline?
Rainer
--
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
2018-12-04 Rainer Orth <r...@cebitec.uni-bielefeld.de>
PR testsuite/88208
* gcc.target/sparc/attr-aligned.c (MAXALIGN) [__sparcv9 ||
__arch64__]: Define.
# HG changeset patch
# Parent bd6c4f1f5f5ef5a6d912a66d6f3b40f21c8dd411
Provide SPARCv9 MAXALIGN in gcc.target/sparc/attr-aligned.c
diff --git a/gcc/testsuite/gcc.target/sparc/attr-aligned.c b/gcc/testsuite/gcc.target/sparc/attr-aligned.c
--- a/gcc/testsuite/gcc.target/sparc/attr-aligned.c
+++ b/gcc/testsuite/gcc.target/sparc/attr-aligned.c
@@ -10,7 +10,11 @@
#define HAS_ALIGN(f, n) __builtin_has_attribute (f, __aligned__ (n))
#define MINALIGN(N) ((N) < 4 ? 4 : (N))
+#if defined(__sparcv9) || defined(__arch64__)
+#define MAXALIGN 16
+#else
#define MAXALIGN 8
+#endif
/* No alignment specified. */
void f (void) { }
