Author: hans
Date: Mon Aug 13 01:28:30 2018
New Revision: 339545
URL: http://llvm.org/viewvc/llvm-project?rev=339545&view=rev
Log:
Merging r339411:
------------------------------------------------------------------------
r339411 | gbiv | 2018-08-10 07:14:43 +0200 (Fri, 10 Aug 2018) | 17 lines
[MemorySSA] "Fix" lifetime intrinsic handling
MemorySSA currently creates MemoryAccesses for lifetime intrinsics, and
sometimes treats them as clobbers. This may/may not be the best way
forward, but while we're doing it, we should consider
MayAlias/PartialAlias to be clobbers.
The ideal fix here is probably to remove all of this reasoning about
lifetimes from MemorySSA + put it into the passes that need to care. But
that's a wayyy broader fix that needs some consensus, and we have
miscompiles + a release branch today, and this should solve the
miscompiles just as well.
differential revision is D43269. Landing without an explicit LGTM (and
without using the special please-autoclose-this syntax) so we can still
use that revision as a place to decide what the right fix here is.
------------------------------------------------------------------------
Modified:
llvm/branches/release_70/ (props changed)
llvm/branches/release_70/lib/Analysis/MemorySSA.cpp
llvm/branches/release_70/test/Transforms/EarlyCSE/memoryssa.ll
llvm/branches/release_70/unittests/Analysis/MemorySSA.cpp
Propchange: llvm/branches/release_70/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Aug 13 01:28:30 2018
@@ -1,3 +1,3 @@
/llvm/branches/Apple/Pertwee:110850,110961
/llvm/branches/type-system-rewrite:133420-134817
-/llvm/trunk:155241,338552,338554,338569,338599,338610,338658,338665,338682,338703,338709,338716,338751,338762,338817,338902,338915,338968,339073,339179,339184,339190,339225,339316,339319,339492
+/llvm/trunk:155241,338552,338554,338569,338599,338610,338658,338665,338682,338703,338709,338716,338751,338762,338817,338902,338915,338968,339073,339179,339184,339190,339225,339316,339319,339411,339492
Modified: llvm/branches/release_70/lib/Analysis/MemorySSA.cpp
URL:
http://llvm.org/viewvc/llvm-project/llvm/branches/release_70/lib/Analysis/MemorySSA.cpp?rev=339545&r1=339544&r2=339545&view=diff
==============================================================================
--- llvm/branches/release_70/lib/Analysis/MemorySSA.cpp (original)
+++ llvm/branches/release_70/lib/Analysis/MemorySSA.cpp Mon Aug 13 01:28:30 2018
@@ -258,13 +258,18 @@ static ClobberAlias instructionClobbersQ
if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(DefInst)) {
// These intrinsics will show up as affecting memory, but they are just
- // markers.
+ // markers, mostly.
+ //
+ // FIXME: We probably don't actually want MemorySSA to model these at all
+ // (including creating MemoryAccesses for them): we just end up inventing
+ // clobbers where they don't really exist at all. Please see D43269 for
+ // context.
switch (II->getIntrinsicID()) {
case Intrinsic::lifetime_start:
if (UseCS)
return {false, NoAlias};
AR = AA.alias(MemoryLocation(II->getArgOperand(1)), UseLoc);
- return {AR == MustAlias, AR};
+ return {AR != NoAlias, AR};
case Intrinsic::lifetime_end:
case Intrinsic::invariant_start:
case Intrinsic::invariant_end:
Modified: llvm/branches/release_70/test/Transforms/EarlyCSE/memoryssa.ll
URL:
http://llvm.org/viewvc/llvm-project/llvm/branches/release_70/test/Transforms/EarlyCSE/memoryssa.ll?rev=339545&r1=339544&r2=339545&view=diff
==============================================================================
--- llvm/branches/release_70/test/Transforms/EarlyCSE/memoryssa.ll (original)
+++ llvm/branches/release_70/test/Transforms/EarlyCSE/memoryssa.ll Mon Aug 13
01:28:30 2018
@@ -106,3 +106,45 @@ end:
store i32 %v2, i32* @G3
ret void
}
+
+;; Check that we respect lifetime.start/lifetime.end intrinsics when deleting
+;; stores that, without the lifetime calls, would be writebacks.
+; CHECK-LABEL: @test_writeback_lifetimes(
+; CHECK-NOMEMSSA-LABEL: @test_writeback_lifetimes(
+define void @test_writeback_lifetimes(i32* %p) {
+entry:
+ %q = getelementptr i32, i32* %p, i64 1
+ %pv = load i32, i32* %p
+ %qv = load i32, i32* %q
+ call void @llvm.lifetime.end.p0i8(i64 8, i32* %p)
+ call void @llvm.lifetime.start.p0i8(i64 8, i32* %p)
+ ; CHECK: store i32 %pv
+ ; CHECK-NOMEMSSA-LABEL: store i32 %pv
+ store i32 %pv, i32* %p
+ ; CHECK: store i32 %qv, i32* %q
+ ; CHECK-NOMEMSSA-LABEL: store i32 %qv, i32* %q
+ store i32 %qv, i32* %q
+ ret void
+}
+
+;; Check that we respect lifetime.start/lifetime.end intrinsics when deleting
+;; stores that, without the lifetime calls, would be writebacks.
+; CHECK-LABEL: @test_writeback_lifetimes_multi_arg(
+; CHECK-NOMEMSSA-LABEL: @test_writeback_lifetimes_multi_arg(
+define void @test_writeback_lifetimes_multi_arg(i32* %p, i32* %q) {
+entry:
+ %pv = load i32, i32* %p
+ %qv = load i32, i32* %q
+ call void @llvm.lifetime.end.p0i8(i64 8, i32* %p)
+ call void @llvm.lifetime.start.p0i8(i64 8, i32* %p)
+ ; CHECK: store i32 %pv
+ ; CHECK-NOMEMSSA-LABEL: store i32 %pv
+ store i32 %pv, i32* %p
+ ; CHECK: store i32 %qv, i32* %q
+ ; CHECK-NOMEMSSA-LABEL: store i32 %qv, i32* %q
+ store i32 %qv, i32* %q
+ ret void
+}
+
+declare void @llvm.lifetime.end.p0i8(i64, i32*)
+declare void @llvm.lifetime.start.p0i8(i64, i32*)
Modified: llvm/branches/release_70/unittests/Analysis/MemorySSA.cpp
URL:
http://llvm.org/viewvc/llvm-project/llvm/branches/release_70/unittests/Analysis/MemorySSA.cpp?rev=339545&r1=339544&r2=339545&view=diff
==============================================================================
--- llvm/branches/release_70/unittests/Analysis/MemorySSA.cpp (original)
+++ llvm/branches/release_70/unittests/Analysis/MemorySSA.cpp Mon Aug 13
01:28:30 2018
@@ -1199,3 +1199,69 @@ TEST_F(MemorySSATest, TestStoreMayAlias)
++I;
}
}
+
+TEST_F(MemorySSATest, LifetimeMarkersAreClobbers) {
+ // Example code:
+ // define void @a(i8* %foo) {
+ // %bar = getelementptr i8, i8* %foo, i64 1
+ // 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)
+ // store i8 0, i8* %foo
+ // store i8 0, i8* %bar
+ // ret void
+ // }
+ //
+ // Patterns like this are possible after inlining; the stores to %foo and
%bar
+ // should both be clobbered by the lifetime.start call if they're dominated
by
+ // it.
+
+ IRBuilder<> B(C);
+ F = Function::Create(
+ FunctionType::get(B.getVoidTy(), {B.getInt8PtrTy()}, false),
+ GlobalValue::ExternalLinkage, "F", &M);
+
+ // Make blocks
+ BasicBlock *Entry = BasicBlock::Create(C, "entry", F);
+
+ B.SetInsertPoint(Entry);
+ Value *Foo = &*F->arg_begin();
+
+ Value *Bar = B.CreateGEP(Foo, B.getInt64(1), "bar");
+
+ B.CreateStore(B.getInt8(0), Foo);
+ B.CreateStore(B.getInt8(0), Bar);
+
+ auto GetLifetimeIntrinsic = [&](Intrinsic::ID ID) {
+ return Intrinsic::getDeclaration(&M, ID, {Foo->getType()});
+ };
+
+ B.CreateCall(GetLifetimeIntrinsic(Intrinsic::lifetime_end),
+ {B.getInt64(2), Foo});
+ Instruction *LifetimeStart = B.CreateCall(
+ GetLifetimeIntrinsic(Intrinsic::lifetime_start), {B.getInt64(2), Foo});
+
+ Instruction *FooStore = B.CreateStore(B.getInt8(0), Foo);
+ Instruction *BarStore = B.CreateStore(B.getInt8(0), Bar);
+
+ setupAnalyses();
+ MemorySSA &MSSA = *Analyses->MSSA;
+
+ MemoryAccess *LifetimeStartAccess = MSSA.getMemoryAccess(LifetimeStart);
+ ASSERT_NE(LifetimeStartAccess, nullptr);
+
+ MemoryAccess *FooAccess = MSSA.getMemoryAccess(FooStore);
+ ASSERT_NE(FooAccess, nullptr);
+
+ MemoryAccess *BarAccess = MSSA.getMemoryAccess(BarStore);
+ ASSERT_NE(BarAccess, nullptr);
+
+ MemoryAccess *FooClobber =
+ MSSA.getWalker()->getClobberingMemoryAccess(FooAccess);
+ EXPECT_EQ(FooClobber, LifetimeStartAccess);
+
+ MemoryAccess *BarClobber =
+ MSSA.getWalker()->getClobberingMemoryAccess(BarAccess);
+ EXPECT_EQ(BarClobber, LifetimeStartAccess);
+}
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
