[llvm] [SystemZ] Don't lower float/double ATOMIC_[LOAD|STORE] to [LOAD|STORE] (PR #75879)

Jonas Paulsson via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 2 17:20:08 PST 2024


JonPsson1 wrote:

Patch improved

- ATOMIC_LOADs are now converted to LOADs before they are selected by means of morphing so that reg/mem patterns work.

- AtomicExpand pass now supposed to only cast atomic load/store to integer for FP128. However, it seems Clang always does this for all FP types and AtomicExpand does not remove the casts. The bitcasts are handled in the backend instead.
  - ATOMIC_LOAD bitcasts handled in SystemZ combineBITCAST()
  - ATOMIC_STORE bitcasts handled in SystemZ Select()

- Until ATOMIC_LOAD is removed and replaced by LOAD + MMO handling, ATOMIC_LOAD is extended to handle an extension type, just like an ordinary LOAD.
  - When an ATOMIC_LOAD result is widened to a legal type, the proper extension attribute is added (or kept if already present).
  - computeKnownBits() and ComputeNumSignBits() extended to handle ATOMIC_LOAD.
  - SExt and ZExt handled in SystemZ combineZERO_EXTEND() and combineSIGN_EXTEND().

- This patch now keeps ATOMIC_LOADs unexposed to general DAG combining whereas before they were all regular LOADs after legalization.
  - Some SystemZ specific lowering can still be done on ATOMIC_LOADs, like emission of REPLICATE (VLREP).
  - There are some optimizations involving LOADs in SystemZISelLowering that do not handle ATOMIC_LOADS as of now (e.g. compares), but it would not be too much work given that the ATOMIC_LOAD nodes have the right extension type and MemoryVT (also for float) set. 

- Another problem is that the SystemZ instruction selection still sees all the ATOMIC_LOADs as LOADs. To remedy this, I tried first to make the selection of ATOMIC_LOADS explicit by means of PatFrags handling both LOAD and ATOMIC_LOAD, but that became quite a task given that most of the instrutions are atomic if properly aligned.
[ExplicitAtomLdSt.patch.tar.gz](https://github.com/llvm/llvm-project/files/13814987/ExplicitAtomLdSt.patch.tar.gz)
 I then simplified this task by the hack of morphing ATOMIC_LOADs into LOADs in convertATOMIC_LOADs(). This is less effort at the moment, but it also has the benefit that SystemZ isel is based on LOADs and their MMOs, so it will already work if the ATOMIC_LOAD opcode is removed.

- Not sure if there are cases of non-atomic SystemZ instructions being used for atomic memory access. I am a bit suspicious about MAEB and friends which are RXF type instructions. Currently clang uses an MADB with this program, while GCC does not:
```

#include <math.h>
#include <float.h>
#include <atomic>
std::atomic<double> Y(0.0);
double fun(double *S) {
  double x = 2.0,z = 10.0;
  double y = Y.load(std::memory_order_seq_cst);
  return fma(x, y, z);
}

```
I am also not sure if SI type instructions are atomic (POP mentions SIY, so it would be a fair guess that they are). I have not looked through all instruction types, but this would be good to do. So far I have marked the MAEB to only match a load if it is non-atomic.

https://github.com/llvm/llvm-project/pull/75879


More information about the llvm-commits mailing list