[PATCH] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135
     }
+    // LLVM lacks atomics support on 32-bit SPARC, so forcibly link with
+    // libatomic as a workaround.
----------------
glaubitz wrote:
> ro wrote:
> > ro wrote:
> > > glaubitz wrote:
> > > > ro wrote:
> > > > > glaubitz wrote:
> > > > > > joerg wrote:
> > > > > > > This comment is misleading. It's not so much that LLVM doesn't support them, but that SPARCv8 doesn't have the necessary hardware support. The v8+ support is incomplete, which is a related problem though.
> > > > > > As far as I know, 64-bit atomics are supported if you enable V8+ in GCC - without linking against `libatomic`:
> > > > > > 
> > > > > > ```
> > > > > > glaubitz at gcc202:~$ cat atomic.c
> > > > > > #include <stdint.h>
> > > > > > 
> > > > > > int main()
> > > > > > {
> > > > > >   int64_t x = 0, y = 1;
> > > > > >   y = __sync_val_compare_and_swap(&x, x, y);
> > > > > >   return 0;
> > > > > > }
> > > > > > glaubitz at gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic
> > > > > > glaubitz at gcc202:~$
> > > > > > ```
> > > > > I know, that's why I referred to my patch to default `clang` on
> > > > > Solaris/sparc to V8+.  I'll update the comment.
> > > > > 
> > > > > I'd tried to actually fix the underlying issue (`clang` not emitting
> > > > > `casx` with `-m32 -mcpu=v9`), but ran into internal errors and
> > > > > areas of LLVM I know nothing about.  I might post a WIP patch
> > > > > for reference since there are several issues there.
> > > > I think @jrtc27 might be able to give advise here having the knowledge on the Tablegen stuff (if my mind serves me right).
> > > > 
> > > > The disassembly shows in any case that `casx` is being emitted as you say:
> > > > 
> > > > ```
> > > > 69c:   c7 f0 50 02     casx  [ %g1 ], %g2, %g3
> > > > ```
> > > Of course it does, thus my reference to `unlike gcc` in the
> > > summary.  What Joerg meant, I believe, is that V8+ support
> > > **in LLVM** is incomplete.
> > Right, `gcc` does all of this correctly.  One LLVM issue, e.g., is
> > that it handles `casx` as 64-bit only (cf. `SparcInstr64Bit.td`)
> > while it should be guarded by `HasV9` instead.
> Interesting. Would `SparcInstr64Bit.td` actually get used when targeting 32-bit SPARC?
> 
> If yes, it looks like replacing the guards `Is64Bit` with `HasV9` should do the trick, shouldn't it?
Ah, the header of `SparcInstr64Bit.td` actually has the following comment:

```
// This file contains instruction definitions and patterns needed for 64-bit
// code generation on SPARC v9.
//
// Some SPARC v9 instructions are defined in SparcInstrInfo.td because they can
// also be used in 32-bit code running on a SPARC v9 CPU.
```

So, I guess move the 64-bit atomics stuff out of `SparcInstr64.td` and guard it with `HasV9` instead of `Is64Bit`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118021