[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

14

This part of the IR isn't necessary

15

You can remove this part too

38

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
+--- |
+  source_filename = "x.ll"
+  target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+
+  define void @f() {
+    entry:
+      br label %if.end
+    if.end:
+      br label %return
+    return:
+      ret void
+  }
+
+  define void @g() {
+  entry:
+    br label %if.end
+  if.end:
+    br label %return
+  return:
+    ret void
+  }
+
+  define void @h() { entry: ret void }
+...
+---
+name:            h
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $r0, $r1
+
+    DBG_VALUE $r0, $noreg
+    DBG_VALUE $r1, $noreg
+    renamable $r0 = nsw tADDhirr killed renamable $r0, killed renamable $r1, 14 /* CC::al */, $noreg
+    DBG_VALUE $r0, $noreg
+    tBX_RET 14 /* CC::al */, $noreg, implicit killed $r0
+...
+---
+name:            f
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    successors: %bb.1(0x30000000), %bb.2(0x50000000)
+    liveins: $r0, $r1, $r2, $r3, $lr, $r6, $r5, $r4
+
+    DBG_VALUE $r0, $noreg
+    DBG_VALUE $r1, $noreg
+    DBG_VALUE $r2, $noreg
+    DBG_VALUE $r3, $noreg
+    tCMPi8 renamable $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
+    t2Bcc %bb.1, 4 /* CC::mi */, killed $cpsr
+
+  bb.2.if.end:
+    liveins: $r0, $r1, $r2, $r3, $r4, $r5, $r6, $lr
+
+    DBG_VALUE $r3, $noreg
+    DBG_VALUE $r2, $noreg
+    DBG_VALUE $r1, $noreg
+    DBG_VALUE $r0, $noreg
+    frame-setup tPUSH 14 /* CC::al */, $noreg, killed $r4, killed $r5, killed $r6, killed $lr, implicit-def $sp, implicit $sp
+    frame-setup CFI_INSTRUCTION def_cfa_offset 16
+    frame-setup CFI_INSTRUCTION offset $lr, -4
+    frame-setup CFI_INSTRUCTION offset $r6, -8
+    frame-setup CFI_INSTRUCTION offset $r5, -12
+    frame-setup CFI_INSTRUCTION offset $r4, -16
+    $r6 = tMOVr killed $r1, 14 /* CC::al */, $noreg
+    DBG_VALUE $r6, $noreg
+    DBG_VALUE $r6, $noreg
+    renamable $r1, dead $cpsr = tMOVi8 11, 14 /* CC::al */, $noreg
+    renamable $r0, dead $cpsr = nsw tMUL killed renamable $r1, killed renamable $r0, 14 /* CC::al */, $noreg
+    DBG_VALUE $r0, $noreg
+    $r1 = tMOVr $r6, 14 /* CC::al */, $noreg
+    $r4 = tMOVr killed $r3, 14 /* CC::al */, $noreg
+    DBG_VALUE $r4, $noreg
+    $r5 = tMOVr killed $r2, 14 /* CC::al */, $noreg
+    DBG_VALUE $r5, $noreg
+    renamable $r0, dead $cpsr = nsw tSUBrr killed renamable $r0, renamable $r6, 14 /* CC::al */, $noreg
+    tBL 14 /* CC::al */, $noreg, @h, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit killed $r1, implicit-def $sp, implicit-def $r0
+    DBG_VALUE $r0, $noreg
+    renamable $r1, dead $cpsr = nsw tADDrr renamable $r0, killed renamable $r6, 14 /* CC::al */, $noreg
+    renamable $r0, dead $cpsr = nsw tMUL killed renamable $r1, killed renamable $r0, 14 /* CC::al */, $noreg
+    DBG_VALUE $r0, $noreg
+    renamable $r1, dead $cpsr = nsw tADDrr killed renamable $r4, killed renamable $r5, 14 /* CC::al */, $noreg
+    renamable $r0 = t2SDIV killed renamable $r0, killed renamable $r1, 14 /* CC::al */, $noreg
+    renamable $r0, dead $cpsr = nsw tADDi8 killed renamable $r0, 2, 14 /* CC::al */, $noreg
+    frame-destroy tPOP_RET 14 /* CC::al */, $noreg, def $r4, def $r5, def $r6, def $pc, implicit killed $r0
+
+  bb.1:
+    liveins: $lr, $r6, $r5, $r4
+
+    DBG_VALUE $r3, $noreg
+    DBG_VALUE $r2, $noreg
+    DBG_VALUE $r1, $noreg
+    DBG_VALUE $r0, $noreg
+    renamable $r0 = t2MOVi -1, 14 /* CC::al */, $noreg, $noreg
+    DBG_VALUE $r0, $noreg
+    tBX_RET 14 /* CC::al */, $noreg, implicit killed $r0
+...
+---
+name:            g
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    successors: %bb.1(0x30000000), %bb.2(0x50000000)
+    liveins: $r0, $r1, $r2, $r3, $lr, $r6, $r5, $r4
+
+    DBG_VALUE $r0, $noreg
+    DBG_VALUE $r1, $noreg
+    DBG_VALUE $r2, $noreg
+    DBG_VALUE $r3, $noreg
+    tCMPi8 renamable $r0, 0, 14 /* CC::al */, $noreg, implicit-def $cpsr
+    t2Bcc %bb.1, 4 /* CC::mi */, killed $cpsr
+
+  bb.2.if.end:
+    liveins: $r0, $r1, $r2, $r3, $r4, $r5, $r6, $lr
+
+    DBG_VALUE $r3, $noreg
+    DBG_VALUE $r2, $noreg
+    DBG_VALUE $r1, $noreg
+    DBG_VALUE $r0, $noreg
+    frame-setup tPUSH 14 /* CC::al */, $noreg, killed $r4, killed $r5, killed $r6, killed $lr, implicit-def $sp, implicit $sp
+    frame-setup CFI_INSTRUCTION def_cfa_offset 16
+    frame-setup CFI_INSTRUCTION offset $lr, -4
+    frame-setup CFI_INSTRUCTION offset $r6, -8
+    frame-setup CFI_INSTRUCTION offset $r5, -12
+    frame-setup CFI_INSTRUCTION offset $r4, -16
+    $r6 = tMOVr killed $r1, 14 /* CC::al */, $noreg
+    DBG_VALUE $r6, $noreg
+    renamable $r1, dead $cpsr = tMOVi8 11, 14 /* CC::al */, $noreg
+    renamable $r0, dead $cpsr = nsw tMUL killed renamable $r1, killed renamable $r0, 14 /* CC::al */, $noreg
+    DBG_VALUE $r0, $noreg
+    $r1 = tMOVr $r6, 14 /* CC::al */, $noreg
+    $r4 = tMOVr killed $r3, 14 /* CC::al */, $noreg
+    DBG_VALUE $r4, $noreg
+    $r5 = tMOVr killed $r2, 14 /* CC::al */, $noreg
+    DBG_VALUE $r5, $noreg
+    renamable $r0, dead $cpsr = nsw tSUBrr killed renamable $r0, renamable $r6, 14 /* CC::al */, $noreg
+    tBL 14 /* CC::al */, $noreg, @h, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit killed $r1, implicit-def $sp, implicit-def $r0
+    DBG_VALUE $r0, $noreg
+    renamable $r1, dead $cpsr = nsw tADDrr renamable $r0, killed renamable $r6, 14 /* CC::al */, $noreg
+    renamable $r0, dead $cpsr = nsw tMUL killed renamable $r1, killed renamable $r0, 14 /* CC::al */, $noreg
+    DBG_VALUE $r0, $noreg
+    renamable $r1, dead $cpsr = nsw tADDrr killed renamable $r4, killed renamable $r5, 14 /* CC::al */, $noreg
+    renamable $r0 = t2SDIV killed renamable $r0, killed renamable $r1, 14 /* CC::al */, $noreg
+    renamable $r0, dead $cpsr = nsw tADDi8 killed renamable $r0, 1, 14 /* CC::al */, $noreg
+    frame-destroy tPOP_RET 14 /* CC::al */, $noreg, def $r4, def $r5, def $r6, def $pc, implicit killed $r0
+
+  bb.1:
+    liveins: $lr, $r6, $r5, $r4
+
+    DBG_VALUE $r3, $noreg
+    DBG_VALUE $r2, $noreg
+    DBG_VALUE $r1, $noreg
+    DBG_VALUE $r0, $noreg
+    renamable $r0 = t2MOVi -1, 14 /* CC::al */, $noreg, $noreg
+    DBG_VALUE $r0, $noreg
+    tBX_RET 14 /* CC::al */, $noreg, implicit killed $r0
+
+...