[PATCH] D115113: [InstCombine] Do not combine atomic and non-atomic loads.
Ricky Zhou via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 21 01:03:18 PST 2021
rickyz updated this revision to Diff 395617.
rickyz marked an inline comment as done.
rickyz added a comment.
Make it clearer that we give up immediately on encountering an atomic load.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115113/new/
https://reviews.llvm.org/D115113
Files:
llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
llvm/test/Transforms/InstCombine/phi.ll
Index: llvm/test/Transforms/InstCombine/phi.ll
===================================================================
--- llvm/test/Transforms/InstCombine/phi.ll
+++ llvm/test/Transforms/InstCombine/phi.ll
@@ -547,6 +547,29 @@
ret i32 %res
}
+; Atomic and non-atomic loads should not be combined.
+define i32 @PR51435(i32* %ptr, i32* %atomic_ptr, i1 %c) {
+; CHECK-LABEL: @PR51435(
+; CHECK: entry:
+; CHECK-NEXT: [[NON_ATOMIC:%.*]] = load i32, i32* %ptr, align 4
+; CHECK: if:
+; CHECK-NEXT: [[ATOMIC:%.*]] = load atomic i32, i32* %atomic_ptr acquire, align 4
+; CHECK: end:
+; CHECK-NEXT: [[COND:%.*]] = phi i32 [ [[NON_ATOMIC]], %entry ], [ [[ATOMIC]], %if ]
+; CHECK-NEXT: ret i32 [[COND]]
+entry:
+ %x = load i32, i32* %ptr, align 4
+ br i1 %c, label %if, label %end
+
+if:
+ %y = load atomic i32, i32* %atomic_ptr acquire, align 4
+ br label %end
+
+end:
+ %cond = phi i32 [ %x, %entry ], [ %y, %if ]
+ ret i32 %cond
+}
+
define i1 @test18(i1 %cond) {
; CHECK-LABEL: @test18(
; CHECK-NEXT: br i1 [[COND:%.*]], label [[TRUE:%.*]], label [[FALSE:%.*]]
Index: llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -667,9 +667,9 @@
// don't sink loads when some have their alignment specified and some don't.
// visitLoadInst will propagate an alignment onto the load when TD is around,
// and if TD isn't around, we can't handle the mixed case.
- bool IsVolatile = FirstLI->isVolatile();
+ const bool IsVolatile = FirstLI->isVolatile();
Align LoadAlignment = FirstLI->getAlign();
- unsigned LoadAddrSpace = FirstLI->getPointerAddressSpace();
+ const unsigned LoadAddrSpace = FirstLI->getPointerAddressSpace();
// We can't sink the load if the loaded value could be modified between the
// load and the PHI.
@@ -684,19 +684,21 @@
FirstLI->getParent()->getTerminator()->getNumSuccessors() != 1)
return nullptr;
- // Check to see if all arguments are the same operation.
for (auto Incoming : drop_begin(zip(PN.blocks(), PN.incoming_values()))) {
BasicBlock *InBB = std::get<0>(Incoming);
Value *InVal = std::get<1>(Incoming);
LoadInst *LI = dyn_cast<LoadInst>(InVal);
- if (!LI || !LI->hasOneUser())
+ if (!LI || !LI->hasOneUser() || LI->isAtomic())
+ return nullptr;
+
+ // Make sure all arguments are the same type of operation.
+ if (LI->isVolatile() != IsVolatile ||
+ LI->getPointerAddressSpace() != LoadAddrSpace)
return nullptr;
// We can't sink the load if the loaded value could be modified between
// the load and the PHI.
- if (LI->isVolatile() != IsVolatile || LI->getParent() != InBB ||
- LI->getPointerAddressSpace() != LoadAddrSpace ||
- !isSafeAndProfitableToSinkLoad(LI))
+ if (LI->getParent() != InBB || !isSafeAndProfitableToSinkLoad(LI))
return nullptr;
LoadAlignment = std::min(LoadAlignment, Align(LI->getAlign()));
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D115113.395617.patch
Type: text/x-patch
Size: 3095 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211221/e0516daa/attachment.bin>
More information about the llvm-commits
mailing list