[CodeGenPrepare] Handle all debug calls in dupRetToEnableTailCallOpts()

[jonpa]

Summary

This patch makes sure that a debug value that is after the bitcast in dupRetToEnableTailCallOpts() is also skipped.

The reduced test case is from SPEC-2006 on SystemZ.

[vsk]

Thanks for the patch. Could you please reduce the test case a bit further? There are kilobytes of debug info metadata here which don't appear necessary for reproducing the problem.

To start, I suggest running opt -strip -metarenamer -debugify on the IR input. That should still reproduce the issue.

[wolfgangp]

test/CodeGen/SystemZ/debuginstr-cgp.mir

55

I'm a bit confused about this. The change was about dbg.value instructions following bitcasts, but here we have the dbg.value following a getelementptr. Perhaps I'm missing something?

[jonpa]

Thanks for the patch. Could you please reduce the test case a bit further? There are kilobytes of debug info metadata > here which don't appear necessary for reproducing the problem.

To start, I suggest running opt -strip -metarenamer -debugify on the IR input. That should still reproduce the issue.

Ah, that's how you do it! :-)

Test case reduced.

test/CodeGen/SystemZ/debuginstr-cgp.mir

55

Ah.. that's what Bugpoint did. It seems that CodeGenPrepare replaced that GEP (with only 0 indexes) with a bitcast before reaching the point of interest. I replaced it with a Bitcast in the test case now for clarity.

[wolfgangp]

The test case looks much better now, thanks.

Usually we include a short set of instructions in the test case on how to produce the IR, including the source code, in case the IR needs to be re-generated in response to a format change etc.

Other than that, LGTM.

[jonpa]

In D57050#1368089, @wolfgangp wrote:

The test case looks much better now, thanks.

Usually we include a short set of instructions in the test case on how to produce the IR, including the source code, in case the IR needs to be re-generated in response to a format change etc.

Other than that, LGTM.

I was not aware of that... The Debug info is just all "dummy" which has just been copied-and-pasted into the test case. I'm afraid I think my .ll test case is gone at the moment. Do you really need me to regenerate it, or could it be accepted given its small size?

(I don't think reducing the source input is meaningful given that the debug-info is not "real", right?)

[wolfgangp]

If that's the case a brief excerpt of the original source construct (C, C++ or otherwise) that was used to produce the IR along with a couple of command lines to produce the MIR file should be sufficient. It doesn't have to be exact, but just so that the poor soul who has to regenerate the IR at some point in the future has something to go on. Check out llvm/test/CodeGen/MIR/X86/branch-folder-with-label.mir for an example.

[jonpa]

I made a new reduced MIR test case from the original source file and this time included the IR as a comment. Hope that works.

[vsk]

I see a minor issue in the test, but the patch looks great to me otherwise. Thanks!

test/CodeGen/SystemZ/debuginstr-cgp.mir

51

Something might have went wrong during the test update -- I think Wolfgang's comment about the missing bitcast applies again?

[jonpa]

Test updated once again to use a bitcast in the IR also before CodeGenPrepare.

[wolfgangp]

LGTM

[jonpa]

Thanks for review!
r352462

lib/CodeGen/CodeGenPrepare.cpp

bool CodeGenPrepare::dupRetToEnableTailCallOpts(BasicBlock *BB) {
 
   if (PN && PN->getParent() != BB)
     return false;
 
   // Make sure there are no instructions between the PHI and return, or that the
   // return is the first instruction in the block.
   if (PN) {
     BasicBlock::iterator BI = BB->begin();
-    do { ++BI; } while (isa<DbgInfoIntrinsic>(BI));
-    if (&*BI == BCI)
-      // Also skip over the bitcast.
-      ++BI;
+    // Skip over debug and the bitcast.
+    do { ++BI; } while (isa<DbgInfoIntrinsic>(BI) || &*BI == BCI);
     if (&*BI != RetI)
       return false;
   } else {
     BasicBlock::iterator BI = BB->begin();
     while (isa<DbgInfoIntrinsic>(BI)) ++BI;
     if (&*BI != RetI)
       return false;
   }

test/CodeGen/SystemZ/debuginstr-cgp.mir

+# Check that the codegenprepare succeeds in dupRetToEnableTailCallOpts() also
+# in the presence of a call to @llvm.dbg.value()
+#
+# RUN: llc %s -mtriple=s390x-linux-gnu -mcpu=z13 -start-before=codegenprepare \
+# RUN:   -stop-after codegenprepare -o - -debug-only=codegenprepare 2>&1 \
+# RUN:   | FileCheck %s
+# REQUIRES: asserts
+#
+# CHECK: To merge:
+# CHECK: bb1:
+# CHECK: To merge:
+# CHECK: bb7:
+#
+# CHECK: define dso_local %0* @Fun
+# CHECK-NOT: bb7:
+
+--- |
+  
+  %0 = type { i32 (...)**, i16, %1* }
+  %1 = type { i32 (...)** }
+  %2 = type { i32 (...)**, %1*, i8, i32, i32, i32, i16, i32, i16, i32, i16*, %3*, %6*, %9 }
+  %3 = type { %4 }
+  %4 = type { i32 (...)**, i8, i32, i32, %5**, %1* }
+  %5 = type { i32, i32 }
+  %6 = type { %7*, %0*, %0*, %0*, %0*, %0*, %0*, %0*, %0*, %0*, %0*, %0*, %0*, %0*, %0*, %1* }
+  %7 = type { %8 }
+  %8 = type { i32 (...)**, i8, i32, i32, %0**, %1* }
+  %9 = type { i8* }
+  %10 = type { %0, i32, i32, %0* }
+  
+  define dso_local %0* @Fun(%2* %arg) #0 {
+  bb:
+    br label %bb1, !dbg !7
+  
+  bb1:                                              ; preds = %bb
+    br i1 undef, label %bb2, label %bb3, !dbg !15
+  
+  bb2:                                              ; preds = %bb1
+    unreachable, !dbg !16
+  
+  bb3:                                              ; preds = %bb1
+    switch i32 undef, label %bb6 [
+      i32 58, label %bb4
+      i32 41, label %bb5
+    ], !dbg !17
+  
+  bb4:                                              ; preds = %bb3
+    %tmp = tail call %10* @Foo(%6* undef, %0* undef, i32 signext 0, i32 signext 0), !dbg !18
+    call void @llvm.dbg.value(metadata %10* %tmp, metadata !11, metadata !DIExpression()), !dbg !18
+    br label %bb7, !dbg !19
+  
+  bb5:                                              ; preds = %bb3
+    br label %bb7, !dbg !20
+  
+  bb6:                                              ; preds = %bb3
+    unreachable, !dbg !21
+  
+  bb7:                                              ; preds = %bb5, %bb4
+    %tmp8 = phi %10* [ %tmp, %bb4 ], [ undef, %bb5 ], !dbg !22
+    call void @llvm.dbg.value(metadata %10* %tmp8, metadata !13, metadata !DIExpression()), !dbg !22
+    %tmp9 = bitcast %10* %tmp8 to %0*
+    call void @llvm.dbg.value(metadata %0* %tmp9, metadata !14, metadata !DIExpression()), !dbg !23
+    ret %0* %tmp9, !dbg !24
+  }
+  
+  ; Function Attrs: nounwind readnone speculatable
+  declare void @llvm.dbg.value(metadata, metadata, metadata) #1
+  
+  declare dso_local %10* @Foo(%6*, %0*, i32, i32) #0
+  
+  attributes #0 = { "target-cpu"="z13" }
+  attributes #1 = { nounwind readnone speculatable "target-cpu"="z13" }
+  
+  !llvm.module.flags = !{!0, !1}
+  !llvm.dbg.cu = !{!2}
+  !llvm.debugify = !{!5, !6}
+  
+  !0 = !{i32 2, !"Debug Info Version", i32 3}
+  !1 = !{i32 1, !"wchar_size", i32 4}
+  !2 = distinct !DICompileUnit(language: DW_LANG_C, file: !3, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4)
+  !3 = !DIFile(filename: "tc.ll", directory: "/")
+  !4 = !{}
+  !5 = !{i32 11}
+  !6 = !{i32 3}
+  !7 = !DILocation(line: 1, column: 1, scope: !8)
+  !8 = distinct !DISubprogram(name: "widget.84", linkageName: "widget.84", scope: null, file: !3, line: 1, type: !9, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !10)
+  !9 = !DISubroutineType(types: !4)
+  !10 = !{!11, !13, !14}
+  !11 = !DILocalVariable(name: "1", scope: !8, file: !3, line: 5, type: !12)
+  !12 = !DIBasicType(name: "ty64", size: 64, encoding: DW_ATE_unsigned)
+  !13 = !DILocalVariable(name: "2", scope: !8, file: !3, line: 9, type: !12)
+  !14 = !DILocalVariable(name: "3", scope: !8, file: !3, line: 10, type: !12)
+  !15 = !DILocation(line: 2, column: 1, scope: !8)
+  !16 = !DILocation(line: 3, column: 1, scope: !8)
+  !17 = !DILocation(line: 4, column: 1, scope: !8)
+  !18 = !DILocation(line: 5, column: 1, scope: !8)
+  !19 = !DILocation(line: 6, column: 1, scope: !8)
+  !20 = !DILocation(line: 7, column: 1, scope: !8)
+  !21 = !DILocation(line: 8, column: 1, scope: !8)
+  !22 = !DILocation(line: 9, column: 1, scope: !8)
+  !23 = !DILocation(line: 10, column: 1, scope: !8)
+  !24 = !DILocation(line: 11, column: 1, scope: !8)
+
+...
+---
+name:            Fun
+alignment:       4
+tracksRegLiveness: true
+...