[PATCH] D75266: SROA: Don't drop atomic load/store alignments (PR45010)

[Hans Wennborg via Phabricator via llvm-commits]

hans created this revision.
hans added reviewers: bkramer, chandlerc, gchatelet, rnk.
Herald added subscribers: jfb, hiraditya.
Herald added a project: LLVM.

SROA will drop the explicit alignment on allocas when the ABI guarantees enough alignment. Because the alignment on new load/store instructions are set base on the alloca's alignment,  that means SROA would end up dropping the alignment from atomic loads and stores, which is not allowed (see bug). For those, make sure to always carry over the alignment from the previous instruction.


https://reviews.llvm.org/D75266

Files:
  llvm/lib/Transforms/Scalar/SROA.cpp
  llvm/test/Transforms/SROA/alignment.ll


Index: llvm/test/Transforms/SROA/alignment.ll
===================================================================
--- llvm/test/Transforms/SROA/alignment.ll
+++ llvm/test/Transforms/SROA/alignment.ll
@@ -228,4 +228,19 @@
   ret void
 }
 
+%struct = type { i32, i32 }
+define dso_local i32 @pr45010(%struct* %A) {
+; CHECK-LABEL: @pr45010
+; CHECK: load atomic volatile i32, {{.*}}, align 4
+
+  %B = alloca %struct, align 4
+  %A.i = getelementptr inbounds %struct, %struct* %A, i32 0, i32 0
+  %B.i = getelementptr inbounds %struct, %struct* %B, i32 0, i32 0
+  %1 = load i32, i32* %A.i, align 4
+  store i32 %1, i32* %B.i, align 4
+  %2 = bitcast %struct* %B to i32*
+  %x = load atomic volatile i32, i32* %2 acquire, align 4
+  ret i32 %x
+}
+
 declare void @populate(i8*)
Index: llvm/lib/Transforms/Scalar/SROA.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/SROA.cpp
+++ llvm/lib/Transforms/Scalar/SROA.cpp
@@ -2519,6 +2519,8 @@
         NewLI->setAAMetadata(AATags);
       if (LI.isVolatile())
         NewLI->setAtomic(LI.getOrdering(), LI.getSyncScopeID());
+      if (NewLI->isAtomic())
+        NewLI->setAlignment(MaybeAlign(LI.getAlignment()));
 
       // Any !nonnull metadata or !range metadata on the old load is also valid
       // on the new load. This is even true in some cases even when the loads
@@ -2709,6 +2711,8 @@
       NewSI->setAAMetadata(AATags);
     if (SI.isVolatile())
       NewSI->setAtomic(SI.getOrdering(), SI.getSyncScopeID());
+    if (NewSI->isAtomic())
+      NewSI->setAlignment(MaybeAlign(SI.getAlignment()));
     Pass.DeadInsts.insert(&SI);
     deleteIfTriviallyDead(OldOp);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75266.246962.patch
Type: text/x-patch
Size: 1681 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200227/69a0b0f0/attachment.bin>