llvm.org GIT mirror llvm / 3e5777f
[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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@339411 91177308-0d34-0410-b5e6-96231b3b80d8 George Burgess IV 1 year, 1 month ago
3 changed file(s) with 115 addition(s) and 2 deletion(s). Raw diff Collapse all Expand all
257257
258258 if (const IntrinsicInst *II = dyn_cast(DefInst)) {
259259 // These intrinsics will show up as affecting memory, but they are just
260 // markers.
260 // markers, mostly.
261 //
262 // FIXME: We probably don't actually want MemorySSA to model these at all
263 // (including creating MemoryAccesses for them): we just end up inventing
264 // clobbers where they don't really exist at all. Please see D43269 for
265 // context.
261266 switch (II->getIntrinsicID()) {
262267 case Intrinsic::lifetime_start:
263268 if (UseCS)
264269 return {false, NoAlias};
265270 AR = AA.alias(MemoryLocation(II->getArgOperand(1)), UseLoc);
266 return {AR == MustAlias, AR};
271 return {AR != NoAlias, AR};
267272 case Intrinsic::lifetime_end:
268273 case Intrinsic::invariant_start:
269274 case Intrinsic::invariant_end:
105105 store i32 %v2, i32* @G3
106106 ret void
107107 }
108
109 ;; Check that we respect lifetime.start/lifetime.end intrinsics when deleting
110 ;; stores that, without the lifetime calls, would be writebacks.
111 ; CHECK-LABEL: @test_writeback_lifetimes(
112 ; CHECK-NOMEMSSA-LABEL: @test_writeback_lifetimes(
113 define void @test_writeback_lifetimes(i32* %p) {
114 entry:
115 %q = getelementptr i32, i32* %p, i64 1
116 %pv = load i32, i32* %p
117 %qv = load i32, i32* %q
118 call void @llvm.lifetime.end.p0i8(i64 8, i32* %p)
119 call void @llvm.lifetime.start.p0i8(i64 8, i32* %p)
120 ; CHECK: store i32 %pv
121 ; CHECK-NOMEMSSA-LABEL: store i32 %pv
122 store i32 %pv, i32* %p
123 ; CHECK: store i32 %qv, i32* %q
124 ; CHECK-NOMEMSSA-LABEL: store i32 %qv, i32* %q
125 store i32 %qv, i32* %q
126 ret void
127 }
128
129 ;; Check that we respect lifetime.start/lifetime.end intrinsics when deleting
130 ;; stores that, without the lifetime calls, would be writebacks.
131 ; CHECK-LABEL: @test_writeback_lifetimes_multi_arg(
132 ; CHECK-NOMEMSSA-LABEL: @test_writeback_lifetimes_multi_arg(
133 define void @test_writeback_lifetimes_multi_arg(i32* %p, i32* %q) {
134 entry:
135 %pv = load i32, i32* %p
136 %qv = load i32, i32* %q
137 call void @llvm.lifetime.end.p0i8(i64 8, i32* %p)
138 call void @llvm.lifetime.start.p0i8(i64 8, i32* %p)
139 ; CHECK: store i32 %pv
140 ; CHECK-NOMEMSSA-LABEL: store i32 %pv
141 store i32 %pv, i32* %p
142 ; CHECK: store i32 %qv, i32* %q
143 ; CHECK-NOMEMSSA-LABEL: store i32 %qv, i32* %q
144 store i32 %qv, i32* %q
145 ret void
146 }
147
148 declare void @llvm.lifetime.end.p0i8(i64, i32*)
149 declare void @llvm.lifetime.start.p0i8(i64, i32*)
11981198 ++I;
11991199 }
12001200 }
1201
1202 TEST_F(MemorySSATest, LifetimeMarkersAreClobbers) {
1203 // Example code:
1204 // define void @a(i8* %foo) {
1205 // %bar = getelementptr i8, i8* %foo, i64 1
1206 // store i8 0, i8* %foo
1207 // store i8 0, i8* %bar
1208 // call void @llvm.lifetime.end.p0i8(i64 8, i32* %p)
1209 // call void @llvm.lifetime.start.p0i8(i64 8, i32* %p)
1210 // store i8 0, i8* %foo
1211 // store i8 0, i8* %bar
1212 // ret void
1213 // }
1214 //
1215 // Patterns like this are possible after inlining; the stores to %foo and %bar
1216 // should both be clobbered by the lifetime.start call if they're dominated by
1217 // it.
1218
1219 IRBuilder<> B(C);
1220 F = Function::Create(
1221 FunctionType::get(B.getVoidTy(), {B.getInt8PtrTy()}, false),
1222 GlobalValue::ExternalLinkage, "F", &M);
1223
1224 // Make blocks
1225 BasicBlock *Entry = BasicBlock::Create(C, "entry", F);
1226
1227 B.SetInsertPoint(Entry);
1228 Value *Foo = &*F->arg_begin();
1229
1230 Value *Bar = B.CreateGEP(Foo, B.getInt64(1), "bar");
1231
1232 B.CreateStore(B.getInt8(0), Foo);
1233 B.CreateStore(B.getInt8(0), Bar);
1234
1235 auto GetLifetimeIntrinsic = [&](Intrinsic::ID ID) {
1236 return Intrinsic::getDeclaration(&M, ID, {Foo->getType()});
1237 };
1238
1239 B.CreateCall(GetLifetimeIntrinsic(Intrinsic::lifetime_end),
1240 {B.getInt64(2), Foo});
1241 Instruction *LifetimeStart = B.CreateCall(
1242 GetLifetimeIntrinsic(Intrinsic::lifetime_start), {B.getInt64(2), Foo});
1243
1244 Instruction *FooStore = B.CreateStore(B.getInt8(0), Foo);
1245 Instruction *BarStore = B.CreateStore(B.getInt8(0), Bar);
1246
1247 setupAnalyses();
1248 MemorySSA &MSSA = *Analyses->MSSA;
1249
1250 MemoryAccess *LifetimeStartAccess = MSSA.getMemoryAccess(LifetimeStart);
1251 ASSERT_NE(LifetimeStartAccess, nullptr);
1252
1253 MemoryAccess *FooAccess = MSSA.getMemoryAccess(FooStore);
1254 ASSERT_NE(FooAccess, nullptr);
1255
1256 MemoryAccess *BarAccess = MSSA.getMemoryAccess(BarStore);
1257 ASSERT_NE(BarAccess, nullptr);
1258
1259 MemoryAccess *FooClobber =
1260 MSSA.getWalker()->getClobberingMemoryAccess(FooAccess);
1261 EXPECT_EQ(FooClobber, LifetimeStartAccess);
1262
1263 MemoryAccess *BarClobber =
1264 MSSA.getWalker()->getClobberingMemoryAccess(BarAccess);
1265 EXPECT_EQ(BarClobber, LifetimeStartAccess);
1266 }