[compiler-rt] Disable compiler-rt on 32-bit Linux SPARC

[glaubitz]

Summary

compiler-rt uses atomic built-ins such as sync_val_compare_and_swap()
which are not available for 64-bit values on 32-bit SPARC. Since these
built-ins are considered legacy by GCC upstream and have been replaced
by the atomic_* built-ins, there are no plans to implement a 64-bit
version of __sync_val_compare_and_swap() for 32-bit SPARC (PR c/63368).

Thus, disable compiler-rt on 32-bit SPARC for the time being which is
what the build system already does for 32-bit PowerPC where compiler-rt
isn't built either.

[ro]

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.

That said, this patch is unacceptable: contrary to the title, it disables 32-bit sparc support for all targets, not just Linux. This is both wrong
(things work perfectly fine on Solaris/sparcv9) and unnecessary.

I'll make further comments in D98575 to preserve the context.

[glaubitz]

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.

Because the discussion there was stuck and I did not receive any more feedback.

That said, this patch is unacceptable: contrary to the title, it disables 32-bit sparc support for all targets, not just Linux. This is both wrong
(things work perfectly fine on Solaris/sparcv9) and unnecessary.

Well, I can rework this to limit this change to Linux only.

[ro]

In D108197#2949215, @glaubitz wrote:

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.

Because the discussion there was stuck and I did not receive any more feedback.

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.

That said, this patch is unacceptable: contrary to the title, it disables 32-bit sparc support for all targets, not just Linux. This is both wrong
(things work perfectly fine on Solaris/sparcv9) and unnecessary.

Well, I can rework this to limit this change to Linux only.

Please wait: I suspect we can use a completely different approach (the Solaris one) on Linux, too.

[glaubitz]

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.

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

[ro]

In D108197#2949305, @glaubitz wrote:

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.

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

Huh? What about this one for example?

[glaubitz]

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?

Ah, it was hidden by the newer revision ...

[glaubitz]

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

But I'm just posting this here for the sake of completeness and I'm happy to continue the discussion in the other review thread.

[vitalybuka]

I don't have an opinion on SPARC 32bit support

[glaubitz]

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

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

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

       test_target_arch(powerpc64 "" "-m64")
     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")
+    elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "sparc|sparcv9")
+      if (NOT "${CMAKE_SYSTEM_NAME}" MATCHES "Linux")
+         test_target_arch(sparc "" "-m32")
+      endif()
       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