compiler-rt/cpu_model: Raise constructor priority to align with GCC

[loladiro]

Summary

GCC recently started setting constructor priority on init_have_lse_atomics [1]
to avoid undefined initialization order with respect to other initializers,
causing accidental use of ll/sc intrinsics on targets where this was not
intended (which presents a minor performance problem as well as a
compatibility problem for users wanting to use the rr debugger). compiler-rt
does not have the same issue as libgcc, since we're already setting init
priority on the constructor. However, our choice of init priority
is the highest application priority (101), while as a compiler support
library, it would be better to use one of the priority levels reserved
for system libraries (1-100, lower integers are higher priority).
GCC ended up using 90, so this commit aligns the value in compiler-rt
to the same value to ensure that there are no subtle initialization order
differences between libgcc and compiler-rt. I do not know of a case
where this currently causes a problem, but because binary builds of these
libraries can have long lifespans in toolchains, I think this is a
prudent, conservative change.

[1] https://github.com/gcc-mirror/gcc/commit/75c4e4909ae2667f56487434f99c2915b4570794

[MaskRay]

constructor(90) is more common.

Please check whether this is needed.

#if __GNUC__ >= 9
#pragma GCC diagnostic ignored "-Wprio-ctor-dtor"
#endif

[loladiro]

In D126424#3538733, @MaskRay wrote:

constructor(90) is more common.

I can make this change, but I wasn't sure if some other supported compiler might need this spelling, since it was there before.

Please check whether this is needed.

#if __GNUC__ >= 9
#pragma GCC diagnostic ignored "-Wprio-ctor-dtor"
#endif

I built with GCC9 to check whether there was a diagnostic for this, but I didn't see one. Unfortunately, I then checked whether the intialization order actually worked and it appears that the HAVE_INIT_PRIORITY check isn't actually working, so let me look into that first, because that's a bigger problem.

[loladiro]

Updated to unconditionally use the init priority. Turns out the HAVE_INIT_PRIORITY config
check doesn't actually exist. However, for all our supported compilers that have the
constructor attribute, they should also have the constructor attribute with init priority,
which we use elsewhere in compiler-rt without extra check.

[MaskRay]

#if __has_attribute(constructor) is unneeded. All supported GCC/Clang supports #if __has_attribute(constructor).

we well as constructors in the C++ standard library (which are at priority 100).

libgcc has a __attribute__ ((constructor(100))) function but it's incorrect to state that "we well as constructors in the C++ standard library (which are at priority 100)".

[loladiro]

In D126424#3539200, @MaskRay wrote:

#if __has_attribute(constructor) is unneeded. All supported GCC/Clang supports #if __has_attribute(constructor).

Ok, the comment seemed to indicate that this was being done for MSVC compatibility. Is building compiler-rt with MSVC no longer supported?

we well as constructors in the C++ standard library (which are at priority 100).

libgcc has a __attribute__ ((constructor(100))) function but it's incorrect to state that "we well as constructors in the C++ standard library (which are at priority 100)".

I was specifically thinking of https://github.com/llvm/llvm-project/blob/main/libcxx/include/__config#L1275 and https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/libstdc%2B%2B-v3/src/c%2B%2B17/default_resource.h#L11, though
it's possible I may have missed another instance. I can see that the comment is overly broad though. I can change it to something like
(which in current versions of libstdc++ and libcxx are at priority 100). Would that address your concern?

[MaskRay]

In D126424#3539240, @loladiro wrote:

In D126424#3539200, @MaskRay wrote:

#if __has_attribute(constructor) is unneeded. All supported GCC/Clang supports #if __has_attribute(constructor).

Ok, the comment seemed to indicate that this was being done for MSVC compatibility. Is building compiler-rt with MSVC no longer supported?

There is #define __has_attribute(attr) 0

we well as constructors in the C++ standard library (which are at priority 100).

libgcc has a __attribute__ ((constructor(100))) function but it's incorrect to state that "we well as constructors in the C++ standard library (which are at priority 100)".

I was specifically thinking of https://github.com/llvm/llvm-project/blob/main/libcxx/include/__config#L1275 and https://github.com/gcc-mirror/gcc/blob/16e2427f50c208dfe07d07f18009969502c25dc8/libstdc%2B%2B-v3/src/c%2B%2B17/default_resource.h#L11, though
it's possible I may have missed another instance. I can see that the comment is overly broad though. I can change it to something like
(which in current versions of libstdc++ and libcxx are at priority 100). Would that address your concern?

It's still too broad. I see that for few things in libstdc++ it's at priority 100 but most aren't.

"Choose init priority 90 to force our constructors to run before any constructors in the user application (starting at priority 101)" suffices.

[loladiro]

In D126424#3539242, @MaskRay wrote:

In D126424#3539240, @loladiro wrote:

In D126424#3539200, @MaskRay wrote:

#if __has_attribute(constructor) is unneeded. All supported GCC/Clang supports #if __has_attribute(constructor).

Ok, the comment seemed to indicate that this was being done for MSVC compatibility. Is building compiler-rt with MSVC no longer supported?

There is #define __has_attribute(attr) 0

I'm sorry, I'm confused what change you'd like me to make here. The __has_attribute was there before. Could you elaborate?

It's still too broad. I see that for few things in libstdc++ it's at priority 100 but most aren't.

"Choose init priority 90 to force our constructors to run before any constructors in the user application (starting at priority 101)" suffices.

Ok.

[MaskRay]

In D126424#3539245, @loladiro wrote:

In D126424#3539242, @MaskRay wrote:

In D126424#3539240, @loladiro wrote:

In D126424#3539200, @MaskRay wrote:

#if __has_attribute(constructor) is unneeded. All supported GCC/Clang supports #if __has_attribute(constructor).

Ok, the comment seemed to indicate that this was being done for MSVC compatibility. Is building compiler-rt with MSVC no longer supported?

There is #define __has_attribute(attr) 0

I'm sorry, I'm confused what change you'd like me to make here. The __has_attribute was there before. Could you elaborate?

Sorry, this is OK.

It's still too broad. I see that for few things in libstdc++ it's at priority 100 but most aren't.

"Choose init priority 90 to force our constructors to run before any constructors in the user application (starting at priority 101)" suffices.

Ok.

[loladiro]

Remove reference C++ stdlib in comment

[MaskRay]

Looks great!

compiler-rt/lib/builtins/cpu_model.c

 //  __aarch64_have_lse_atomics for AArch64.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef __has_attribute
 #define __has_attribute(attr) 0
 #endif
 
-#if defined(HAVE_INIT_PRIORITY)
-#define CONSTRUCTOR_ATTRIBUTE __attribute__((__constructor__ 101))
-#elif __has_attribute(__constructor__)
-#define CONSTRUCTOR_ATTRIBUTE __attribute__((__constructor__))
+#if __has_attribute(constructor)
+#if __GNUC__ >= 9
+// Ordinarily init priorities below 101 are disallowed as they are reserved for the
+// implementation. However, we are the implementation, so silence the diagnostic,
+// since it doesn't apply to us.
+#pragma GCC diagnostic ignored "-Wprio-ctor-dtor"
+#endif
+// We're choosing init priority 90 to force our constructors to run before any
+// constructors in the end user application (starting at priority 101). This value
+// matches the libgcc choice for the same functions.
+#define CONSTRUCTOR_ATTRIBUTE __attribute__((constructor(90)))
 #else
 // FIXME: For MSVC, we should make a function pointer global in .CRT$X?? so that
 // this runs during initialization.
 #define CONSTRUCTOR_ATTRIBUTE
 #endif
 
 #if (defined(__i386__) || defined(_M_IX86) || defined(__x86_64__) ||           \
      defined(_M_X64)) &&                                                       \