Any news on this?

I have created a new revision which works fine for me on mips64el, see: https://reviews.llvm.org/D126418

In D125872#3524240, @gkistanova wrote:

Thanks, Adrian!

In D125572#3516637, @eugenis wrote:

LGTM

Seems fine to me, if fstatat64 is indeed the canonical syscall on sparc.

Could someone land this for me? Thanks!

Maybe after you land the sparc fix.

In D124212#3509781, @jrtc27 wrote:

There's a lot of really quite wonky whitespace reformatting in this diff

This change unfortunately broke the build on Linux/sparc64.

In D111497#3313643, @0x59616e wrote:

@AnnikaCodes if you agree, I can update the diff so that we can land this ASAP.

Issue has been fixed in GCC, we can drop this change now.

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

Any news on this?

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

Thanks, that's actually a workaround I have also used in the past!

Ah, I actually ran into this very problem! Thanks for fixing this.

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

FWIW, gcc actually behaves differently on Solaris:

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

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

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:

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.

Could someone land this for me? Thanks!

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

Minor improvement to the commit description.

Updated the patch as requested and dropped all changes except the removal of sparcv9 and sparcv9__.

Made changes according to review and dropped every change except removing sparcv9 and sparcv9__.

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

This simple hack makes it build:

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.

I found some time again to work on this.

Hi! Any news on this?

In D111497#3086076, @ricky26 wrote:

I'm still getting test failures with this updated patch.

In D111497#3082769, @jrtc27 wrote:

In D111497#3082763, @glaubitz wrote:

In D111497#3082590, @jrtc27 wrote:

That's probably https://github.com/llvm/llvm-project/commit/26b675d65eb2d1f117a39eee965652f590373232. It's in unrelated code so you can ignore it, and the buildbot probably isn't even enabling the experimental M68k backend?

FWIW, the M68k backend is actually tested on a different buildbot: http://lab.llvm.org:8014/#/builders/180

Sure, but that's all for post-commit checks, not pre-commit like the failure here.

In D111497#3082590, @jrtc27 wrote:

That's probably https://github.com/llvm/llvm-project/commit/26b675d65eb2d1f117a39eee965652f590373232. It's in unrelated code so you can ignore it, and the buildbot probably isn't even enabling the experimental M68k backend?

Could someone land this?

It seems we need this change on all architectures except arm, arm64, ppc64el, s390x and x86_64. Those are the only ones which implement NativeRegisterContextLinux_$ARCH.

This failure affects 32-bit PowerPC as well. Can we get this fixed?

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.

@ricky26 Could you also get this fix backported in the LLVM fork of Rust?

As suggested here [1] by @jrtc27, I think the better solution is to rename the enum Kind to KindTy.

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

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

I have updated the patch to disable compiler-rt on Linux only.

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

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.

In D108197#2949312, @ro wrote:

I'm not seeing any comments from you in the previous patch unless I am missing something.

Huh? What about this one for example?

In D108197#2949283, @ro wrote:

So what? Just ping the patch like everyone else. Abandoning it and submitting effectively the same patch anew just makes work harder for the reviewers.

In my case, I just stopped responding at some point because I got the impression that my comments and questions were mostly evaded or ignored. Talking to a brick wall isn't my favourite pastime.

In D108197#2949186, @ro wrote:

Why on earth did you abandon the original patch D98575? This patch is very similar to the last version there. It makes it much harder to review and comment on the issues already raised there.

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

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

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:

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:

In D100148#2881382, @MaskRay wrote:

Since @glaubitz is here:

I want to set LLVM_ENABLE_PER_TARGET_RUNTIME_DIR to on by default so that the libraries for x86-64 will be in lib/clang/13.0.0/lib/x86_64-unknown-linux-gnu/libclang_rt.profile.a instead of lib/clang/13.0.0/lib/linux/libclang_rt.profile-x86_64.a.
Clang driver supports both paths.

In D100148#2880499, @hvdijk wrote:

Updated to use Triple.isX32().

Should we perhaps just merge this as is, ahead of the update to compiler-rt to create x32 objects? For non-x32, this change is NFC, for x32, the change results in a better error message about compiler-rt objects not existing rather than being for the wrong architecture.

In D105457#2874516, @dblaikie wrote:

Any ideas what version of the standard library these buildbots are using? This /looks/ a bit like a bug in the standard library in use, perhaps? (also because it's not showing up in lots of other buildbots/developers, I assume?)

In D105457#2873294, @dyung wrote:

This change I suspect is causing a build failure on a build bot.

https://lab.llvm.org/buildbot/#/builders/110/builds/4827

FAILED: /usr/bin/c++  -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/unittests/AST -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/include -Itools/clang/include -Iinclude -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/include -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googletest/include -I/home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/llvm/utils/unittest/googlemock/include -march=broadwell -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG    -Wno-variadic-macros -fno-exceptions -fno-rtti -UNDEBUG -Wno-suggest-override -std=c++14 -MD -MT tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -MF tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o.d -o tools/clang/unittests/AST/CMakeFiles/ASTTests.dir/StmtPrinterTest.cpp.o -c /home/ssglocal/clang-cmake-x86_64-avx2-linux/clang-cmake-x86_64-avx2-linux/llvm/clang/unittests/AST/StmtPrinterTest.cpp
/tmp/cc8Ycpsn.s: Assembler messages:
/tmp/cc8Ycpsn.s:11414: Error: symbol `_ZNSt17_Function_handlerIFbPKN5clang4StmtEENS0_UlS3_E2_EE9_M_invokeERKSt9_Any_dataOS3_' is already defined
/tmp/cc8Ycpsn.s:11937: Error: symbol `_ZNSt14_Function_base13_Base_managerIN5clangUlPKNS1_4StmtEE2_EE10_M_managerERSt9_Any_dataRKS7_St18_Manager_operation' is already defined
ninja: build stopped: subcommand failed.

Can you take a look?

In D104467#2825937, @gkistanova wrote:

Do you need a help with the commit?

In D104467#2825433, @gkistanova wrote:

LGTM

Please feel free to connect to the staging.

Impressive work. Thanks a lot to everyone involved, especially to sushmaunnibhavi!

@hvdijk Could you also later then commit the change for me? I don't have the necessary rights. Thanks!

In D99988#2811762, @hvdijk wrote:

I've looked into which bits of compiler-rt already work for x32 and it's actually a lot, yet despite that it should not be enabled: what gets built for builtins works, but various functions that should be defined are left out, and the sanitizers that work depend on libunwind, which does not even build. So, I think it's fine to just go ahead and commit this as it then, and we can fix and enable things later on. (I'm working on that btw, but it'll be a while before anything's ready to be submitted.)

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

I could find out quickly whether the ThreadDescriptorSize for SPARC is correct but otherwise LGTM.

@hvdijk I just noticed that the sanitizer defines SANITIZER_X32 for x32, so I assume your patch itself is already correct.

In D100892#2703122, @MaskRay wrote:

@glaubitz I submitted https://cfarm.tetaneutral.net/users/new/ but am waiting for the approval, so I haven't tested this on sparc, just think it should theoretically work.

This seems to have broken the sanitizer build on Linux SPARC: http://lab.llvm.org:8014/#/builders/113/builds/756

In D98537#2684910, @myhsu wrote:

In D98537#2682808, @ricky26 wrote:

@myhsu when you get a moment, can you check that this is all OK now? :)

sorry I missed your last update. LGTM now

In D100148#2683710, @hvdijk wrote:

In D100148#2683325, @glaubitz wrote:

Hi! Can we get this patch merged as-is or do we need anything else?

Sorry, miscommunication. I was thinking you could use this to update D99988 to actually build the relevant compiler-rt bits for x32, and then once both that and this are ready, both can be submitted at the same time. The reason for that is that this is not useful by itself, and if it turns out that D99988 actually requires some other clang change as well that we are not seeing yet, it will be easier to update this if it has not been pushed yet. Do you think it would be better to push this first?

Hi! Can we get this patch merged as-is or do we need anything else?

So, we only need D99988 and this one (D100148) now and the LLVM package will finally build on Debian without any additional tweaks.