efriedma 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: > 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@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@gcc202:~$ gcc -m32 -mv8plus atomic.c -o atomic > > > > > > > glaubitz@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`? The hard part is adding support for using 64-bit registers on 32-bit targets, in general. You need to declare the register class correctly, and call addRegisterClass() in SparcISelLowering.cpp to declare that 64-bit integer registers exist. Once you mark the 64-bit registers legal, legalization rules for a bunch of basic operations will change, so you'll need to do a bunch of work to get basic 64-bit arithmetic/load/store/etc. working again. Once you have all that, telling isel to emit 64-bit atomics should be easy. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118021/new/ https://reviews.llvm.org/D118021 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits