[llvm] [SimplifyCFG] Use isWritableObject() API (PR #110127)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 26 07:42:50 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-transforms
Author: Nikita Popov (nikic)
<details>
<summary>Changes</summary>
SimplifyCFG store speculation currently has some homegrown code to check for a writable object, handling the alloca special case only.
Switch it to use the generic isWritableObject() API, which means that we also support byval arguments, allocator return values, and writable arguments.
I've adjusted isWritableObject() to also check for the noalias attribute when handling writable. Otherwise, I don't think that we can generalize from at-entry writability. This was not relevant for previous uses of the function, because they'd already require noalias for other reasons anyway.
---
Full diff: https://github.com/llvm/llvm-project/pull/110127.diff
3 Files Affected:
- (modified) llvm/lib/Analysis/AliasAnalysis.cpp (+4-1)
- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+9-8)
- (modified) llvm/test/Transforms/SimplifyCFG/speculate-store.ll (+6-15)
``````````diff
diff --git a/llvm/lib/Analysis/AliasAnalysis.cpp b/llvm/lib/Analysis/AliasAnalysis.cpp
index d90bb213f4208b..20cdbb63203224 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<Argument>(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 1f2c9389c008bd..937afe2e4b8e9e 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,16 @@ static Value *isSafeToSpeculateStore(Instruction *I, BasicBlock *BrBB,
if (auto *LI = dyn_cast<LoadInst>(&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<AllocaInst>(getUnderlyingObject(StorePtr));
- if (AI && !PointerMayBeCaptured(AI, false, true))
+ Value *Obj = getUnderlyingObject(StorePtr);
+ bool ExplicitlyDereferenceableOnly;
+ if (isWritableObject(Obj, ExplicitlyDereferenceableOnly) &&
+ !PointerMayBeCaptured(Obj, false, 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 d6da9fd8ae20cb..5addd0e3ad8ee1 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]]
;
``````````
</details>
https://github.com/llvm/llvm-project/pull/110127
More information about the llvm-commits
mailing list