[MemorySSAUpdater] Avoid creating self-referencing MemoryDefs

Fix for https://bugs.llvm.org/show_bug.cgi?id=38807, which occurred
while compiling SemaTemplateInstantiate.cpp with clang and GVNHoist
enabled. In the following example:

      1=def(entry)
      /        \
2=def(1)       4=def(1)
3=def(2)       5=def(4)

When removing the MemoryDef 2=def(1) from its basic block, and just
before adding it to the end of the parent basic block, we first
replace all its uses with the defining memory access:

3=def(2) -> 3=def(1)

Then we call insertDef for adding 2=def(1) to the parent basic block,
where we replace the uses of 1=def(entry) with 2=def(1). Doing so we
create a self reference:

2=def(1) -> 2=def(2)  (bad)
3=def(1) -> 3=def(2)  (ok)
4=def(1) -> 4=def(2)  (ok)

Differential Revision: https://reviews.llvm.org/D51801

llvm-svn: 341947
This commit is contained in:
Alexandros Lamprineas 2018-09-11 14:29:59 +00:00
parent db597b91b7
commit 8d73ea30c1
2 changed files with 53 additions and 4 deletions

View File

@ -266,16 +266,15 @@ void MemorySSAUpdater::insertDef(MemoryDef *MD, bool RenameUses) {
for (auto UI = DefBefore->use_begin(), UE = DefBefore->use_end();
UI != UE;) {
Use &U = *UI++;
// Leave the uses alone
if (isa<MemoryUse>(U.getUser()))
// Leave the MemoryUses alone.
// Also make sure we skip ourselves to avoid self references.
if (isa<MemoryUse>(U.getUser()) || U.getUser() == MD)
continue;
U.set(MD);
}
}
// and that def is now our defining access.
// We change them in this order otherwise we will appear in the use list
// above and reset ourselves.
MD->setDefiningAccess(DefBefore);
SmallVector<WeakVH, 8> FixupList(InsertedPHIs.begin(), InsertedPHIs.end());

View File

@ -0,0 +1,50 @@
; RUN: opt < %s -early-cse-memssa -gvn-hoist -S | FileCheck %s
; Make sure opt doesn't crash. On top of that, the instructions
; of the side blocks should be hoisted to the entry block.
%s = type { i32, i64 }
%S = type { %s, i32 }
;CHECK-LABEL: @foo
define void @foo(i32* %arg) {
bb0:
%0 = bitcast i32* %arg to %S*
%call.idx.i = getelementptr %S, %S* %0, i64 0, i32 0, i32 0
%call.idx.val.i = load i32, i32* %call.idx.i
br label %bb1
;CHECK: bb1:
;CHECK: %call264 = call zeroext i1 @bar
;CHECK: store i32 %call.idx.val.i, i32* %call.idx.i
;CHECK: %1 = getelementptr inbounds %S, %S* %0, i64 0, i32 0, i32 1
;CHECK: store i64 undef, i64* %1
;CHECK: br i1 %call264, label %bb2, label %bb3
bb1:
%call264 = call zeroext i1 @bar()
br i1 %call264, label %bb2, label %bb3
;CHECK: bb2:
;CHECK-NOT: store i32 %call.idx.val.i, i32* %call.idx.i
;CHECK-NOT: store i64 undef, i64* %{.*}
bb2:
store i32 %call.idx.val.i, i32* %call.idx.i
%1 = getelementptr inbounds %S, %S* %0, i64 0, i32 0, i32 1
store i64 undef, i64* %1
ret void
;CHECK: bb3:
;CHECK-NOT: store i32 %call.idx.val.i, i32* %call.idx.i
;CHECK-NOT: store i64 undef, i64* %{.*}
bb3:
store i32 %call.idx.val.i, i32* %call.idx.i
%2 = getelementptr inbounds %S, %S* %0, i64 0, i32 0, i32 1
store i64 undef, i64* %2
ret void
}
declare zeroext i1 @bar()