diff --git a/llvm/lib/Analysis/AliasAnalysis.cpp b/llvm/lib/Analysis/AliasAnalysis.cpp index d90bb213f420..20cdbb632032 100644 --- a/llvm/lib/Analysis/AliasAnalysis.cpp +++ b/llvm/lib/Analysis/AliasAnalysis.cpp @@ -892,7 +892,10 @@ bool llvm::isWritableObject(const Value *Object, return true; if (auto *A = dyn_cast(Object)) { - if (A->hasAttribute(Attribute::Writable)) { + // Also require noalias, otherwise writability at function entry cannot be + // generalized to writability at other program points, even if the pointer + // does not escape. + if (A->hasAttribute(Attribute::Writable) && A->hasNoAliasAttr()) { ExplicitlyDereferenceableOnly = true; return true; } diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 1f2c9389c008..1ee6c17bdad3 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -28,6 +28,7 @@ #include "llvm/Analysis/DomTreeUpdater.h" #include "llvm/Analysis/GuardUtils.h" #include "llvm/Analysis/InstructionSimplify.h" +#include "llvm/Analysis/Loads.h" #include "llvm/Analysis/MemorySSA.h" #include "llvm/Analysis/MemorySSAUpdater.h" #include "llvm/Analysis/TargetTransformInfo.h" @@ -3057,16 +3058,17 @@ static Value *isSafeToSpeculateStore(Instruction *I, BasicBlock *BrBB, if (auto *LI = dyn_cast(&CurI)) { if (LI->getPointerOperand() == StorePtr && LI->getType() == StoreTy && LI->isSimple() && LI->getAlign() >= StoreToHoist->getAlign()) { - // Local objects (created by an `alloca` instruction) are always - // writable, so once we are past a read from a location it is valid to - // also write to that same location. - // If the address of the local object never escapes the function, that - // means it's never concurrently read or written, hence moving the store - // from under the condition will not introduce a data race. - auto *AI = dyn_cast(getUnderlyingObject(StorePtr)); - if (AI && !PointerMayBeCaptured(AI, false, true)) + Value *Obj = getUnderlyingObject(StorePtr); + bool ExplicitlyDereferenceableOnly; + if (isWritableObject(Obj, ExplicitlyDereferenceableOnly) && + !PointerMayBeCaptured(Obj, /*ReturnCaptures=*/false, + /*StoreCaptures=*/true) && + (!ExplicitlyDereferenceableOnly || + isDereferenceablePointer(StorePtr, StoreTy, + LI->getDataLayout()))) { // Found a previous load, return it. return LI; + } } // The load didn't work out, but we may still find a store. } diff --git a/llvm/test/Transforms/SimplifyCFG/speculate-store.ll b/llvm/test/Transforms/SimplifyCFG/speculate-store.ll index d6da9fd8ae20..5addd0e3ad8e 100644 --- a/llvm/test/Transforms/SimplifyCFG/speculate-store.ll +++ b/llvm/test/Transforms/SimplifyCFG/speculate-store.ll @@ -201,11 +201,8 @@ define i64 @load_before_store_noescape_byval(ptr byval([2 x i32]) %a, i64 %i, i3 ; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [2 x i32], ptr [[A]], i64 0, i64 [[I:%.*]] ; CHECK-NEXT: [[V:%.*]] = load i32, ptr [[ARRAYIDX]], align 4 ; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[V]], [[B:%.*]] -; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_END:%.*]] -; CHECK: if.then: -; CHECK-NEXT: store i32 [[B]], ptr [[ARRAYIDX]], align 4 -; CHECK-NEXT: br label [[IF_END]] -; CHECK: if.end: +; CHECK-NEXT: [[SPEC_STORE_SELECT:%.*]] = select i1 [[CMP]], i32 [[B]], i32 [[V]] +; CHECK-NEXT: store i32 [[SPEC_STORE_SELECT]], ptr [[ARRAYIDX]], align 4 ; CHECK-NEXT: [[V2:%.*]] = load i64, ptr [[A]], align 8 ; CHECK-NEXT: ret i64 [[V2]] ; @@ -235,11 +232,8 @@ define i64 @load_before_store_noescape_malloc(i64 %i, i32 %b) { ; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [2 x i32], ptr [[A]], i64 0, i64 [[I:%.*]] ; CHECK-NEXT: [[V:%.*]] = load i32, ptr [[ARRAYIDX]], align 4 ; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[V]], [[B:%.*]] -; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_END:%.*]] -; CHECK: if.then: -; CHECK-NEXT: store i32 [[B]], ptr [[ARRAYIDX]], align 4 -; CHECK-NEXT: br label [[IF_END]] -; CHECK: if.end: +; CHECK-NEXT: [[SPEC_STORE_SELECT:%.*]] = select i1 [[CMP]], i32 [[B]], i32 [[V]] +; CHECK-NEXT: store i32 [[SPEC_STORE_SELECT]], ptr [[ARRAYIDX]], align 4 ; CHECK-NEXT: [[V2:%.*]] = load i64, ptr [[A]], align 8 ; CHECK-NEXT: ret i64 [[V2]] ; @@ -267,11 +261,8 @@ define i64 @load_before_store_noescape_writable(ptr noalias writable dereference ; CHECK-NEXT: [[ARRAYIDX:%.*]] = getelementptr inbounds [2 x i32], ptr [[A]], i64 0, i64 1 ; CHECK-NEXT: [[V:%.*]] = load i32, ptr [[ARRAYIDX]], align 4 ; CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[V]], [[B:%.*]] -; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_END:%.*]] -; CHECK: if.then: -; CHECK-NEXT: store i32 [[B]], ptr [[ARRAYIDX]], align 4 -; CHECK-NEXT: br label [[IF_END]] -; CHECK: if.end: +; CHECK-NEXT: [[SPEC_STORE_SELECT:%.*]] = select i1 [[CMP]], i32 [[B]], i32 [[V]] +; CHECK-NEXT: store i32 [[SPEC_STORE_SELECT]], ptr [[ARRAYIDX]], align 4 ; CHECK-NEXT: [[V2:%.*]] = load i64, ptr [[A]], align 8 ; CHECK-NEXT: ret i64 [[V2]] ;