In D146851#4223144, @jhenderson wrote:

LGTM, but should get the AIX/Solaris people have verified the fix.

Even with this patch, the ScopedPrinterTest.PrintNumber test still FAILs on Solaris:

/vol/llvm/src/llvm-project/dist/llvm/unittests/Support/ScopedPrinterTest.cpp:91: Failure
Expected equality of these values:
[...]
With diff:
@@ -23,5 +23,5 @@
   \"float-max\": 3.4028234663852886e+38,
   \"float-min\": 1.1754943508222875e-38,
-  \"float-inf\": inf,
+  \"float-inf\": infinity,
   \"float-nan\": nan,
   \"float-42.0\": 42,
@@ -29,5 +29,5 @@
   \"double-max\": 1.7976931348623157e+308,
   \"double-min\": 2.2250738585072014e-308,
-  \"double-inf\": inf,
+  \"double-inf\": infinity,
   \"double-nan\": nan,
   \"double-42.0\": 42,

LGTM, thanks.

• You still haven't mentioned which target is affected.
• There needs to be a comment explaining why _TIME_BITS is #undefed.
• Please pass the patch through clang-format-diff.py: there's a TAB before !defined which should just be a blank.

Please state what target the problem occurs on and how/where the patch was tested. _TIME_BITS doesn't exist on Solaris 11.4 and, AFAICS, neither on Illumos.

In D143249#4103303, @tstellar wrote:

LGTM, Thanks, can you open the backport request for release/16.x ?

I forgot: this is certainly something that should go onto the release/16.x branch ASAP.

Ping? It's been a week. Thanks.

The patch is no longer crucial: since third-party/benchmark doesn't build with -Werror anymore, so there are just warnings left. Besides, the patch is now upstream, so whenever someone imports the current version into LLVM, even those will be gone.

I just ran 2-stage builds on sparcv9-sun-solaris2.11 (with both gcc 12.2.0 and clang 15.0.0 in stage 1) : the builds completed successfully and
test results are back to what they were about a month ago (with two unrelated issues). Thanks.

This patch broke Solaris/sparcv9 bootstrap. Details in Issue #59646.

In D91620#3993001, @haowei wrote:

We are seeing following test failures on Fuchsia's Clang Linux x64 builders after this change (255c3e3dcb06299aa2365f70817322a8a381c351) landed:

MemorySanitizer-Unit :: ./Msan-x86_64-Test/34/38
MemorySanitizer-Unit :: ./Msan-x86_64-Test/MemorySanitizer/SelectPartial
MemorySanitizer-Unit :: ./Msan-x86_64-with-call-Test/34/38
MemorySanitizer-Unit :: ./Msan-x86_64-with-call-Test/MemorySanitizer/SelectPartial
MemorySanitizer-Unit :: ./Msan-x86_64-with-call-Test/MemorySanitizer/SelectPartial

I got distracted after returning from vacation, but I'll reland the patch shortly after retesting on x86_64-pc-linux-gnu.

For good measure tested on sparcv9-sun-solaris2.11, amd64-pc-solaris2.11, and x86_64-pc-linux-gnu.

As usual, it's complicated ;-)

In D129156#3949194, @awarzynski wrote:

This introduced a new failure on Solaris:

FAIL: Flang :: Driver/pass-plugin-not-found.f90

Running the failing command manually shows:

error: unable to load plugin 'X.Y': 'Could not load library 'X.Y': ld.so.1: flang-new: X.Y: open failed: No such file or directory'

while the test currently expects

error: unable to load plugin 'X.Y': 'Could not load library 'X.Y': X.Y: cannot open shared object file: No such file or directory'

This expectation is unjustified, I believe: the message is produced by PassPlugin::Load -> sys::DynamicLibrary::getPermanentLibrary -> HandleSet::DLOpen -> ::dlerror. Obviously the output of the last is system-dependent; I don't think we can put any requirements on it (maybe not even the exact wording of the No such file or directory part.

Ping? It's been 2 1/2 weeks...

Disable subtests of tsan/tests/unit/tsan_trace_test.cpp that deadlock with SANITIZER_DEBUG.

In D91620#3851164, @vitalybuka wrote:

Please disregard this. It was something wrong with my setup.

So I applied the patch on the actual bot and still see tsan go timeouts.

I'd be grateful if the owners of the affected builders could retest the patch before relanding, preferably in Release and Debug/COMPILER_RT_DEBUG builds. Thanks a lot.

• Use string(APPEND) for COMPILER_RT_TEST_COMPILER_CFLAGS.
• Omit -O3 from COMPILER_RT_TEST_COMPILER_CFLAGS in non-debug builds for now.
• Provide __sanitizer::integral_constant<bool, true>::value instantiation.
• XFAIL tsan/Linux/check_memcpy.c in debug builds.

I think I've figured out what's wrong. AFAICS there are three separate issues:

In D91620#3836888, @uweigand wrote:

[...]

The -O3;-g on the link line seems to come from compiler-rt/test/lit.common.configured. I suspect that can be traced down to code in compiler-rt/CMakeLists.txt which replaces " " by ";" in COMPILER_RT_TEST_COMPILER_CFLAGS. I don't really understand this and it seems terribly fragile to me, but I believe this can be avoided by prepending the new instances of setting COMPILER_RT_TEST_COMPILER_CFLAGS in compiler-rt/CMakeLists.txt with a blank. I have a patch for that and am trying to test it, but as mentioned the issue hasn't affected me. @uweigand may be able to help.

I don't see anything particularly special to s390x here, with the only exception of -mbackchain. This flag is added to sanitizer tests on s390x, where we need it for the same reason other platforms tend to need -fno-omit-frame-pointer. It's unclear how that would be related to this ";" issue.

This seems to cause another regression, apparently only visible in cross builds: clang-s390x-linux buildbot: a link failure like this:

******************** TEST 'AddressSanitizer-s390x-linux ::
TestCases/Linux/init-order-dlopen.cpp' FAILED ********************
Script:
[...]

This patch caused a regression on the sanitizer-x86_64-linux-autoconf buildbot running a debug build:

ThreadSanitizer-x86_64 :: Linux/check_memcpy.c

The executable now contains calls to memset. AFAICS this is a consequence of changing debug builds from -O1 to -O0 and thus is expected. Therefore I suggest to just add

XFAIL: !compiler-rt-optimized

Unfortunately I missed this in my local builds: I had run Debug builds on x86_64-pc-linux-gnu both with a vanilla tree and my patch, but already the vanilla results were insanely bad (309 FAILs), so I didn't notice the regression.

• Update COMPILER_RT_UNITTEST_CFLAGS as requested.
• Change SANITIZER_COMMON_CFLAGS to -O0 for COMPILER_RT_DEBUG, cannot reliably debug with optimization.
• compiler-rt/lib/sanitizer_common/tests/sanitizer_type_traits_test.cpp tests SanitizerCommon.IsTriviallyDestructible and SanitizerCommon.IsTriviallyCopyable fail to link with debug build:

FAILED: projects/compiler-rt/lib/sanitizer_common/tests/Sanitizer-i386-Test
[...]
Undefined                       first referenced
 symbol                             in file
_ZN11__sanitizer17integral_constantIbLb1EE5valueE SANITIZER_TEST_OBJECTS.sanitizer_type_traits_test.cpp.i386.o
ld: fatal: symbol referencing errors

Maybe one can provide a definition of `__sanitizer::integral_constant<bool, true>::value`?

Given that there were no comments from the SPARC reviewers in more than a week, I've now committed the patch as is.

Never use RLIMIT_RSS on Apple systems.

Run test on Linux/sparc64, too.

In D133561#3817542, @rovka wrote:

In D133561#3808337, @ro wrote:

In D133561#3792286, @rovka wrote:

This doesn't have any tests, could you please add some? If memory holds, you should be able to add some RUN/CHECK lines here: https://github.com/llvm/llvm-project/blob/main/flang/test/Fir/target-rewrite-complex.fir

The new testcase PASSes on sparcv9-sun-solaris2.11 and sparc64-unknown-linux-gnu. However, for 32-bit sparc it runs into an assertion failure

[...]

Not sure what the issue is there. I'm actually a bit surprised that it works for i386. When I tried to enable flang for armv7, MLIR didn't have very good support for 32-bit platforms, so I left it at that. If you know that users of 32-bit sparc really have a great need for a Fortran compiler, then feel free to investigate further, but I'm not sure how many other 32-bit platforms have an interest in flang/mlir, so maintaining it might turn out to be a non-trivial amount of work.

No worries: I could easily keep the patch local to gcc for a while, and LLVM isn't affected anyway (apart from the staging buildmaster being down for more than a week).

In D133561#3792286, @rovka wrote:

This doesn't have any tests, could you please add some? If memory holds, you should be able to add some RUN/CHECK lines here: https://github.com/llvm/llvm-project/blob/main/flang/test/Fir/target-rewrite-complex.fir

Add testcase.

Ping? It's been a weed and a half.

In D133579#3785892, @fhahn wrote:

The mentioned issue talks about Flang (https://github.com/llvm/llvm-project/issues/57642), but the changes seem to all be in C/C++ files?

In D133406#3781465, @MaskRay wrote:

Add comment, tests.

In D133405#3779190, @MaskRay wrote:

So, sparc64 gcc does seem to define the macro by default. (I am using Debian testing and can't tell whether it's distro setting or upstream default) ¯\_(ツ)_/¯

% sparc64-linux-gnu-gcc -E -dM -xc /dev/null -nostdinc | grep NO_INLINE
#define __NO_INLINE__ 1

In D133493#3777633, @MaskRay wrote:

If you filed a bug for the issue, it might be good to refer to it in CMakeLists.txt:)

In D133493#3777132, @fhahn wrote:

Sure, done: Issue #57626.

In D133405#3776096, @MaskRay wrote:

It's not a compiler's job to define this workaround...

In D127964#3737997, @spatel wrote:

Are tests passing on Solaris after 2981a9490277a7920936d287c?

In D127964#3735654, @spatel wrote:

I've looked around some more and it seems the Solaris libm acts within the C standard: all of C99, p.219, C11, p.239, and C17, p.147 state

A domain error may occur if both arguments are zero.

I've also found the atan2 docs on cppreference.com which says the same, adding

If the implementation supports IEEE floating-point arithmetic (IEC 60559),

Retested on amd64-pc-solaris2.11, sparcv9-sun-solaris2.11, and x86_64-pc-linux-gnu.

In D131148#3732341, @rriddle wrote:

In D127964#3732256, @spatel wrote:

The llvm/test/Transforms/EarlyCSE/atan.ll test FAILs on Solaris (both sparcv9 and amd64):

/vol/llvm/src/llvm-project/local/llvm/test/Transforms/EarlyCSE/atan.ll:55:15: error: CHECK-NEXT: expected string not found in input
; CHECK-NEXT: ret float -0.000000e+00
              ^

Comparing to the Linux/x86_64 version of the output, the only difference is

--- /homes/ro/atan.ll.x86_64	2022-08-18 12:38:48.313115000 +0200
+++ /homes/ro/atan.ll.sparcv9	2022-08-18 11:50:22.976866000 +0200
@@ -25,7 +25,8 @@
 }

In D131148#3700505, @mehdi_amini wrote:

Actually that makes sense, this patch does not seem right.