[GlobalISel] Don't emit lost debug location remarks when legalizing tail calls

There were a bunch of lost debug location remarks that show up when legalizing
tail calls on AArch64.

This would happen because we drop the return in the block where we emit the
tail call. So, we end up dropping the debug location, which makes the
LostDebugLocObserver report a missing debug location.

Although it's *true* that we lose these debug locations, this isn't
a particularly useful remark. We expect to drop these debug locations when
emitting tail calls. Suppressing remarks in this case is preferable, since the
amount of noise could hide actual debug location related bugs.

To do this, I just plumbed the LostDebugLocObserver through the relevant
LegalizerHelper functions. This is the only case I can think of where we need
the LostDebugLocObserver in the LegalizerHelper. So, rather than storing it
in the LegalizerHelper proper and mucking around with the constructors, I
figured it'd be cleanest to take the simplest path for now.

This clears up ~20 noisy lost debug location remarks on CTMark in AArch64 at
-Os.

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

llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h

@@ -32,6 +32,7 @@ class LegalizerInfo;
 class Legalizer;
 class MachineRegisterInfo;
 class GISelChangeObserver;
+class LostDebugLocObserver;
 class TargetLowering;
 
 class LegalizerHelper {
@@ -78,10 +79,11 @@ public:
   ///
   /// Considered as an opaque blob, the legal code will use and define the same
   /// registers as \p MI.
-  LegalizeResult legalizeInstrStep(MachineInstr &MI);
+  LegalizeResult legalizeInstrStep(MachineInstr &MI,
+                                   LostDebugLocObserver &LocObserver);
 
   /// Legalize an instruction by emiting a runtime library call instead.
-  LegalizeResult libcall(MachineInstr &MI);
+  LegalizeResult libcall(MachineInstr &MI, LostDebugLocObserver &LocObserver);
 
   /// Legalize an instruction by reducing the width of the underlying scalar
   /// type.
@@ -408,9 +410,9 @@ createLibcall(MachineIRBuilder &MIRBuilder, RTLIB::Libcall Libcall,
               ArrayRef<CallLowering::ArgInfo> Args);
 
 /// Create a libcall to memcpy et al.
-LegalizerHelper::LegalizeResult createMemLibcall(MachineIRBuilder &MIRBuilder,
-                                                 MachineRegisterInfo &MRI,
-                                                 MachineInstr &MI);
+LegalizerHelper::LegalizeResult
+createMemLibcall(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI,
+                 MachineInstr &MI, LostDebugLocObserver &LocObserver);
 
 } // End namespace llvm.
 

llvm/lib/CodeGen/GlobalISel/Legalizer.cpp

@@ -230,7 +230,7 @@ Legalizer::legalizeMachineFunction(MachineFunction &MF, const LegalizerInfo &LI,
       }
 
       // Do the legalization for this instruction.
-      auto Res = Helper.legalizeInstrStep(MI);
+      auto Res = Helper.legalizeInstrStep(MI, LocObserver);
       // Error out if we couldn't legalize this instruction. We may want to
       // fall back to DAG ISel instead in the future.
       if (Res == LegalizerHelper::UnableToLegalize) {

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

@@ -16,6 +16,7 @@
 #include "llvm/CodeGen/GlobalISel/CallLowering.h"
 #include "llvm/CodeGen/GlobalISel/GISelChangeObserver.h"
 #include "llvm/CodeGen/GlobalISel/LegalizerInfo.h"
+#include "llvm/CodeGen/GlobalISel/LostDebugLocObserver.h"
 #include "llvm/CodeGen/GlobalISel/MIPatternMatch.h"
 #include "llvm/CodeGen/GlobalISel/Utils.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
@@ -101,7 +102,8 @@ LegalizerHelper::LegalizerHelper(MachineFunction &MF, const LegalizerInfo &LI,
     TLI(*MF.getSubtarget().getTargetLowering()) { }
 
 LegalizerHelper::LegalizeResult
-LegalizerHelper::legalizeInstrStep(MachineInstr &MI) {
+LegalizerHelper::legalizeInstrStep(MachineInstr &MI,
+                                   LostDebugLocObserver &LocObserver) {
   LLVM_DEBUG(dbgs() << "Legalizing: " << MI);
 
   MIRBuilder.setInstrAndDebugLoc(MI);
@@ -116,7 +118,7 @@ LegalizerHelper::legalizeInstrStep(MachineInstr &MI) {
     return AlreadyLegal;
   case Libcall:
     LLVM_DEBUG(dbgs() << ".. Convert to libcall\n");
-    return libcall(MI);
+    return libcall(MI, LocObserver);
   case NarrowScalar:
     LLVM_DEBUG(dbgs() << ".. Narrow scalar\n");
     return narrowScalar(MI, Step.TypeIdx, Step.NewType);
@@ -562,7 +564,7 @@ simpleLibcall(MachineInstr &MI, MachineIRBuilder &MIRBuilder, unsigned Size,
 
 LegalizerHelper::LegalizeResult
 llvm::createMemLibcall(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI,
-                       MachineInstr &MI) {
+                       MachineInstr &MI, LostDebugLocObserver &LocObserver) {
   auto &Ctx = MIRBuilder.getMF().getFunction().getContext();
 
   SmallVector<CallLowering::ArgInfo, 3> Args;
@@ -620,8 +622,13 @@ llvm::createMemLibcall(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI,
   if (!CLI.lowerCall(MIRBuilder, Info))
     return LegalizerHelper::UnableToLegalize;
 
+
   if (Info.LoweredTailCall) {
     assert(Info.IsTailCall && "Lowered tail call when it wasn't a tail call?");
+
+    // Check debug locations before removing the return.
+    LocObserver.checkpoint(true);
+
     // We must have a return following the call (or debug insts) to get past
     // isLibCallInTailPosition.
     do {
@@ -632,6 +639,9 @@ llvm::createMemLibcall(MachineIRBuilder &MIRBuilder, MachineRegisterInfo &MRI,
       // Delete the old return.
       Next->eraseFromParent();
     } while (MI.getNextNode());
+
+    // We expect to lose the debug location from the return.
+    LocObserver.checkpoint(false);
   }
 
   return LegalizerHelper::Legalized;
@@ -668,7 +678,7 @@ conversionLibcall(MachineInstr &MI, MachineIRBuilder &MIRBuilder, Type *ToType,
 }
 
 LegalizerHelper::LegalizeResult
-LegalizerHelper::libcall(MachineInstr &MI) {
+LegalizerHelper::libcall(MachineInstr &MI, LostDebugLocObserver &LocObserver) {
   LLT LLTy = MRI.getType(MI.getOperand(0).getReg());
   unsigned Size = LLTy.getSizeInBits();
   auto &Ctx = MIRBuilder.getMF().getFunction().getContext();
@@ -765,7 +775,7 @@ LegalizerHelper::libcall(MachineInstr &MI) {
   case TargetOpcode::G_MEMMOVE:
   case TargetOpcode::G_MEMSET: {
     LegalizeResult Result =
-        createMemLibcall(MIRBuilder, *MIRBuilder.getMRI(), MI);
+        createMemLibcall(MIRBuilder, *MIRBuilder.getMRI(), MI, LocObserver);
     if (Result != Legalized)
       return Result;
     MI.eraseFromParent();

llvm/test/CodeGen/AArch64/GlobalISel/debug-loc-legalize-tail-call.mir

@@ -0,0 +1,43 @@
+# RUN: llc %s -mtriple=aarch64-unknown-unknown -run-pass=legalizer -verify-machineinstrs -pass-remarks-missed=gisel* -o - 2>&1 | FileCheck %s
+
+# When we create a tail call, we expect to drop the return's debug location.
+# Ensure that we don't get a missed remark for debug locations in this case.
+
+# CHECK-NOT: remark: file.ll:[[#]]:[[#]]: lost [[#]] debug locations during pass
+
+--- |  
+  define void @snork() !dbg !6 { unreachable }
+  
+  !llvm.module.flags = !{!0}
+  !llvm.dbg.cu = !{!1}
+  !llvm.debugify = !{!4, !5}
+  
+  !0 = !{i32 2, !"Debug Info Version", i32 3}
+  !1 = distinct !DICompileUnit(language: DW_LANG_C, file: !2, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !3)
+  !2 = !DIFile(filename: "file.ll", directory: "/")
+  !3 = !{}
+  !4 = !{i32 2}
+  !5 = !{i32 1}
+  !6 = distinct !DISubprogram(name: "snork", linkageName: "snork", scope: null, file: !2, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !1, retainedNodes: !8)
+  !7 = !DISubroutineType(types: !3)
+  !8 = !{!9}
+  !9 = !DILocalVariable(name: "1", scope: !6, file: !2, line: 2, type: !10)
+  !10 = !DIBasicType(name: "ty32", size: 32, encoding: DW_ATE_unsigned)
+  !11 = !DILocation(line: 1, column: 1, scope: !6)
+  !12 = !DILocation(line: 2, column: 1, scope: !6)
+
+...
+---
+name:            snork
+alignment:       4
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    %0:_(p0) = G_IMPLICIT_DEF debug-location !DILocation(line: 0, scope: !6)
+    %1:_(s8) = G_CONSTANT i8 0
+    %2:_(s64) = G_IMPLICIT_DEF debug-location !DILocation(line: 0, scope: !6)
+    G_MEMSET %0(p0), %1(s8), %2(s64), 1, debug-location !11 :: (store 1)
+    DBG_VALUE 0, 0, !9, !DIExpression(), debug-location !12
+    RET_ReallyLR debug-location !12
+
+...