Change how finalizeBundle selects debug location for the BUNDLE instruction

[bjope]

Summary

Previously a BUNDLE instruction inherited the DebugLoc from the
first instruction in the bundle, even if that DebugLoc had no
DILocation. With this commit this is changed into selecting the
first DebugLoc that has a DILocation, by searching among the
bundled instructions.

The idea is to reduce amount of bundles that are lacking
debug locations.

[mattd]

I like this change! Is there ever the case where there would be two different DILocations within a bundle? Perhaps that questions doesn't matter since bundles are treated as a single item.

[JDevlieghere]

This LGTM!

llvm/trunk/lib/CodeGen/MachineInstrBundle.cpp

 char &llvm::FinalizeMachineBundlesID = FinalizeMachineBundles::ID;
 INITIALIZE_PASS(FinalizeMachineBundles, "finalize-mi-bundles",
                 "Finalize machine instruction bundles", false, false)
 
 bool FinalizeMachineBundles::runOnMachineFunction(MachineFunction &MF) {
   return llvm::finalizeBundles(MF);
 }
 
+/// Return the first found DebugLoc that has a DILocation, given a range of
+/// instructions. The search range is from FirstMI to LastMI (exclusive). If no
+/// DILocation is found, then an empty location is returned.
+static DebugLoc getDebugLoc(MachineBasicBlock::instr_iterator FirstMI,
+                            MachineBasicBlock::instr_iterator LastMI) {
+  for (auto MII = FirstMI; MII != LastMI; ++MII)
+    if (MII->getDebugLoc().get())
+      return MII->getDebugLoc();
+  return DebugLoc();
+}
 
 /// finalizeBundle - Finalize a machine instruction bundle which includes
 /// a sequence of instructions starting from FirstMI to LastMI (exclusive).
 /// This routine adds a BUNDLE instruction to represent the bundle, it adds
 /// IsInternalRead markers to MachineOperands which are defined inside the
 /// bundle, and it copies externally visible defs and uses to the BUNDLE
 /// instruction.
 void llvm::finalizeBundle(MachineBasicBlock &MBB,
                           MachineBasicBlock::instr_iterator FirstMI,
                           MachineBasicBlock::instr_iterator LastMI) {
   assert(FirstMI != LastMI && "Empty bundle?");
   MIBundleBuilder Bundle(MBB, FirstMI, LastMI);
 
   MachineFunction &MF = *MBB.getParent();
   const TargetInstrInfo *TII = MF.getSubtarget().getInstrInfo();
   const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
 
   MachineInstrBuilder MIB =
-      BuildMI(MF, FirstMI->getDebugLoc(), TII->get(TargetOpcode::BUNDLE));
+      BuildMI(MF, getDebugLoc(FirstMI, LastMI), TII->get(TargetOpcode::BUNDLE));
   Bundle.prepend(MIB);
 
   SmallVector<unsigned, 32> LocalDefs;
   SmallSet<unsigned, 32> LocalDefSet;
   SmallSet<unsigned, 8> DeadDefSet;
   SmallSet<unsigned, 16> KilledDefSet;
   SmallVector<unsigned, 8> ExternUses;
   SmallSet<unsigned, 8> ExternUseSet;

llvm/trunk/test/CodeGen/Hexagon/packetize-debug-loc.mir

+# RUN: llc -march=hexagon -run-pass hexagon-packetizer %s -o - | FileCheck %s
+
+##############################################################################
+# This test case is not really hexagon specific, but we use hexagon to get
+# bundling.
+#
+# The goal is to verify that the BUNDLE instruction is getting the first
+# non-empty debug location among the bundled instructions.
+##############################################################################
+
+--- |
+  define void @test() { ret void }
+
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "llvm", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !3, retainedTypes: !3)
+  !1 = distinct !DISubprogram(name: "test", scope: !3, file: !3, line: 1, type: !2, isLocal: false, isDefinition: true, scopeLine: 2, flags: DIFlagPrototyped, isOptimized: true, unit: !0)
+  !2 = !DISubroutineType(types: !3)
+  !3 = !{null}
+  !5 = !DILocation(line: 126, column: 9, scope: !2)
+  !6 = !DILocation(line: 321, column: 4, scope: !2)
+
+...
+---
+name: test
+tracksRegLiveness: true
+body: |
+  bb.0:
+    liveins: $r1
+    $r2 = L2_loadri_io $r1, 0, debug-location !5
+    $r3 = L2_loadri_io $r1, 0, debug-location !6
+
+    $r4 = L2_loadri_io $r1, 0
+    $r5 = L2_loadri_io $r1, 0, debug-location !5
+
+    $r6 = L2_loadri_io $r1, 0, debug-location !6
+    $r7 = L2_loadri_io $r1, 0
+
+    $r8 = L2_loadri_io $r1, 0
+    $r9 = L2_loadri_io $r1, 0
+...
+
+# CHECK-LABEL: name: test
+
+# CHECK: BUNDLE
+# CHECK-SAME: debug-location [[DL1:[0-9x<>]+]]
+# CHECK-NEXT: L2_loadri_io $r1, 0, debug-location [[DL1]]
+# CHECK-NEXT: L2_loadri_io $r1, 0, debug-location [[DL2:[0-9x<>]+]]
+
+# CHECK: BUNDLE
+# CHECK-SAME: debug-location [[DL1]]
+# CHECK-NEXT: L2_loadri_io $r1, 0{{$}}
+# CHECK-NEXT: L2_loadri_io $r1, 0, debug-location [[DL1]]
+
+# CHECK: BUNDLE
+# CHECK-SAME: debug-location [[DL2]]
+# CHECK-NEXT: L2_loadri_io $r1, 0, debug-location [[DL2]]
+
+# No debug-location printouts expected for the last bundle
+# CHECK-NOT: debug-location