[llvm] [SROA] Prevent load atomic vector from being generated (PR #112432)

via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 17 20:26:45 PDT 2024


https://github.com/jofrn updated https://github.com/llvm/llvm-project/pull/112432

>From cb05b5699e5a65093aaefe26a8128a400c459a9c Mon Sep 17 00:00:00 2001
From: jofernau <Joe.Fernau at amd.com>
Date: Tue, 15 Oct 2024 16:06:27 -0400
Subject: [PATCH 1/5] [SROA] Prevent load atomic vector from being generated

These are illegal, and they can be formed from SROA via indirect
volatile loads in the AllocaSliceRewriter.
---
 llvm/lib/Transforms/Scalar/SROA.cpp        |  5 +++++
 llvm/test/Transforms/SROA/atomic-vector.ll | 19 +++++++++++++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 llvm/test/Transforms/SROA/atomic-vector.ll

diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index 92589ab17da313..450ecdf20ef009 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -2853,6 +2853,11 @@ class AllocaSliceRewriter : public InstVisitor<AllocaSliceRewriter, bool> {
 
   bool visitLoadInst(LoadInst &LI) {
     LLVM_DEBUG(dbgs() << "    original: " << LI << "\n");
+
+    // load atomic vector would be generated, which is illegal
+    if (LI.isAtomic() && NewAI.getAllocatedType()->isVectorTy())
+      return false;
+
     Value *OldOp = LI.getOperand(0);
     assert(OldOp == OldPtr);
 
diff --git a/llvm/test/Transforms/SROA/atomic-vector.ll b/llvm/test/Transforms/SROA/atomic-vector.ll
new file mode 100644
index 00000000000000..d43ae653fba1dd
--- /dev/null
+++ b/llvm/test/Transforms/SROA/atomic-vector.ll
@@ -0,0 +1,19 @@
+; RUN: opt < %s -passes='sroa' -S 2>&1 | FileCheck %s --check-prefix=ERR
+; RUN: opt < %s -passes='sroa' -S | FileCheck %s
+
+define float @atomic_vector() {
+; ERR-NOT: atomic load operand must have integer, pointer, or floating point type!
+; ERR-NOT:   <1 x float>  {{%.*}} = load atomic volatile <1 x float>, ptr {{%.*}} acquire, align 4
+; CHECK:      %1 = alloca <1 x float>, align 4
+; CHECK-NEXT: store <1 x float> undef, ptr %1, align 4
+; CHECK-NEXT: %2 = load atomic volatile float, ptr %1 acquire, align 4
+; CHECK-NEXT: ret float %2
+  %1 = alloca <1 x float>
+  %2 = alloca <1 x float>
+  %3 = alloca ptr
+  call void @llvm.memcpy.p0.p0.i64(ptr %2, ptr %1, i64 4, i1 false)
+  store ptr %2, ptr %3
+  %4 = load ptr, ptr %3
+  %5 = load atomic volatile float, ptr %4 acquire, align 4
+  ret float %5
+}

>From 0c6150556041dc4ede3039a0f00c7395d7c0898c Mon Sep 17 00:00:00 2001
From: jofernau <Joe.Fernau at amd.com>
Date: Tue, 15 Oct 2024 17:16:48 -0400
Subject: [PATCH 2/5] Autogenerate assertions

---
 llvm/test/Transforms/SROA/atomic-vector.ll | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/llvm/test/Transforms/SROA/atomic-vector.ll b/llvm/test/Transforms/SROA/atomic-vector.ll
index d43ae653fba1dd..c81dc25b0ad52a 100644
--- a/llvm/test/Transforms/SROA/atomic-vector.ll
+++ b/llvm/test/Transforms/SROA/atomic-vector.ll
@@ -1,13 +1,13 @@
-; RUN: opt < %s -passes='sroa' -S 2>&1 | FileCheck %s --check-prefix=ERR
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
 ; RUN: opt < %s -passes='sroa' -S | FileCheck %s
 
 define float @atomic_vector() {
-; ERR-NOT: atomic load operand must have integer, pointer, or floating point type!
-; ERR-NOT:   <1 x float>  {{%.*}} = load atomic volatile <1 x float>, ptr {{%.*}} acquire, align 4
-; CHECK:      %1 = alloca <1 x float>, align 4
-; CHECK-NEXT: store <1 x float> undef, ptr %1, align 4
-; CHECK-NEXT: %2 = load atomic volatile float, ptr %1 acquire, align 4
-; CHECK-NEXT: ret float %2
+; CHECK-LABEL: define float @atomic_vector() {
+; CHECK-NEXT:    [[TMP1:%.*]] = alloca <1 x float>, align 4
+; CHECK-NEXT:    store <1 x float> undef, ptr [[TMP1]], align 4
+; CHECK-NEXT:    [[TMP2:%.*]] = load atomic volatile float, ptr [[TMP1]] acquire, align 4
+; CHECK-NEXT:    ret float [[TMP2]]
+;
   %1 = alloca <1 x float>
   %2 = alloca <1 x float>
   %3 = alloca ptr

>From 083346d194bd64f5152a109c6b07e343cd3cf166 Mon Sep 17 00:00:00 2001
From: jofernau <Joe.Fernau at amd.com>
Date: Wed, 16 Oct 2024 09:02:16 -0400
Subject: [PATCH 3/5] Add i32,ptr tests and rename variables

---
 llvm/test/Transforms/SROA/atomic-vector.ll | 51 ++++++++++++++++++----
 1 file changed, 43 insertions(+), 8 deletions(-)

diff --git a/llvm/test/Transforms/SROA/atomic-vector.ll b/llvm/test/Transforms/SROA/atomic-vector.ll
index c81dc25b0ad52a..1b6cc275681a80 100644
--- a/llvm/test/Transforms/SROA/atomic-vector.ll
+++ b/llvm/test/Transforms/SROA/atomic-vector.ll
@@ -8,12 +8,47 @@ define float @atomic_vector() {
 ; CHECK-NEXT:    [[TMP2:%.*]] = load atomic volatile float, ptr [[TMP1]] acquire, align 4
 ; CHECK-NEXT:    ret float [[TMP2]]
 ;
-  %1 = alloca <1 x float>
-  %2 = alloca <1 x float>
-  %3 = alloca ptr
-  call void @llvm.memcpy.p0.p0.i64(ptr %2, ptr %1, i64 4, i1 false)
-  store ptr %2, ptr %3
-  %4 = load ptr, ptr %3
-  %5 = load atomic volatile float, ptr %4 acquire, align 4
-  ret float %5
+  %src = alloca <1 x float>
+  %val = alloca <1 x float>
+  %direct = alloca ptr
+  call void @llvm.memcpy.p0.p0.i64(ptr %val, ptr %src, i64 4, i1 false)
+  store ptr %val, ptr %direct
+  %indirect = load ptr, ptr %direct
+  %ret = load atomic volatile float, ptr %indirect acquire, align 4
+  ret float %ret
+}
+
+define i32 @atomic_vector_int() {
+; CHECK-LABEL: define i32 @atomic_vector_int() {
+; CHECK-NEXT:    [[VAL:%.*]] = alloca <1 x i32>, align 4
+; CHECK-NEXT:    store <1 x i32> undef, ptr [[VAL]], align 4
+; CHECK-NEXT:    [[RET:%.*]] = load atomic volatile i32, ptr [[VAL]] acquire, align 4
+; CHECK-NEXT:    ret i32 [[RET]]
+;
+  %src = alloca <1 x i32>
+  %val = alloca <1 x i32>
+  %direct = alloca ptr
+  call void @llvm.memcpy.p0.p0.i64(ptr %val, ptr %src, i64 4, i1 false)
+  store ptr %val, ptr %direct
+  %indirect = load ptr, ptr %direct
+  %ret = load atomic volatile i32, ptr %indirect acquire, align 4
+  ret i32 %ret
+}
+
+define ptr @atomic_vector_ptr() {
+; CHECK-LABEL: define ptr @atomic_vector_ptr() {
+; CHECK-NEXT:    [[SRC_SROA_0:%.*]] = alloca [4 x i8], align 8
+; CHECK-NEXT:    [[VAL_SROA_0:%.*]] = alloca ptr, align 8
+; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[VAL_SROA_0]], ptr align 8 [[SRC_SROA_0]], i64 4, i1 false)
+; CHECK-NEXT:    [[VAL_SROA_0_0_VAL_SROA_0_0_RET:%.*]] = load atomic volatile ptr, ptr [[VAL_SROA_0]] acquire, align 4
+; CHECK-NEXT:    ret ptr [[VAL_SROA_0_0_VAL_SROA_0_0_RET]]
+;
+  %src = alloca <1 x ptr>
+  %val = alloca <1 x ptr>
+  %direct = alloca ptr
+  call void @llvm.memcpy.p0.p0.i64(ptr %val, ptr %src, i64 4, i1 false)
+  store ptr %val, ptr %direct
+  %indirect = load ptr, ptr %direct
+  %ret = load atomic volatile ptr, ptr %indirect acquire, align 4
+  ret ptr %ret
 }

>From cf475055717cfca041ea204f9ec4053a9ed5afaf Mon Sep 17 00:00:00 2001
From: jofernau <Joe.Fernau at amd.com>
Date: Thu, 17 Oct 2024 00:06:05 -0400
Subject: [PATCH 4/5] memcpy size must be >=8 to generate load atomic <1 x ptr>

---
 llvm/test/Transforms/SROA/atomic-vector.ll | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/llvm/test/Transforms/SROA/atomic-vector.ll b/llvm/test/Transforms/SROA/atomic-vector.ll
index 1b6cc275681a80..6258475aaff841 100644
--- a/llvm/test/Transforms/SROA/atomic-vector.ll
+++ b/llvm/test/Transforms/SROA/atomic-vector.ll
@@ -37,16 +37,15 @@ define i32 @atomic_vector_int() {
 
 define ptr @atomic_vector_ptr() {
 ; CHECK-LABEL: define ptr @atomic_vector_ptr() {
-; CHECK-NEXT:    [[SRC_SROA_0:%.*]] = alloca [4 x i8], align 8
-; CHECK-NEXT:    [[VAL_SROA_0:%.*]] = alloca ptr, align 8
-; CHECK-NEXT:    call void @llvm.memcpy.p0.p0.i64(ptr align 8 [[VAL_SROA_0]], ptr align 8 [[SRC_SROA_0]], i64 4, i1 false)
+; CHECK-NEXT:    [[VAL_SROA_0:%.*]] = alloca <1 x ptr>, align 8
+; CHECK-NEXT:    store <1 x ptr> undef, ptr [[VAL_SROA_0]], align 8
 ; CHECK-NEXT:    [[VAL_SROA_0_0_VAL_SROA_0_0_RET:%.*]] = load atomic volatile ptr, ptr [[VAL_SROA_0]] acquire, align 4
 ; CHECK-NEXT:    ret ptr [[VAL_SROA_0_0_VAL_SROA_0_0_RET]]
 ;
   %src = alloca <1 x ptr>
   %val = alloca <1 x ptr>
   %direct = alloca ptr
-  call void @llvm.memcpy.p0.p0.i64(ptr %val, ptr %src, i64 4, i1 false)
+  call void @llvm.memcpy.p0.p0.i64(ptr %val, ptr %src, i64 8, i1 false)
   store ptr %val, ptr %direct
   %indirect = load ptr, ptr %direct
   %ret = load atomic volatile ptr, ptr %indirect acquire, align 4

>From ef4fe92fb0b07a93c1db1bf5ff1b481080ad56b2 Mon Sep 17 00:00:00 2001
From: jofernau <Joe.Fernau at amd.com>
Date: Thu, 17 Oct 2024 23:25:57 -0400
Subject: [PATCH 5/5] Use isValidAtomicTy

---
 llvm/include/llvm/IR/Instructions.h | 4 ++++
 llvm/lib/IR/Instructions.cpp        | 7 +++++++
 llvm/lib/Transforms/Scalar/SROA.cpp | 4 +++-
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index 88c8c709c306d9..9cd3212680e869 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -250,6 +250,10 @@ class LoadInst : public UnaryInstruction {
            !isVolatile();
   }
 
+  /// Returns false if this type would be invalid in the
+  /// creation of a load atomic instruction.
+  static bool isValidAtomicTy(Type *Ty, const DataLayout &DL);
+
   Value *getPointerOperand() { return getOperand(0); }
   const Value *getPointerOperand() const { return getOperand(0); }
   static unsigned getPointerOperandIndex() { return 0U; }
diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index 009e0c03957c97..d3a5238cd36864 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -1247,6 +1247,13 @@ void LoadInst::AssertOK() {
          "Ptr must have pointer type.");
 }
 
+bool LoadInst::isValidAtomicTy(Type *Ty, const DataLayout &DL) {
+  if (!Ty->isIntOrPtrTy() && !Ty->isFloatingPointTy())
+    return false;
+  unsigned Size = DL.getTypeSizeInBits(Ty);
+  return Size >= 8 && !(Size & (Size - 1));
+}
+
 static Align computeLoadStoreDefaultAlign(Type *Ty, InsertPosition Pos) {
   assert(Pos.isValid() &&
          "Insertion position cannot be null when alignment not provided!");
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index 450ecdf20ef009..5e0eda71d5fefe 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -2855,7 +2855,8 @@ class AllocaSliceRewriter : public InstVisitor<AllocaSliceRewriter, bool> {
     LLVM_DEBUG(dbgs() << "    original: " << LI << "\n");
 
     // load atomic vector would be generated, which is illegal
-    if (LI.isAtomic() && NewAI.getAllocatedType()->isVectorTy())
+    if (LI.isAtomic() &&
+        !LoadInst::isValidAtomicTy(NewAI.getAllocatedType(), DL))
       return false;
 
     Value *OldOp = LI.getOperand(0);
@@ -2880,6 +2881,7 @@ class AllocaSliceRewriter : public InstVisitor<AllocaSliceRewriter, bool> {
                (canConvertValue(DL, NewAllocaTy, TargetTy) ||
                 (IsLoadPastEnd && NewAllocaTy->isIntegerTy() &&
                  TargetTy->isIntegerTy() && !LI.isVolatile()))) {
+
       Value *NewPtr =
           getPtrToNewAI(LI.getPointerAddressSpace(), LI.isVolatile());
       LoadInst *NewLI = IRB.CreateAlignedLoad(NewAI.getAllocatedType(), NewPtr,



More information about the llvm-commits mailing list