[llvm] [SimplifyCFG] Use isWritableObject() API (PR #110127)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 26 07:42:15 PDT 2024
https://github.com/nikic created https://github.com/llvm/llvm-project/pull/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.
>From 6815057f951b1694f6e311c3f0a50f4056d612fd Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Thu, 26 Sep 2024 16:18:10 +0200
Subject: [PATCH] [SimplifyCFG] Use isWritableObject() API
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.
---
llvm/lib/Analysis/AliasAnalysis.cpp | 5 ++++-
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 17 ++++++++-------
.../Transforms/SimplifyCFG/speculate-store.ll | 21 ++++++-------------
3 files changed, 19 insertions(+), 24 deletions(-)
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]]
;
More information about the llvm-commits
mailing list