[llvm] 52b86d3 - [MemorySSA] Use provided memory location even if instruction is call
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 4 11:30:37 PST 2020
Author: Nikita Popov
Date: 2020-11-04T20:30:22+01:00
New Revision: 52b86d35a401eaaeaffbd5ed99b0cd3f4250254d
URL: https://github.com/llvm/llvm-project/commit/52b86d35a401eaaeaffbd5ed99b0cd3f4250254d
DIFF: https://github.com/llvm/llvm-project/commit/52b86d35a401eaaeaffbd5ed99b0cd3f4250254d.diff
LOG: [MemorySSA] Use provided memory location even if instruction is call
If getClobberingMemoryAccess() is called with an explicit
MemoryLocation, but the starting access happens to be a call, the
provided location is currently ignored, and alias analysis queries
will be performed against the call instruction instead. Something
similar happens if the starting access is a load with a MemoryDef.
Change the implementation to not set Q.Inst in the first place if
we want to perform a MemoryLocation-based query, to make sure it
can't be turned into an Instruction-based query along the way...
Additionally, remove the special handling that lifetime.start
intrinsics currently get. They simply report NoAlias for clobbers
between lifetime.start and other calls, but that's obviously not
right if the other call is something like a memset or memcpy. The
default behavior we get from getModRefInfo() will already do the
right thing here.
Differential Revision: https://reviews.llvm.org/D88782
Added:
Modified:
llvm/lib/Analysis/MemorySSA.cpp
llvm/unittests/Analysis/MemorySSATest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/MemorySSA.cpp b/llvm/lib/Analysis/MemorySSA.cpp
index 016a37d284a9..32e13267f3e8 100644
--- a/llvm/lib/Analysis/MemorySSA.cpp
+++ b/llvm/lib/Analysis/MemorySSA.cpp
@@ -264,7 +264,6 @@ instructionClobbersQuery(const MemoryDef *MD, const MemoryLocation &UseLoc,
const Instruction *UseInst, AliasAnalysisType &AA) {
Instruction *DefInst = MD->getMemoryInst();
assert(DefInst && "Defining instruction not actually an instruction");
- const auto *UseCall = dyn_cast<CallBase>(UseInst);
Optional<AliasResult> AR;
if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(DefInst)) {
@@ -276,11 +275,6 @@ instructionClobbersQuery(const MemoryDef *MD, const MemoryLocation &UseLoc,
// clobbers where they don't really exist at all. Please see D43269 for
// context.
switch (II->getIntrinsicID()) {
- case Intrinsic::lifetime_start:
- if (UseCall)
- return {false, NoAlias};
- AR = AA.alias(MemoryLocation(II->getArgOperand(1)), UseLoc);
- return {AR != NoAlias, AR};
case Intrinsic::lifetime_end:
case Intrinsic::invariant_start:
case Intrinsic::invariant_end:
@@ -296,14 +290,14 @@ instructionClobbersQuery(const MemoryDef *MD, const MemoryLocation &UseLoc,
}
}
- if (UseCall) {
- ModRefInfo I = AA.getModRefInfo(DefInst, UseCall);
+ if (auto *CB = dyn_cast_or_null<CallBase>(UseInst)) {
+ ModRefInfo I = AA.getModRefInfo(DefInst, CB);
AR = isMustSet(I) ? MustAlias : MayAlias;
return {isModOrRefSet(I), AR};
}
if (auto *DefLoad = dyn_cast<LoadInst>(DefInst))
- if (auto *UseLoad = dyn_cast<LoadInst>(UseInst))
+ if (auto *UseLoad = dyn_cast_or_null<LoadInst>(UseInst))
return {!areLoadsReorderable(UseLoad, DefLoad), MayAlias};
ModRefInfo I = AA.getModRefInfo(DefInst, UseLoc);
@@ -2361,7 +2355,7 @@ MemorySSA::ClobberWalkerBase<AliasAnalysisType>::getClobberingMemoryAccessBase(
UpwardsMemoryQuery Q;
Q.OriginalAccess = StartingUseOrDef;
Q.StartingLoc = Loc;
- Q.Inst = I;
+ Q.Inst = nullptr;
Q.IsCall = false;
// Unlike the other function, do not walk to the def of a def, because we are
diff --git a/llvm/unittests/Analysis/MemorySSATest.cpp b/llvm/unittests/Analysis/MemorySSATest.cpp
index 5c0c48b78831..2e56d7c6917a 100644
--- a/llvm/unittests/Analysis/MemorySSATest.cpp
+++ b/llvm/unittests/Analysis/MemorySSATest.cpp
@@ -1203,12 +1203,14 @@ TEST_F(MemorySSATest, LifetimeMarkersAreClobbers) {
// Example code:
// define void @a(i8* %foo) {
// %bar = getelementptr i8, i8* %foo, i64 1
+ // %baz = getelementptr i8, i8* %foo, i64 2
// store i8 0, i8* %foo
// store i8 0, i8* %bar
- // call void @llvm.lifetime.end.p0i8(i64 8, i32* %p)
- // call void @llvm.lifetime.start.p0i8(i64 8, i32* %p)
+ // call void @llvm.lifetime.end.p0i8(i64 3, i8* %foo)
+ // call void @llvm.lifetime.start.p0i8(i64 3, i8* %foo)
// store i8 0, i8* %foo
// store i8 0, i8* %bar
+ // call void @llvm.memset.p0i8(i8* %baz, i8 0, i64 1)
// ret void
// }
//
@@ -1228,6 +1230,7 @@ TEST_F(MemorySSATest, LifetimeMarkersAreClobbers) {
Value *Foo = &*F->arg_begin();
Value *Bar = B.CreateGEP(B.getInt8Ty(), Foo, B.getInt64(1), "bar");
+ Value *Baz = B.CreateGEP(B.getInt8Ty(), Foo, B.getInt64(2), "baz");
B.CreateStore(B.getInt8(0), Foo);
B.CreateStore(B.getInt8(0), Bar);
@@ -1237,12 +1240,13 @@ TEST_F(MemorySSATest, LifetimeMarkersAreClobbers) {
};
B.CreateCall(GetLifetimeIntrinsic(Intrinsic::lifetime_end),
- {B.getInt64(2), Foo});
+ {B.getInt64(3), Foo});
Instruction *LifetimeStart = B.CreateCall(
- GetLifetimeIntrinsic(Intrinsic::lifetime_start), {B.getInt64(2), Foo});
+ GetLifetimeIntrinsic(Intrinsic::lifetime_start), {B.getInt64(3), Foo});
Instruction *FooStore = B.CreateStore(B.getInt8(0), Foo);
Instruction *BarStore = B.CreateStore(B.getInt8(0), Bar);
+ Instruction *BazMemSet = B.CreateMemSet(Baz, B.getInt8(0), 1, Align(1));
setupAnalyses();
MemorySSA &MSSA = *Analyses->MSSA;
@@ -1256,6 +1260,9 @@ TEST_F(MemorySSATest, LifetimeMarkersAreClobbers) {
MemoryAccess *BarAccess = MSSA.getMemoryAccess(BarStore);
ASSERT_NE(BarAccess, nullptr);
+ MemoryAccess *BazAccess = MSSA.getMemoryAccess(BazMemSet);
+ ASSERT_NE(BazAccess, nullptr);
+
MemoryAccess *FooClobber =
MSSA.getWalker()->getClobberingMemoryAccess(FooAccess);
EXPECT_EQ(FooClobber, LifetimeStartAccess);
@@ -1263,6 +1270,15 @@ TEST_F(MemorySSATest, LifetimeMarkersAreClobbers) {
MemoryAccess *BarClobber =
MSSA.getWalker()->getClobberingMemoryAccess(BarAccess);
EXPECT_EQ(BarClobber, LifetimeStartAccess);
+
+ MemoryAccess *BazClobber =
+ MSSA.getWalker()->getClobberingMemoryAccess(BazAccess);
+ EXPECT_EQ(BazClobber, LifetimeStartAccess);
+
+ MemoryAccess *LifetimeStartClobber =
+ MSSA.getWalker()->getClobberingMemoryAccess(
+ LifetimeStartAccess, MemoryLocation(Foo));
+ EXPECT_EQ(LifetimeStartClobber, LifetimeStartAccess);
}
TEST_F(MemorySSATest, DefOptimizationsAreInvalidatedOnMoving) {
@@ -1583,3 +1599,71 @@ TEST_F(MemorySSATest, TestAddedEdgeToBlockWithNoPhiAddNewPhis) {
MemoryPhi *MPE = MSSA.getMemoryAccess(EBlock);
EXPECT_EQ(MPD, MPE->getIncomingValueForBlock(DBlock));
}
+
+TEST_F(MemorySSATest, TestCallClobber) {
+ F = Function::Create(
+ FunctionType::get(B.getVoidTy(), {B.getInt8PtrTy()}, false),
+ GlobalValue::ExternalLinkage, "F", &M);
+
+ Value *Pointer1 = &*F->arg_begin();
+ BasicBlock *Entry(BasicBlock::Create(C, "", F));
+ B.SetInsertPoint(Entry);
+ Value *Pointer2 = B.CreateGEP(B.getInt8Ty(), Pointer1, B.getInt64(1));
+ Instruction *StorePointer1 = B.CreateStore(B.getInt8(0), Pointer1);
+ Instruction *StorePointer2 = B.CreateStore(B.getInt8(0), Pointer2);
+ Instruction *MemSet = B.CreateMemSet(Pointer2, B.getInt8(0), 1, Align(1));
+
+ setupAnalyses();
+ MemorySSA &MSSA = *Analyses->MSSA;
+ MemorySSAWalker *Walker = Analyses->Walker;
+
+ MemoryUseOrDef *Store1Access = MSSA.getMemoryAccess(StorePointer1);
+ MemoryUseOrDef *Store2Access = MSSA.getMemoryAccess(StorePointer2);
+ MemoryUseOrDef *MemSetAccess = MSSA.getMemoryAccess(MemSet);
+
+ MemoryAccess *Pointer1Clobber = Walker->getClobberingMemoryAccess(
+ MemSetAccess, MemoryLocation(Pointer1, LocationSize::precise(1)));
+ EXPECT_EQ(Pointer1Clobber, Store1Access);
+
+ MemoryAccess *Pointer2Clobber = Walker->getClobberingMemoryAccess(
+ MemSetAccess, MemoryLocation(Pointer2, LocationSize::precise(1)));
+ EXPECT_EQ(Pointer2Clobber, MemSetAccess);
+
+ MemoryAccess *MemSetClobber = Walker->getClobberingMemoryAccess(MemSetAccess);
+ EXPECT_EQ(MemSetClobber, Store2Access);
+}
+
+TEST_F(MemorySSATest, TestLoadClobber) {
+ F = Function::Create(
+ FunctionType::get(B.getVoidTy(), {B.getInt8PtrTy()}, false),
+ GlobalValue::ExternalLinkage, "F", &M);
+
+ Value *Pointer1 = &*F->arg_begin();
+ BasicBlock *Entry(BasicBlock::Create(C, "", F));
+ B.SetInsertPoint(Entry);
+ Value *Pointer2 = B.CreateGEP(B.getInt8Ty(), Pointer1, B.getInt64(1));
+ Instruction *LoadPointer1 =
+ B.CreateLoad(B.getInt8Ty(), Pointer1, /* Volatile */ true);
+ Instruction *LoadPointer2 =
+ B.CreateLoad(B.getInt8Ty(), Pointer2, /* Volatile */ true);
+
+ setupAnalyses();
+ MemorySSA &MSSA = *Analyses->MSSA;
+ MemorySSAWalker *Walker = Analyses->Walker;
+
+ MemoryUseOrDef *Load1Access = MSSA.getMemoryAccess(LoadPointer1);
+ MemoryUseOrDef *Load2Access = MSSA.getMemoryAccess(LoadPointer2);
+
+ // When providing a memory location, we should never return a load as the
+ // clobber.
+ MemoryAccess *Pointer1Clobber = Walker->getClobberingMemoryAccess(
+ Load2Access, MemoryLocation(Pointer1, LocationSize::precise(1)));
+ EXPECT_TRUE(MSSA.isLiveOnEntryDef(Pointer1Clobber));
+
+ MemoryAccess *Pointer2Clobber = Walker->getClobberingMemoryAccess(
+ Load2Access, MemoryLocation(Pointer2, LocationSize::precise(1)));
+ EXPECT_TRUE(MSSA.isLiveOnEntryDef(Pointer2Clobber));
+
+ MemoryAccess *Load2Clobber = Walker->getClobberingMemoryAccess(Load2Access);
+ EXPECT_EQ(Load2Clobber, Load1Access);
+}
More information about the llvm-commits
mailing list