[PATCH] D118024: [sanitizer_common] Use atomic builtin in sanitizer_atomic_clang.h

[Rainer Orth via Phabricator via llvm-commits]

ro marked an inline comment as done.
ro added a comment.

I've meanwhile found that the 3 regressions mentioned in the summary are unrelated: I had updated versions of D91607 <https://reviews.llvm.org/D91607> and D91608 <https://reviews.llvm.org/D91608> in my
tree (still unreviewed after more than a year): the latter of those enables `sanitizer_common` testing on sparc.  The `DenseMapCustomTest.DefaultMinReservedSizeTest` is new since the patches were originally submitted and AFAICS the failure happens because sparc uses 8k pages instead of 4k e.g. on x86.  The CompactRingBuffer.int64 failures is worked around in D91617 <https://reviews.llvm.org/D91617>.



================
Comment at: compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:94
 // proceed the template definitions above.
 #if defined(_MIPS_SIM) && defined(_ABIO32)
   #include "sanitizer_atomic_clang_mips.h"
----------------
ro wrote:
> efriedma wrote:
> > Have you looked at extending this code to SPARC?  I'm a little concerned that the dependency on libatomic might cause issues (however we write it).
> Not yet: I'd hoped to be able to come up with a minimal workaround for LLVM not fully implementing atomics on SPARC v8+ (or even better, fix that problem instead).
> 
> Switching from `__sync_val_compare_and_swap` to `__atomic_compare_exchange` shouldn't make a difference for any target inlining atomics IIUC.  Those that don't would just trade link failures for one function for another.
I've now tried, but results aren't encouraging: the code isn't
target-specific AFAICS, but on sparc I get two regressions compared to the `__atomic_compare_exchange` variant:
```
+  SanitizerCommon-Unit :: ./Sanitizer-sparc-Test/SanitizerCommon.Mutex
+  SanitizerCommon-Unit :: ./Sanitizer-sparc-Test/SanitizerCommon.ThreadRegistryThreadedTest
```
At least one of those never times out and hangs `ninja check-all` indefinitively.  FWIW, I've repeated the same experiment on `amd64-pc-solaris2.11` with the added change of disabling `sanitizer_atomic_clang_mips.h`.  I get the same failures there, and also
```
  SanitizerCommon-Unit :: ./Sanitizer-x86_64-Test/SanitizerCommon.AtomicCompareE
xchangeTest
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118024/new/

https://reviews.llvm.org/D118024