[compiler-rt] Force baseline on 32-bit SPARC to V8+ when using gcc

[glaubitz]

Summary

Unlike clang or Solaris Studio, gcc does not enable 64-bit atomic
operations on 32-bit SPARC even when using the V9 baseline. This
causes the compiler-rt library failing to build with gcc when
targeting 32-bit SPARC.

Passing "-mcpu=ultrasparc3" to the compiler raises the baseline to
V8+ which enables 64-bit atomic operations on 32-bit SPARC, thus
allowing the compiler-rt library to be built successfully with gcc
during the stage1 build.

[ro]

Sorry to chime in so late, but I believe this patch has several problems:

• For one, it's at best a partial solution to the problem of atomics on 32-bit SPARC: those are bound to also occur outside of the LLVM tree and this patch does nothing to handle that.
• Even so, it's unclear what CPUs are supposed to run the generated 32-bit code: if it's only supposed to run on SPARC V9 CPUs, why not use the same solution I did for Solaris/sparcv9 in D86621: always default to -mcpu=v9? If the code can also be run on V8 CPUs, the patch is wrong: if run there, the V9 insns will cause the resulting binaries to randomly crash with SIGILL: not exactly a good user experience. In that case, the driver should link with --as-needed -latomic --no-as-needed instead.
• Besides, I don't see the point of linking with -latomic when you already pass -mcpu=v9. It shouldn't be necessary and is certainly wrong on Solaris/sparcv9 where it only adds an unnecessary dependency.

[glaubitz]

In D98575#2632875, @ro wrote:

Sorry to chime in so late, but I believe this patch has several problems:

• For one, it's at best a partial solution to the problem of atomics on 32-bit SPARC: those are bound to also occur outside of the LLVM tree and this patch does nothing to handle that.

Not sure I understand that particular problem. Why would we care what's happening outside LLVM?

• Even so, it's unclear what CPUs are supposed to run the generated 32-bit code: if it's only supposed to run on SPARC V9 CPUs, why not use the same solution I did for Solaris/sparcv9 in D86621: always default to -mcpu=v9? If the code can also be run on V8 CPUs, the patch is wrong: if run there, the V9 insns will cause the resulting binaries to randomly crash with SIGILL: not exactly a good user experience. In that case, the driver should link with --as-needed -latomic --no-as-needed instead.
• Besides, I don't see the point of linking with -latomic when you already pass -mcpu=v9. It shouldn't be necessary and is certainly wrong on Solaris/sparcv9 where it only adds an unnecessary dependency.

The -latomic is actually necesary even with -mcpu=v9 since it will still complaing about `atomic_load_8`` being unresolved. But I think the easiest fix will be just to link against libatomic and drop the `-mcpu=v9`` again.

[glaubitz]

FWIW, the reason I needed `-mcpu=v9``` was this bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27558

[glaubitz]

Ping. Any chance we can get this in? It's the minimal approach to fix the compiler-rt build on 32-bit SPARC.

[vitalybuka]

compiler-rt/cmake/base-config-ix.cmake

194

Why do you need to (CMAKE_SIZEOF_VOID_P EQUAL 4)?
Let test_target_arch pick appropriate one?

Which target does not build without the patch?

[jrtc27]

compiler-rt/cmake/base-config-ix.cmake

192–197

test_target_arch(sparc "" "-m32" "-latomic")
test_target_arch(sparcv9 "" "-m64")

is what we want if building for v8 or below, surely?

[glaubitz]

compiler-rt/cmake/base-config-ix.cmake

192–197

That would probably work as well. I can try it.

194

32-bit SPARC needs libatomic otherwise the linker complains about missing symbols due to this GCC bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81358

[vitalybuka]

compiler-rt/cmake/base-config-ix.cmake

194

I am asking which build target it fails, build log would be nice.
I suspect it's some tests?

if this is just GCC bug, maybe if("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")

[glaubitz]

compiler-rt/cmake/base-config-ix.cmake

194

Well, I just remembered, it also happens when building with clang.

It's currently not visible in the logs as it's overshadowed by this bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=27558

I can patch the glibc on the buildbot to work around this issue.

Buildbot is this one: http://lab.llvm.org:8014/#/builders/113

[glaubitz]

compiler-rt/cmake/base-config-ix.cmake

192–197

Sorry for the late reply!

I just gave it a try and added "-latomic" as proposed but it doesn't work and the linker will still complain.

I assume we somehow need to append -latomic to CMAKE_LD_FLAGS?

Shall I just add append("-latomic" CMAKE_LD_FLAGS) unconditionally?

[glaubitz]

compiler-rt/cmake/base-config-ix.cmake

192–197

OK, just passing append("-latomic" CMAKE_LD_FLAGS) doesn't work either. So far, only my original patch works.

Any better suggestions?

[glaubitz]

I just realized that compiler-rt doesn't actually test on 32-bit PowerPC which would explain why we don't see the issue there:

elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "powerpc")
  if(CMAKE_SYSTEM_NAME MATCHES "AIX")
    test_target_arch(powerpc "" "-m32")
  endif()
  test_target_arch(powerpc64 "" "-m64")
elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "s390x")

And since adding append("-latomic" CMAKE_LD_FLAGS) unconditionally didn't work, it just means that this statement doesn't work anyway and the target pointer size check just disabled the test for sparc altogether.

[glaubitz]

Reading through this bug report, it seems we might not be able to use __sync_val_compare_and_swap_8 on 32-bit PowerPC and SPARC:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63368

Should we maybe just change the configuration for sparc and sparcv9 to mimic the one for powerpc and powerpc64?

Thus, the suggested change would just be:

diff --git a/compiler-rt/cmake/base-config-ix.cmake b/compiler-rt/cmake/base-config-ix.cmake
index 8526252dc93a..126ea156f0b8 100644
--- a/compiler-rt/cmake/base-config-ix.cmake
+++ b/compiler-rt/cmake/base-config-ix.cmake
@@ -220,7 +220,6 @@ macro(test_targets)
     elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "s390x")
       test_target_arch(s390x "" "")
     elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "sparc")
-      test_target_arch(sparc "" "-m32")
       test_target_arch(sparcv9 "" "-m64")
     elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "mipsel|mips64el")
       # Gcc doesn't accept -m32/-m64 so we do the next best thing and use

[glaubitz]

More research reveals that the __sync_ built-ins are actually legacy stuff:

https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html

which means that __sync_val_compare_and_swap_8 is not going to be implemented for 32-bit PowerPC and SPARC.

So, I guess we have no other option to either switch the code to the new functions:

https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html

or just disable the test for 32-bit SPARC.

[glaubitz]

Replaced by https://reviews.llvm.org/D108197

[ro]

In D98575#2632907, @glaubitz wrote:

In D98575#2632875, @ro wrote:

Sorry to chime in so late, but I believe this patch has several problems:

• For one, it's at best a partial solution to the problem of atomics on 32-bit SPARC: those are bound to also occur outside of the LLVM tree and this patch does nothing to handle that.

Not sure I understand that particular problem. Why would we care what's happening outside LLVM?

Because user code may use atomics just like compiler-rt? Or imagine you port another part of LLVM that uses atomics, e.g.openmp: will you add another instance of your hack there (and the next lib and the next) instead of going for a general solution?

In the case of user code, the user experience you produce is horrible: users get link failures and have to figure out all by themselves what's wrong and how to work around the issue.

• Even so, it's unclear what CPUs are supposed to run the generated 32-bit code: if it's only supposed to run on SPARC V9 CPUs, why not use the same solution I did for Solaris/sparcv9 in D86621: always default to -mcpu=v9? If the code can also be run on V8 CPUs, the patch is wrong: if run there, the V9 insns will cause the resulting binaries to randomly crash with SIGILL: not exactly a good user experience. In that case, the driver should link with --as-needed -latomic --no-as-needed instead.
• Besides, I don't see the point of linking with -latomic when you already pass -mcpu=v9. It shouldn't be necessary and is certainly wrong on Solaris/sparcv9 where it only adds an unnecessary dependency.

You've left this crucial question unanswered, but the whole direction of solving the issue depends on it. Reading Debian SPARC Port suggests that the 32-bit Linux/sparc kernel has been abandoned long ago, which means you can assume an UltraSPARC/SPARC V9 cpu. In that case, you can default clang -m32 to -mcpu=v9, just on Solaris/sparc, and be done with it because V9 dues support the insns necessary for atomics support even in 32-bit mode.

The -latomic is actually necesary even with -mcpu=v9 since it will still complaing about `atomic_load_8`` being unresolved. But I think the easiest fix will be just to link against libatomic and drop the `-mcpu=v9`` again.

Instead, you should have investigated where those references are coming from rather than blindly adding -latomics. The Solaris/sparcv9 port with -m32 works just fine without, so there shouldn't be an immediate reason why that cannot work on Linux.

[glaubitz]

In D98575#2949331, @ro wrote:

• Even so, it's unclear what CPUs are supposed to run the generated 32-bit code: if it's only supposed to run on SPARC V9 CPUs, why not use the same solution I did for Solaris/sparcv9 in D86621: always default to -mcpu=v9? If the code can also be run on V8 CPUs, the patch is wrong: if run there, the V9 insns will cause the resulting binaries to randomly crash with SIGILL: not exactly a good user experience. In that case, the driver should link with --as-needed -latomic --no-as-needed instead.
• Besides, I don't see the point of linking with -latomic when you already pass -mcpu=v9. It shouldn't be necessary and is certainly wrong on Solaris/sparcv9 where it only adds an unnecessary dependency.

You've left this crucial question unanswered, but the whole direction of solving the issue depends on it. Reading Debian SPARC Port suggests that the 32-bit Linux/sparc kernel has been abandoned long ago, which means you can assume an UltraSPARC/SPARC V9 cpu. In that case, you can default clang -m32 to -mcpu=v9, just on Solaris/sparc, and be done with it because V9 dues support the insns necessary for atomics support even in 32-bit mode.

Well, for one, GCC still defaults to SPARCv7 for 32-bit SPARC which is why it would a big jump if LLVM stopped doing so.

And, secondly, Debian alone shouldn't dictate the way LLVM is configured. The wiki page you linked above is related to the 32-bit Debian SPARC port which isn't currently actively maintained, so it doesn't matter. The upstream kernel, on the other hand, still supports older 32-bit SPARC variants.

Furthermore, LLVM supports other SPARC architectures like LEON which are not v9 which is why I'm not sure whether it would be a good choice if we enforced SPARC v9 for all 32-bit Linux targets.

But I would be interested in testing your suggested change. Could you maybe provide a patch unless it's something more complex to achieve?

Thanks!

[glaubitz]

Oh, another question on top: Does OpenMP work without issues on Solaris/SPARC?

We currently have it disabled on Debian/sparc64, although I don't remember the reason:

https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/snapshot/debian/rules#L210

[glaubitz]

@ro Do you have a patch for your suggested baseline raise?

[ro]

In D98575#2954497, @glaubitz wrote:

@ro Do you have a patch for your suggested baseline raise?

I've already mentioned it in my first comment: D86621. It had worked fine for quite some time, but has only recently been broken again by 960cb490dd16961c61b541efbcc95eb085464ad8 (sanitizer_common: replace RWMutex/BlockingMutex with Mutex) where link errors due to undefined references to __atomic_load_8 occured.

It turns out that clang -m32 -mcpu=v9 is buggy in several respects:

• MaxAtomicInlineWidth isn't set correctly
• The casx insn, which gcc uses for 8-byte __sync_val_compare_and_swap_8 incorrectly isn't enabled, but restricted to -m64 only

Even with tentative patches for both issues, I get an internal error which I've not yet analyzed.

gcc gets this all right; given the bad state of Sparc in LLVM, it's best to try and compile the same source with clang and gcc in case of problems: gcc is almost guaranteed to be right, given that it has an active sparc maintainer while LLVM lacks one.

Please note that I'm on vacation for two weeks and thus unlikely to respond much if at all during that time.

[jrtc27]

In D98575#2954804, @ro wrote:

In D98575#2954497, @glaubitz wrote:

@ro Do you have a patch for your suggested baseline raise?

I've already mentioned it in my first comment: D86621. It had worked fine for quite some time, but has only recently been broken again by 960cb490dd16961c61b541efbcc95eb085464ad8 (sanitizer_common: replace RWMutex/BlockingMutex with Mutex) where link errors due to undefined references to __atomic_load_8 occured.

It turns out that clang -m32 -mcpu=v9 is buggy in several respects:

• MaxAtomicInlineWidth isn't set correctly
• The casx insn, which gcc uses for 8-byte __sync_val_compare_and_swap_8 incorrectly isn't enabled, but restricted to -m64 only

Even with tentative patches for both issues, I get an internal error which I've not yet analyzed.

gcc gets this all right; given the bad state of Sparc in LLVM, it's best to try and compile the same source with clang and gcc in case of problems: gcc is almost guaranteed to be right, given that it has an active sparc maintainer while LLVM lacks one.

I'm not convinced by that. Changing whether or not you use a libcall is an ABI break, no? A libcall implementation based on a (hash) table of spin locks isn't compatible with an inline atomic instruction, e.g. a load or store in the libcall version could tear, which is fine only if everyone uses the libcall version.

[glaubitz]

I tried adding -mcpu=v9 to the default build flags now:

diff --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 0c3419390c27..15045b42d877 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -341,6 +341,11 @@ if( LLVM_ENABLE_PIC )
   endif()
 endif()
 
+if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux" AND LLVM_NATIVE_ARCH STREQUAL "Sparc")
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -mcpu=v9")
+  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -mcpu=v9")
+endif()
+
 if(NOT WIN32 AND NOT CYGWIN AND NOT (${CMAKE_SYSTEM_NAME} MATCHES "AIX" AND CMAKE_CXX_COMPILER_ID STREQUAL "GNU"))
   # MinGW warns if -fvisibility-inlines-hidden is used.
   # GCC on AIX warns if -fvisibility-inlines-hidden is used.

Although -mcpu=v9 is passed to the compiler in this case, I'm still getting the same error:

/usr/bin/ld: projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommon.sparc.dir/sanitizer_libignore.cpp.o: in function `bool __sanitizer::atomic_compare_exchange_strong<__sanitizer::atomic_uint64_t>(__sanitizer::atomic_uint64_t volatile*, __sanitizer::atomic_uint64_t::Type*, __sanitizer::atomic_uint64_t::Type, __sanitizer::memory_order)':
/home/glaubitz/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:80: undefined reference to `__sync_val_compare_and_swap_8'
/usr/bin/ld: /home/glaubitz/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:80: undefined reference to `__sync_val_compare_and_swap_8'
/usr/bin/ld: /home/glaubitz/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:80: undefined reference to `__sync_val_compare_and_swap_8'
/usr/bin/ld: /home/glaubitz/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:80: undefined reference to `__sync_val_compare_and_swap_8'
/usr/bin/ld: /home/glaubitz/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:80: undefined reference to `__sync_val_compare_and_swap_8'
/usr/bin/ld: projects/compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommon.sparc.dir/sanitizer_libignore.cpp.o:/home/glaubitz/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:80: more undefined references to `__sync_val_compare_and_swap_8' follow
collect2: error: ld returned 1 exit status

This is building stage1 with GCC. Shouldn't that work?

[glaubitz]

This comment was removed by glaubitz.

[glaubitz]

In D98575#2954804, @ro wrote:

Please note that I'm on vacation for two weeks and thus unlikely to respond much if at all during that time.

Any chance we can get back to this? I agree we should use the same as it's being used on Solaris. But I don't know how to implement it, I tried to mimic your patch for Linux but that didn't work.

[ro]

In D98575#2954829, @jrtc27 wrote:

I'm not convinced by that. Changing whether or not you use a libcall is an ABI break, no? A libcall implementation based on a (hash) table of spin locks isn't compatible with an inline atomic instruction, e.g. a load or store in the libcall version could tear, which is fine only if everyone uses the libcall version.

This won't be a issue for the Solaris and Debian/sparc64 cases: libatomic on 32-bit SPARC defaults to -mcpu=v9 unless gcc is configured with --with-cpu. Besides, I've verified that the 32-bit libatomic.so on both Solaris/SPARC and Debian/sparc64 uses casx in its __sync_val_compare_and_swap_8 implementation.

You're certainly right, though, that one has to be careful for which platforms the v9 default for 32-bit SPARC can be enabled. It's certainly safe for Solaris, but I have no idea which Linux distributions can or cannot use it.

[ro]

In D98575#3036296, @glaubitz wrote:

In D98575#2954804, @ro wrote:

Please note that I'm on vacation for two weeks and thus unlikely to respond much if at all during that time.

Any chance we can get back to this? I agree we should use the same as it's being used on Solaris. But I don't know how to implement it, I tried to mimic your patch for Linux but that didn't work.

Please reread what I wrote above: defaulting to -mcpu=v9 on 32-bit SPARC worked for quite some time until 960cb490dd16961c61b541efbcc95eb085464ad8 doubled the size of Mutex, requiring 64-bit atomics which clang unlike gcc incorrectly doesn't inline. So right now my previous patch is not enough.

That said, I'm unlikely to come back to this any time soon for two reasons:

• During the GCC 11 release cycle I massively neglected my GCC Solaris maintainer duties, working on various LLVM issues instead with little gain in the end. I'm not going to repeat this for GCC 12: stage 3 is upcoming and I intend to concentrate on that.
• Besides, fixing LLVM to inline 64-bit atomics requires codegen changes. The last time I ventured into that area (make LLVM support 128-bit long double on 32-bit SPARC following the SysV SPARC psABI) took much time, but amounted to nothing in the end. It seems this area is beyond my abilities.

[glaubitz]

Hello!

I found some time again to work on this.

In D98575#2632875, @ro wrote:

Sorry to chime in so late, but I believe this patch has several problems:

• For one, it's at best a partial solution to the problem of atomics on 32-bit SPARC: those are bound to also occur outside of the LLVM tree and this patch does nothing to handle that.
• Even so, it's unclear what CPUs are supposed to run the generated 32-bit code: if it's only supposed to run on SPARC V9 CPUs, why not use the same solution I did for Solaris/sparcv9 in D86621: always default to -mcpu=v9? If the code can also be run on V8 CPUs, the patch is wrong: if run there, the V9 insns will cause the resulting binaries to randomly crash with SIGILL: not exactly a good user experience. In that case, the driver should link with --as-needed -latomic --no-as-needed instead.
• Besides, I don't see the point of linking with -latomic when you already pass -mcpu=v9. It shouldn't be necessary and is certainly wrong on Solaris/sparcv9 where it only adds an unnecessary dependency.

So, it turns out that on Debian, GCC already defaults to "-mv8plus", so the baseline should be 32-bit SPARCv9.

However, the linker is still complaining about the missing __sync_val_compare_and_swap_8 when building with the system compiler (which is GCC):

/home/glaubitz/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:80: undefined reference to `__sync_val_compare_and_swap_8'
/usr/bin/ld: /home/glaubitz/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:80: undefined reference to `__sync_val_compare_and_swap_8'
/usr/bin/ld: /home/glaubitz/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:80: undefined reference to `__sync_val_compare_and_swap_8'
/usr/bin/ld: /home/glaubitz/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:80: undefined reference to `__sync_val_compare_and_swap_8'
/usr/bin/ld: /home/glaubitz/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:80: undefined reference to `__sync_val_compare_and_swap_8'

I honestly have no clue what the problem is.

[glaubitz]

And looking at this particular GCC bug [1] again, the main problem seems that we are using __sync_val_compare_and_swap() which doesn't support 64-bit values.

So, we can only either turn the test off on 32-bit SPARC or switch everything to __atomic_compare_exchange().

PS: We can still raise the baseline to v9 on 32-bit SPARC.

[glaubitz]

This simple hack makes it build:

diff --git a/compiler-rt/cmake/base-config-ix.cmake b/compiler-rt/cmake/base-config-ix.cmake
index d7b0124f3546..45f043d23f74 100644
--- a/compiler-rt/cmake/base-config-ix.cmake
+++ b/compiler-rt/cmake/base-config-ix.cmake
@@ -206,7 +206,7 @@ macro(test_targets)
     elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "s390x")
       test_target_arch(s390x "" "")
     elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "sparc")
-      test_target_arch(sparc "" "-m32")
+      test_target_arch(sparc "" "-m32 -latomic")
       test_target_arch(sparcv9 "" "-m64")
     elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "mipsel|mips64el")
       # Gcc doesn't accept -m32/-m64 so we do the next best thing and use
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h b/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h
index fc13ca52dda7..bfe30c775663 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h
@@ -77,7 +77,7 @@ inline bool atomic_compare_exchange_strong(volatile T *a, typename T::Type *cmp,
   typedef typename T::Type Type;
   Type cmpv = *cmp;
   Type prev;
-  prev = __sync_val_compare_and_swap(&a->val_dont_use, cmpv, xchg);
+  prev = __atomic_compare_exchange(&a->val_dont_use, &cmpv, &xchg, false, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);
   if (prev == cmpv) return true;
   *cmp = prev;
   return false;

which is in line with what is reported here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63368

[glaubitz]

Created a bug report for this now on Github: https://github.com/llvm/llvm-project/issues/53337

[glaubitz]

I made another discovery which explains why passing -m32 -mcpu=v9 doesn't help.

For GCC, V8+ and 32-bit V9 are apparently not the same:

glaubitz@gcc202:~$ echo | gcc -mv8plus -E -dM -|grep -i SWAP
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
glaubitz@gcc202:~$ echo | gcc -mcpu=v9 -m32 -E -dM -|grep -i SWAP
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
glaubitz@gcc202:~$

[glaubitz]

After more research, it turned out that the reason for the lack of 64-bit
atomic operations on 32-bit SPARC was that gcc does not enable these
on this target unless "-mvplus8" is passed in the compiler flags. Even
setting the CPU type to V9 will not enable 64-bit atomic operations by
default.

Thus, passing "-mv8plus" when building compiler-rt allows the build to
succeed during stage1. For the stage2 build, we will still need to set the
default baseline for 32-bit SPARC on Linux to V9 similar to D86621.

[glaubitz]

Hmm, it looks like the testsuite is stumbling over this change as it actually tries to run clang in g++-mode without understanding the -mv8plus flag:

--

********************
UNSUPPORTED: UBSan-Standalone-sparcv9 :: TestCases/Integer/summary.cpp (79753 of 79896)
FAIL: UBSan-Standalone-sparc :: TestCases/TypeCheck/misaligned.cpp (79754 of 79896)
******************** TEST 'UBSan-Standalone-sparc :: TestCases/TypeCheck/misaligned.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';      /home/glaubitz/llvm-project/build/./bin/clang  --driver-mode=g++  -m32 -mv8plus  -gline-tables-only -fsanitize=alignment /home/glaubitz/llvm-project/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp -O3 -o /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp
: 'RUN: at line 2';    /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp l0 &&  /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp s0 &&  /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp r0 &&  /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp m0 &&  /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp f0 &&  /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp n0 &&  /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp u0
: 'RUN: at line 3';    /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp l1 2>&1 | FileCheck /home/glaubitz/llvm-project/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp --check-prefix=CHECK-LOAD --strict-whitespace
: 'RUN: at line 4';    /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp r1 2>&1 | FileCheck /home/glaubitz/llvm-project/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp --check-prefix=CHECK-REFERENCE
: 'RUN: at line 5';    /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp m1 2>&1 | FileCheck /home/glaubitz/llvm-project/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp --check-prefix=CHECK-MEMBER
: 'RUN: at line 6';    /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp f1 2>&1 | FileCheck /home/glaubitz/llvm-project/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp --check-prefix=CHECK-MEMFUN
: 'RUN: at line 7';    /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp n1 2>&1 | FileCheck /home/glaubitz/llvm-project/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp --check-prefix=CHECK-NEW
: 'RUN: at line 8';    /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp u1 2>&1 | FileCheck /home/glaubitz/llvm-project/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp --check-prefix=CHECK-UPCAST
: 'RUN: at line 9';   env UBSAN_OPTIONS=print_stacktrace=1  /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp l1 2>&1 | FileCheck /home/glaubitz/llvm-project/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp --check-prefix=CHECK-LOAD --check-prefix=CHECK-STACK-LOAD
: 'RUN: at line 11';      /home/glaubitz/llvm-project/build/./bin/clang  --driver-mode=g++  -m32 -mv8plus  -fsanitize=alignment -fno-sanitize-recover=alignment /home/glaubitz/llvm-project/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp -O3 -o /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp
: 'RUN: at line 12';   not  /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp s1 2>&1 | FileCheck /home/glaubitz/llvm-project/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp --check-prefix=CHECK-STORE
: 'RUN: at line 13';   not  /home/glaubitz/llvm-project/build/projects/compiler-rt/test/ubsan/Standalone-sparc/TestCases/TypeCheck/Output/misaligned.cpp.tmp w1 2>&1 | FileCheck /home/glaubitz/llvm-project/compiler-rt/test/ubsan/TestCases/TypeCheck/misaligned.cpp --check-prefix=CHECK-WILD
--
Exit Code: 1

Command Output (stderr):
--
clang-14: error: unknown argument: '-mv8plus'

--

[glaubitz]

OK, it seems we can use -mcpu=ultrasparc3 instead which is also understood by clang:

glaubitz@gcc202:~$ echo | gcc -m32 -mcpu=ultrasparc3  -E -dM -|grep -i SWAP
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
glaubitz@gcc202:~$ echo | gcc -m32 -mcpu=v9  -E -dM -|grep -i SWAP
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
glaubitz@gcc202:~$

[glaubitz]

Use "-mcpu=ultrasparc3" instead of "-mv8plus" as the latter will break the stage1 testsuite with clang.

[glaubitz]

FWIW, gcc actually behaves differently on Solaris:

sysadmin@deimos:~$ echo | gcc -m32 -mcpu=v9 -E -dM -|grep -i SWAP
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
sysadmin@deimos:~$ echo | gcc -m32 -mcpu=ultrasparc3 -E -dM -|grep -i SWAP
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
sysadmin@deimos:~$

while on Linux:

glaubitz@gcc202:~$ echo | gcc -m32 -mcpu=v9 -E -dM -|grep -i SWAP
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
glaubitz@gcc202:~$ echo | gcc -m32 -mcpu=ultrasparc3 -E -dM -|grep -i SWAP
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
#define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
glaubitz@gcc202:~$

[glaubitz]

gcc might be fixed in the future to enable 64-bit atomic operations with -m32 -mcpu=v9, see [1].

In the meantime, it would be nice if we could get this change merged since it fixes the LLVM stage1 build with the current gcc versions on Linux without breaking anything else.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104189

[glaubitz]

OK, so gcc is going to enable V8plus for 32-bit V9 as well. So we can just drop this patch ;-).

[glaubitz]

GCC has now been updated to use the V8+ baselone on sparc64 in 32-bit mode, so we can drop this patch altogether.

https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=23987912ddb4207de0714d81237f93f613557d1f

compiler-rt/cmake/base-config-ix.cmake

     elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "s390x")
       test_target_arch(s390x "" "")
     elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "sparc")
-      test_target_arch(sparc "" "-m32")
-      test_target_arch(sparcv9 "" "-m64")
+      if (CMAKE_SIZEOF_VOID_P EQUAL 4)
+        test_target_arch(sparc "" "-m32")
+        append("-latomic" CMAKE_LD_FLAGS)
+      else()
+        test_target_arch(sparcv9 "" "-m64")
+      endif()
     elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "mipsel|mips64el")
       # Gcc doesn't accept -m32/-m64 so we do the next best thing and use
       # -mips32r2/-mips64r2. We don't use -mips1/-mips3 because we want to match