[MachineOutliner] Do not outline debug instructions

[chill]

Summary

The debug location is removed from any outlined instruction. This causes the
MachineVerifier to crash on outlined DBG_VALUE instructions.

Then, debug instructions are "invisible" to the outliner, that is, two ranges of
instructions from different functions are considered identical if the only
difference is debug instructions (is that right?). Since a debug instruction
from one function is unlikely to provide sensible debug information about all
functions, sharing an outlined sequence, this patch just removes debug
instructions from the outlined functions.

[jmorse]

(Adding debug-info),

If I understand correctly, the outliner "commons" code between different functions, yes? In that case, then as you say it's definitely correct to drop variable locations, they'll be especially broken seeing how the outlined function is given a separate DISubprogram.

[paquette]

This seems correct to me. Outlined functions should not contain debug instructions.

Only thing is that it'd be a lot nicer to have a MIR test here. That would make it much clearer what the testcase is actually doing.

llvm/test/CodeGen/ARM/machine-outliner-remove-debug-instr.ll

1 ↗

(On Diff #298411)

Can this be a MIR test instead?

[chill]

Ping.

[paquette]

llvm/test/CodeGen/ARM/machine-outliner-remove-debug-instr.mir

2

• You can pull the triple into the runline
• Don't need -- for all of the flags

15

Remove comments in the IR

40

I don't think you need to include the actual IR here?

This should work:

define i32 @h() { ret void }
define i32 @f() { ret void }
define i32 @g() { ret void }
...

If it doesn't work, then the MIR is referencing the IR in some way. It may be possible to just drop the references to the IR in the MIR in order to get the simplest test possible.

It would be ideal to end up with something like this:

# RUN: ...
--- |
define i32 @h() { ret void }
define i32 @f() { ret void }
define i32 @g() { ret void }
...
---
name:            h
... MIR test ...

Or at least as close to the above as possible.

171

I don't think most of the entries here are necessary.

I think all you need is something like

name:            h
tracksRegLiveness: true
body:             |

(see: llvm/test/CodeGen/AArch64/machine-outliner-calls.mir for an example)

200

I think you can drop the references to the IR here.

(Judging by llvm/test/CodeGen/AArch64/ldst-opt-mte-with-dbg.mir)

[paquette]

Can you reduce the contents of f, g, and h so that you only have

• The instructions which are outlined
• Debug instructions which should not be outlined

This will make the testcase a lot easier for people reading it to understand what's going on. It will also make it less likely for it to fail because of unrelated changes.

llvm/test/CodeGen/ARM/machine-outliner-remove-debug-instr.mir

15

This part of the IR isn't necessary

16

You can remove this part too

39

Is this function outlined from at all? Can it just be removed?

[chill]

That should be the minimum number of instructions now, one less and the outlining does not pass the cost threshold.

[paquette]

LGTM!

[chill]

Cheers!

llvm/lib/CodeGen/MachineOutliner.cpp

MachineFunction *MachineOutliner::createOutlinedFunction(
   // Insert the new function into the module.
   MF.insert(MF.begin(), &MBB);
 
   MachineFunction *OriginalMF = FirstCand.front()->getMF();
   const std::vector<MCCFIInstruction> &Instrs =
       OriginalMF->getFrameInstructions();
   for (auto I = FirstCand.front(), E = std::next(FirstCand.back()); I != E;
        ++I) {
+    if (I->isDebugInstr())
+      continue;
     MachineInstr *NewMI = MF.CloneMachineInstr(&*I);
     if (I->isCFIInstruction()) {
       unsigned CFIIndex = NewMI->getOperand(0).getCFIIndex();
       MCCFIInstruction CFI = Instrs[CFIIndex];
       (void)MF.addFrameInst(CFI);
     }
     NewMI->dropMemRefs(MF);
 

llvm/test/CodeGen/ARM/machine-outliner-remove-debug-instr.mir

+# RUN: llc -verify-machineinstrs -run-pass=machine-outliner -mtriple=thumbv7m-none-eabi %s -o - | FileCheck %s
+
+# Check the outlined function does not contain debug instructions
+# CHECK-LABEL: name: f
+# CHECK:       tBL {{.*}}  @OUTLINED_FUNCTION_0,
+
+# CHECK-LABEL: name: g
+# CHECK:       tBL {{.*}}  @OUTLINED_FUNCTION_0,
+
+# CHECK-LABEL: name: OUTLINED_FUNCTION_0
+# CHECK-NOT:   DBG_VALUE
+# CHECK:       tTAILJMPdND @h
+--- |
+  define void @f() { entry: ret void }
+
+  define void @g() { entry: ret void }
+
+  declare void @h()
+...
+---
+name:            f
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $r0, $r1, $r2, $r3, $r4, $lr
+
+    frame-setup tPUSH 14, $noreg, killed $r4, killed $lr, implicit-def $sp, implicit $sp
+
+    $r4 = tMOVr $r1, 14, $noreg
+    DBG_VALUE $r4, $noreg
+    renamable $r0, dead $cpsr = nsw tMUL renamable $r1, killed renamable $r0, 14, $noreg
+    renamable $r0, dead $cpsr = nsw tSUBrr killed renamable $r0, renamable $r1, 14, $noreg
+    tBL 14, $noreg, @h, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit killed $r1, implicit-def $sp, implicit-def $r0
+
+    frame-destroy tPOP_RET 14, $noreg, def $r4, def $pc, implicit killed $r0
+...
+---
+name:            g
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $r0, $r1, $r2, $r3, $r4, $lr
+
+    frame-setup tPUSH 14, $noreg, killed $r4, killed $lr, implicit-def $sp, implicit $sp
+
+    $r4 = tMOVr $r1, 14, $noreg
+    DBG_VALUE $r4, $noreg
+    renamable $r0, dead $cpsr = nsw tMUL renamable $r1, killed renamable $r0, 14, $noreg
+    renamable $r0, dead $cpsr = nsw tSUBrr killed renamable $r0, renamable $r1, 14, $noreg
+    tBL 14, $noreg, @h, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit killed $r1, implicit-def $sp, implicit-def $r0
+
+    frame-destroy tPOP_RET 14, $noreg, def $r4, def $pc, implicit killed $r0
+...