[llvm] 64249f1 - [CodeExtractor] Fix extraction of a value used only by intrinsics outside of region

[Ehud Katz via llvm-commits]

Author: Ehud Katz
Date: 2020-04-25T11:44:47+03:00
New Revision: 64249f177e4644cfe3a0ba80ade274175771950c

URL: https://github.com/llvm/llvm-project/commit/64249f177e4644cfe3a0ba80ade274175771950c
DIFF: https://github.com/llvm/llvm-project/commit/64249f177e4644cfe3a0ba80ade274175771950c.diff

LOG: [CodeExtractor] Fix extraction of a value used only by intrinsics outside of region

We should only skip `lifetime` and `dbg` intrinsics when searching for users.
Other intrinsics are legit users that can't be ignored.

Without this fix, the testcase would result in an invalid IR. `memcpy`
will have a reference to the, now, external value (local to the
extracted loop function).

Fix PR42194

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

Added: 
    llvm/test/Transforms/CodeExtractor/LoopExtractor_alloca.ll

Modified: 
    llvm/lib/Transforms/Utils/CodeExtractor.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Utils/CodeExtractor.cpp b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
index a261040d10bc..5532c2cc5aaf 100644
--- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -451,18 +451,24 @@ CodeExtractor::getLifetimeMarkers(const CodeExtractorAnalysisCache &CEAC,
   for (User *U : Addr->users()) {
     IntrinsicInst *IntrInst = dyn_cast<IntrinsicInst>(U);
     if (IntrInst) {
+      // We don't model addresses with multiple start/end markers, but the
+      // markers do not need to be in the region.
       if (IntrInst->getIntrinsicID() == Intrinsic::lifetime_start) {
-        // Do not handle the case where Addr has multiple start markers.
         if (Info.LifeStart)
           return {};
         Info.LifeStart = IntrInst;
+        continue;
       }
       if (IntrInst->getIntrinsicID() == Intrinsic::lifetime_end) {
         if (Info.LifeEnd)
           return {};
         Info.LifeEnd = IntrInst;
+        continue;
       }
-      continue;
+      // At this point, permit debug uses outside of the region.
+      // This is fixed in a later call to fixupDebugInfoPostExtraction().
+      if (isa<DbgInfoIntrinsic>(IntrInst))
+        continue;
     }
     // Find untracked uses of the address, bail.
     if (!definedInRegion(Blocks, U))

diff  --git a/llvm/test/Transforms/CodeExtractor/LoopExtractor_alloca.ll b/llvm/test/Transforms/CodeExtractor/LoopExtractor_alloca.ll
new file mode 100644
index 000000000000..33de795eb200
--- /dev/null
+++ b/llvm/test/Transforms/CodeExtractor/LoopExtractor_alloca.ll
@@ -0,0 +1,58 @@
+; RUN: opt -debugify -loop-extract -S < %s | FileCheck %s
+
+; This tests 2 cases:
+; 1. loop1 should be extracted into a function, without extracting %v1 alloca.
+; 2. loop2 should be extracted into a function, with the %v2 alloca.
+;
+; This used to produce an invalid IR, where `memcpy` will have a reference to
+; the, now, external value (local to the extracted loop function).
+
+; CHECK-LABEL: define void @test()
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   %v1 = alloca i32
+; CHECK-NEXT:   call void @llvm.dbg.value(metadata i32* %v1
+; CHECK-NEXT:   %p1 = bitcast i32* %v1 to i8*
+; CHECK-NEXT:   call void @llvm.dbg.value(metadata i8* %p1,
+; CHECK-NEXT:   call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 undef, i8* %p1, i64 4, i1 true)
+
+; CHECK-LABEL: define internal void @test.loop2()
+; CHECK-NEXT: newFuncRoot:
+; CHECK-NEXT:   %v2 = alloca i32
+; CHECK-NEXT:   %p2 = bitcast i32* %v2 to i8*
+
+; CHECK-LABEL: define internal void @test.loop1(i8* %p1)
+; CHECK-NEXT: newFuncRoot:
+; CHECK-NEXT:   br
+
+define void @test() {
+entry:
+  %v1 = alloca i32, align 4
+  %v2 = alloca i32, align 4
+  %p1 = bitcast i32* %v1 to i8*
+  %p2 = bitcast i32* %v2 to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 undef, i8* %p1, i64 4, i1 true)
+  br label %loop1
+
+loop1:
+  call void @llvm.lifetime.start.p0i8(i64 4, i8* %p1)
+  %r1 = call i32 @foo(i8* %p1)
+  call void @llvm.lifetime.end.p0i8(i64 4, i8* %p1)
+  %cmp1 = icmp ne i32 %r1, 0
+  br i1 %cmp1, label %loop1, label %loop2
+
+loop2:
+  call void @llvm.lifetime.start.p0i8(i64 4, i8* %p2)
+  %r2 = call i32 @foo(i8* %p2)
+  call void @llvm.lifetime.end.p0i8(i64 4, i8* %p2)
+  %cmp2 = icmp ne i32 %r2, 0
+  br i1 %cmp2, label %loop2, label %exit
+
+exit:
+  ret void
+}
+
+declare i32 @foo(i8*)
+
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg)