[libcxx-commits] [clang-tools-extra] [llvm] [libc] [flang] [compiler-rt] [libcxx] [clang] [AMDGPU] Fix lack of LDS DMA check in the AA handling (PR #75249)

[Stanislav Mekhanoshin via libcxx-commits]

https://github.com/rampitec updated https://github.com/llvm/llvm-project/pull/75249

>From 82606c4447e8aa8edde90ed420f1c48707967695 Mon Sep 17 00:00:00 2001
From: Stanislav Mekhanoshin <Stanislav.Mekhanoshin at amd.com>
Date: Tue, 12 Dec 2023 13:45:47 -0800
Subject: [PATCH 1/3] [AMDGPU] Fix lack of LDS DMA check in the AA handling

SIInstrInfo::areMemAccessesTriviallyDisjoint does a DS offset
checks, but does not account for LDS DMA instructions. Added
these checks. Without it code falls through and returns true
which is wrong. As a result mayAlias would always return false
for LDS DMA and a regular LDS instruction or 2 LDS DMA instructions.

At the moment this is NFCI because we do not use this AA in a
context which may touch LDS DMA instructions. This is also
unreacheable now because of the ordered memory ref checks just
above in the function and LDS DMA is marked as volatile. This
volatile marking is removed in PR #75247, therefore I'd submit
this check before #75247.
---
 llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | 4 ++--
 llvm/lib/Target/AMDGPU/SIInstrInfo.h   | 8 ++++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index d4e4526795f3b3..c485eb299d52a3 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -3656,8 +3656,8 @@ bool SIInstrInfo::areMemAccessesTriviallyDisjoint(const MachineInstr &MIa,
   // underlying address space, even if it was lowered to a different one,
   // e.g. private accesses lowered to use MUBUF instructions on a scratch
   // buffer.
-  if (isDS(MIa)) {
-    if (isDS(MIb))
+  if (isDS(MIa) || isLDSDMA(MIa)) {
+    if (isDS(MIb) || isLDSDMA(MIb))
       return checkInstOffsetsDoNotOverlap(MIa, MIb);
 
     return !isFLAT(MIb) || isSegmentSpecificFLAT(MIb);
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.h b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
index e794d8cf7cc220..97800bda775cda 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.h
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.h
@@ -546,6 +546,14 @@ class SIInstrInfo final : public AMDGPUGenInstrInfo {
     return get(Opcode).TSFlags & SIInstrFlags::DS;
   }
 
+  static bool isLDSDMA(const MachineInstr &MI) {
+    return isVALU(MI) && (isMUBUF(MI) || isFLAT(MI));
+  }
+
+  bool isLDSDMA(uint16_t Opcode) {
+    return isVALU(Opcode) && (isMUBUF(Opcode) || isFLAT(Opcode));
+  }
+
   static bool isGWS(const MachineInstr &MI) {
     return MI.getDesc().TSFlags & SIInstrFlags::GWS;
   }

>From d8d9f3aab2d2fff2911a99d096685e78faf3d917 Mon Sep 17 00:00:00 2001
From: Stanislav Mekhanoshin <Stanislav.Mekhanoshin at amd.com>
Date: Wed, 13 Dec 2023 11:42:10 -0800
Subject: [PATCH 2/3] Bail early in areMemAccessesTriviallyDisjoint

---
 llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 57eaefd41b2622..31669764144530 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -3651,6 +3651,9 @@ bool SIInstrInfo::areMemAccessesTriviallyDisjoint(const MachineInstr &MIa,
   if (MIa.hasOrderedMemoryRef() || MIb.hasOrderedMemoryRef())
     return false;
 
+  if (isLDSDMA(MIa) || isLDSDMA(MIb))
+    return false;
+
   // TODO: Should we check the address space from the MachineMemOperand? That
   // would allow us to distinguish objects we know don't alias based on the
   // underlying address space, even if it was lowered to a different one,

>From 609be418b81f6ce8c9b323f60636af01f862a994 Mon Sep 17 00:00:00 2001
From: Stanislav Mekhanoshin <Stanislav.Mekhanoshin at amd.com>
Date: Wed, 13 Dec 2023 11:45:50 -0800
Subject: [PATCH 3/3] Remove old code

---
 llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 31669764144530..d05d3c6996261f 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -3659,8 +3659,8 @@ bool SIInstrInfo::areMemAccessesTriviallyDisjoint(const MachineInstr &MIa,
   // underlying address space, even if it was lowered to a different one,
   // e.g. private accesses lowered to use MUBUF instructions on a scratch
   // buffer.
-  if (isDS(MIa) || isLDSDMA(MIa)) {
-    if (isDS(MIb) || isLDSDMA(MIb))
+  if (isDS(MIa)) {
+    if (isDS(MIb))
       return checkInstOffsetsDoNotOverlap(MIa, MIb);
 
     return !isFLAT(MIb) || isSegmentSpecificFLAT(MIb);