[llvm] 30ac5f9 - [InstCombine] Do not combine atomic and non-atomic loads
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 30 07:07:50 PST 2022
Author: Ricky Zhou
Date: 2022-01-30T10:05:11-05:00
New Revision: 30ac5f9e64366a4190ccb205cc774732bc1131c8
URL: https://github.com/llvm/llvm-project/commit/30ac5f9e64366a4190ccb205cc774732bc1131c8
DIFF: https://github.com/llvm/llvm-project/commit/30ac5f9e64366a4190ccb205cc774732bc1131c8.diff
LOG: [InstCombine] Do not combine atomic and non-atomic loads
Before this change, InstCombine was willing to fold atomic and
non-atomic loads through a PHI node as long as the first PHI argument
is not an atomic load. The combined load would be non-atomic, which is
incorrect.
Fix this by only combining the loads in a PHI node when all of the
arguments are non-atomic loads.
Thanks to Eli Friedman for pointing out the bug at
https://github.com/llvm/llvm-project/issues/50777#issuecomment-981045342!
Fixes #50777
Differential Revision: https://reviews.llvm.org/D115113
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
llvm/test/Transforms/InstCombine/phi.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index e23558e07efd..09694d50468f 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -666,7 +666,7 @@ Instruction *InstCombinerImpl::foldPHIArgLoadIntoPHI(PHINode &PN) {
// sunk load: whether it is volatile, and what its alignment is.
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.
@@ -681,19 +681,21 @@ Instruction *InstCombinerImpl::foldPHIArgLoadIntoPHI(PHINode &PN) {
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, LI->getAlign());
diff --git a/llvm/test/Transforms/InstCombine/phi.ll b/llvm/test/Transforms/InstCombine/phi.ll
index 51d5baf0ef53..2a89b4349055 100644
--- a/llvm/test/Transforms/InstCombine/phi.ll
+++ b/llvm/test/Transforms/InstCombine/phi.ll
@@ -547,17 +547,15 @@ bb2: ; preds = %bb1, %entry
ret i32 %res
}
-; FIXME: Atomic and non-atomic loads should not be combined.
+; 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: br i1 %c, label %if, label %end
+; 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-NEXT: br label %end
; CHECK: end:
-; CHECK-NEXT: [[COND_IN:%.*]] = phi i32* [ %ptr, %entry ], [ %atomic_ptr, %if ]
-; CHECK-NEXT: [[COND:%.*]] = load i32, i32* [[COND_IN]], align 4
+; CHECK-NEXT: [[COND:%.*]] = phi i32 [ [[NON_ATOMIC]], %entry ], [ [[ATOMIC]], %if ]
; CHECK-NEXT: ret i32 [[COND]]
entry:
%x = load i32, i32* %ptr, align 4
More information about the llvm-commits
mailing list