[llvm] f445e39 - [SimplifyCFG] Use isWritableObject() API (#110127)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 30 01:03:52 PDT 2024
Author: Nikita Popov
Date: 2024-09-30T10:03:46+02:00
New Revision: f445e39ab271d07733f0f45048badd9e58905aec
URL: https://github.com/llvm/llvm-project/commit/f445e39ab271d07733f0f45048badd9e58905aec
DIFF: https://github.com/llvm/llvm-project/commit/f445e39ab271d07733f0f45048badd9e58905aec.diff
LOG: [SimplifyCFG] Use isWritableObject() API (#110127)
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.
Added:
Modified:
llvm/lib/Analysis/AliasAnalysis.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/Transforms/SimplifyCFG/speculate-store.ll
Removed:
################################################################################
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..1ee6c17bdad397 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<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, /*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 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]]
;
More information about the llvm-commits
mailing list