[PATCH] D146425: [SystemZ] Enable AtomicExpandPass for i128

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa updated this revision to Diff 509957.
jonpa added a comment.

- Reworked shouldCoalesce()

The code there for i128 was supposed to simply check for clobbered GR128 regs inside the small region of the two combined live ranges. This was limited to only allow coalescing in small regions (in a single MBB), as it seems (still does) that coalescing for there register pairs easily creates spilling.

As it seemed unacceptable to not coalesce away multiple copies inside a CDSG loop, this was at first attempt extended to allow coalescing in these loops specifically as well, but now this has been merged to simply scan the involved LiveIntervals for phys reg clobbers. This handles the register allocator problem still (to not run out of registers), and also enables a bit more coalescing, including the CDSG loops.

However, this "improved" coalescing now seems to create a bit more spilling again:

main <> patched

  Spill|Reload   :               642820               643901    +1081
  Copies         :              1010858              1010342     -516

In theory, of course it would have been very nice if the register allocator would not run out of registers (still does without this), and also if it could split ("uncoalesce") as needed to minimize spilling. It does unfortunately not, so there remains the option to try to avoid spilling in shouldCoalesce(). Should we keep it simple here, or should we experiment a little and try to at least not increase spilling compared to main? Not sure if there is a natural heuristic that includes the CDSG loop without explicitly checking for that case...

- Comparing to GCC:

Both clang++ and g++ aligns this to 16 bytes:

  std::atomic<__int128> Atomic_int128;         //C++

However, while clang aligns this also to 16 bytes, GCC aligns only to 8, which I suspect is an error by GCC(?):

  Atomic __int128 Atomic_int128;                    //C




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

https://reviews.llvm.org/D146425

Files:
  clang/lib/Basic/Targets/SystemZ.h
  clang/test/CodeGen/SystemZ/atomic-alignment.c
  clang/test/CodeGen/SystemZ/gnu-atomic-builtins-i128-16Al.c
  clang/test/CodeGen/SystemZ/gnu-atomic-builtins-i128-8Al.c
  clang/test/CodeGen/SystemZ/gnu-atomic-builtins-i16.c
  clang/test/CodeGen/SystemZ/gnu-atomic-builtins-i32.c
  clang/test/CodeGen/SystemZ/gnu-atomic-builtins-i64.c
  clang/test/CodeGen/SystemZ/gnu-atomic-builtins-i8.c
  clang/test/CodeGen/SystemZ/gnu-atomic_is_lock_free.c
  llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
  llvm/lib/Target/SystemZ/SystemZISelLowering.h
  llvm/lib/Target/SystemZ/SystemZRegisterInfo.cpp
  llvm/lib/Target/SystemZ/SystemZTargetMachine.cpp
  llvm/test/CodeGen/SystemZ/atomicrmw-ops-i128.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D146425.509957.patch
Type: text/x-patch
Size: 81458 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230331/a4316115/attachment-0001.bin>