[CSInfo][MIPS] Update CSInfo during MipsDelaySlotFiller

[ntesic]

Summary

In MipsDelaySlotFiller, when replacing old call-branch with the compact branch
instruction, an assertion is caused by erasing the old call with unhandled CSInfo.
The problem was reported in Bug 48695.
This patch fixes it, by moving call site info from the old call instruction to its replace.

[nickdesaulniers]

Patch fixes the reported bug; added test cases produces a crash before patch is applied. LGTM with minor nit.

llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp

568–572

Do you want to use

ToErase->eraseFromParent();

here?

[atanasyan]

LGTM

[nathanchance]

This fixes 32r6{,el}_defconfig and does not regress any of my other MIPS kernel builds.

[djtodoro]

some comments inline, lgtm otherwise

llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp

568–572

+1

570

Should we use the shouldUpdateCallSiteInfo() predicate instead?

llvm/test/DebugInfo/MIR/Mips/call-site-info-update-delay-slot-filler.mir

34

Do we need all these attributes?

[ntesic]

Addressed comments:

• Use shouldUpdateCallSiteInfo() predicate
• Use ToErase->eraseFromParent()
• Reduce function attributes in test

@nickdesaulniers @djtodoro I agree with your suggestions.
@atanasyan @nathanchance Thank you all for your comments!

[atanasyan]

@ntesic Do you have commit access?

[ntesic]

In D94685#2504510, @atanasyan wrote:

@ntesic Do you have commit access?

My colleague will commit this. Thanks!

llvm/lib/Target/Mips/MipsDelaySlotFiller.cpp

Iter MipsDelaySlotFiller::replaceWithCompactBranch(MachineBasicBlock &MBB,
                                                    Iter Branch,
                                                    const DebugLoc &DL) {
   const MipsSubtarget &STI = MBB.getParent()->getSubtarget<MipsSubtarget>();
   const MipsInstrInfo *TII = STI.getInstrInfo();
 
   unsigned NewOpcode = TII->getEquivalentCompactForm(Branch);
   Branch = TII->genInstrWithNewOpc(NewOpcode, Branch);
 
-  std::next(Branch)->eraseFromParent();
+  auto *ToErase = cast<MachineInstr>(&*std::next(Branch));
+  // Update call site info for the Branch.
+  if (ToErase->shouldUpdateCallSiteInfo())
+    ToErase->getMF()->moveCallSiteInfo(ToErase, cast<MachineInstr>(&*Branch));
+  ToErase->eraseFromParent();
   return Branch;
 }
 
 // For given opcode returns opcode of corresponding instruction with short
 // delay slot.
 // For the pseudo TAILCALL*_MM instructions return the short delay slot
 // form. Unfortunately, TAILCALL<->b16 is denied as b16 has a limited range
 // that is too short to make use of for tail calls.

llvm/test/DebugInfo/MIR/Mips/call-site-info-update-delay-slot-filler.mir

+## Test mips32r6:
+# RUN: llc -mtriple mips-linux-gnu -emit-call-site-info -run-pass=mips-delay-slot-filler %s -o -| FileCheck %s
+
+## Following code is used for producing this test case.
+##
+## (*pm_power_off)();
+## machine_power_off() { pm_power_off(); }
+##
+## Used command:
+## clang --target=mips-linux-gnu -march=mips32r6 -g -O2 -c m.c -mllvm -stop-before=mips-delay-slot-filler
+
+## Confirm that call site info exists after mips-delay-slot-filler, although, 
+## the pass replaced the call instruction.
+# CHECK: callSites:
+# CHECK-NEXT: - { bb: {{.*}}, offset: {{.*}}, fwdArgRegs:
+
+--- |
+  ; ModuleID = 'm.c'
+  source_filename = "m.c"
+  target datalayout = "E-m:m-p:32:32-i8:8:32-i16:16:32-i64:64-n32-S64"
+  target triple = "mips-unknown-linux-gnu"
+  
+  @pm_power_off = dso_local local_unnamed_addr global i32 (...)* null, align 4, !dbg !0
+  
+  ; Function Attrs: nounwind
+  define dso_local i32 @machine_power_off() local_unnamed_addr #0 !dbg !14 {
+  entry:
+    %0 = load i32 ()*, i32 ()** bitcast (i32 (...)** @pm_power_off to i32 ()**), align 4, !dbg !17
+    %call = tail call i32 %0(), !dbg !17
+    ret i32 undef, !dbg !17
+  }
+  
+  attributes #0 = { "target-cpu"="mips32r6" }
+  
+  !llvm.dbg.cu = !{!2}
+  !llvm.module.flags = !{!10, !11, !12}
+  !llvm.ident = !{!13}
+  
+  !0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+  !1 = distinct !DIGlobalVariable(name: "pm_power_off", scope: !2, file: !3, line: 1, type: !6, isLocal: false, isDefinition: true)
+  !2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 12.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+  !3 = !DIFile(filename: "m.c", directory: "/dir")
+  !4 = !{}
+  !5 = !{!0}
+  !6 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 32)
+  !7 = !DISubroutineType(types: !8)
+  !8 = !{!9, null}
+  !9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !10 = !{i32 7, !"Dwarf Version", i32 4}
+  !11 = !{i32 2, !"Debug Info Version", i32 3}
+  !12 = !{i32 1, !"wchar_size", i32 4}
+  !13 = !{!"clang version 12.0.0"}
+  !14 = distinct !DISubprogram(name: "machine_power_off", scope: !3, file: !3, line: 2, type: !15, scopeLine: 2, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !4)
+  !15 = !DISubroutineType(types: !16)
+  !16 = !{!9}
+  !17 = !DILocation(line: 2, column: 23, scope: !14)
+
+...
+---
+name:            machine_power_off
+alignment:       4
+stack:
+  - { id: 0, name: '', type: spill-slot, offset: -4, size: 4, alignment: 4, 
+      stack-id: default, callee-saved-register: '$ra', callee-saved-restored: true, 
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+callSites:
+  - { bb: 0, offset: 6, fwdArgRegs: [] }
+body:             |
+  bb.0.entry:
+    liveins: $ra
+  
+    $sp = ADDiu $sp, -24
+    CFI_INSTRUCTION def_cfa_offset 24
+    SW killed $ra, $sp, 20 :: (store 4 into %stack.0)
+    CFI_INSTRUCTION offset $ra_64, -4
+    renamable $at = LUi target-flags(mips-abs-hi) @pm_power_off, debug-location !17
+    renamable $t9 = LW killed renamable $at, target-flags(mips-abs-lo) @pm_power_off, debug-location !17 :: (dereferenceable load 4 from `i32 ()** bitcast (i32 (...)** @pm_power_off to i32 ()**)`)
+    JALRPseudo $t9, csr_o32_fp64, implicit-def dead $ra, implicit-def $sp, implicit-def dead $v0, debug-location !17
+    $ra = LW $sp, 20, debug-location !17 :: (load 4 from %stack.0)
+    $sp = ADDiu $sp, 24
+    PseudoReturn undef $ra, implicit undef $v0, debug-location !17
+
+...