[SystemZ] Test improvements for atomic load/store instructions (NFC).

[JonPsson1]

Make sure to test atomic load and store instructions with and w/out natural alignment.

[uweigand]

The IR specs say: "align must be explicitly specified on atomic loads, and the load has undefined behavior if the alignment is not set to a value which is at least the size in bytes of the pointee" -- given that, does it really make sense to add test cases that are undefined IR according to the spec?

[JonPsson1]

Ahh.. maybe not. Maybe then AtomicExpand should not even emit libcalls, but rather assert and abort?

It seems that AtomicExpand decides to handle the unaligned cases with libcall:

// Determine if a particular atomic operation has a supported size,
// and is of appropriate alignment, to be passed through for target
// lowering. (Versus turning into a __atomic libcall)
template <typename Inst>
static bool atomicSizeSupported(const TargetLowering *TLI, Inst *I) {
  unsigned Size = getAtomicOpSize(I);
  Align Alignment = I->getAlign();
  return Alignment >= Size &&
         Size <= TLI->getMaxAtomicSizeInBitsSupported() / 8;
}

SelectionDAGBuilder however rejects this:

void SelectionDAGBuilder::visitAtomicLoad(const LoadInst &I) {
  SDLoc dl = getCurSDLoc();
  AtomicOrdering Order = I.getOrdering();
...
  if (!TLI.supportsUnalignedAtomics() &&
      I.getAlign().value() < MemVT.getSizeInBits() / 8)
    report_fatal_error("Cannot generate unaligned atomic load");
...

So with the notion of supporting unaligned atomics, it's a little unclear to me as indeed the spec says " the load has undefined behavior if the alignment is not set to a value which is at least the size in bytes of the pointee."

If there is library support for the unaligned cases, I guess the libcalls should be allowed. If not, the libcalls should not get emitted by AtomicExpand.

[JonPsson1]

Per our discussion, I reverted the previous test changes for <=64 bits, but updated the 128-bit tests as they actually should have been changed with c568927, since the default alignment for _Atomic long double is now 16 bytes, right?

[jyknight]

If there is library support for the unaligned cases, I guess the libcalls should be allowed. If not, the libcalls should not get emitted by AtomicExpand.

Yes; the non-size-specialized __atomic_* libcalls work on any alignment. AtomicExpandPass's behavior here is correctly/intentionally handling that condition. The IR spec wording should be updated to relax the constraint.

[JonPsson1]

If there is library support for the unaligned cases, I guess the libcalls should be allowed. If not, the libcalls should not get emitted by AtomicExpand.

Yes; the non-size-specialized __atomic_* libcalls work on any alignment. AtomicExpandPass's behavior here is correctly/intentionally handling that condition. The IR spec wording should be updated to relax the constraint.

I made a suggestion here for the spec: #75871

[uweigand]

Let's wait until we have agreement whether or not misaligned atomic load/store is supposed to be supported at the IR level. If yes, we should test the relevant cases for us: which would mean both 8-byte and 16-byte aligned 128-bit accesses.

Also, the related test case improvement from #75879 should be pulled in here: missing checks for serialization, test all 128-bit operations on both z10 and z13.

[JonPsson1]

Now that the spec was updated (#75871), I have updated the tests here again per previous discussion.

[uweigand]

LGTM, thanks!