[DebugInfo][If-Converter] Update call site info during the optimization

During the If-Converter optimization pay attention when copying or
deleting call instructions in order to keep call site information in
valid state.

Reviewers: aprantl, vsk, efriedma

Reviewed By: vsk, efriedma

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

llvm-svn: 374068

llvm/include/llvm/CodeGen/MachineFunction.h

@@ -36,6 +36,7 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Recycler.h"
+#include "llvm/Target/TargetMachine.h"
 #include <cassert>
 #include <cstdint>
 #include <memory>
@@ -400,6 +401,17 @@ private:
   /// Map a call instruction to call site arguments forwarding info.
   CallSiteInfoMap CallSitesInfo;
 
+  /// A helper function that returns call site info for a give call
+  /// instruction if debug entry value support is enabled.
+  CallSiteInfoMap::iterator getCallSiteInfo(const MachineInstr *MI) {
+    assert(MI->isCall() &&
+           "Call site info refers only to call instructions!");
+
+    if (!Target.Options.EnableDebugEntryValues)
+      return CallSitesInfo.end();
+    return CallSitesInfo.find(MI);
+  }
+
   // Callbacks for insertion and removal.
   void handleInsertion(MachineInstr &MI);
   void handleRemoval(MachineInstr &MI);
@@ -977,12 +989,24 @@ public:
     return CallSitesInfo;
   }
 
-  /// Update call sites info by deleting entry for \p Old call instruction.
-  /// If \p New is present then transfer \p Old call info to it. This function
-  /// should be called before removing call instruction or before replacing
-  /// call instruction with new one.
-  void updateCallSiteInfo(const MachineInstr *Old,
-                          const MachineInstr *New = nullptr);
+  /// Following functions update call site info. They should be called before
+  /// removing, replacing or copying call instruction.
+
+  /// Move the call site info from \p Old to \New call site info. This function
+  /// is used when we are replacing one call instruction with another one to
+  /// the same callee.
+  void moveCallSiteInfo(const MachineInstr *Old,
+                        const MachineInstr *New);
+
+  /// Erase the call site info for \p MI. It is used to remove a call
+  /// instruction from the instruction stream.
+  void eraseCallSiteInfo(const MachineInstr *MI);
+
+  /// Copy the call site info from \p Old to \ New. Its usage is when we are
+  /// making a copy of the instruction that will be inserted at different point
+  /// of the instruction stream.
+  void copyCallSiteInfo(const MachineInstr *Old,
+                        const MachineInstr *New);
 };
 
 //===--------------------------------------------------------------------===//

llvm/lib/CodeGen/BranchFolding.cpp

@@ -162,6 +162,11 @@ void BranchFolder::RemoveDeadBlock(MachineBasicBlock *MBB) {
   // Avoid matching if this pointer gets reused.
   TriedMerging.erase(MBB);
 
+  // Update call site info.
+  std::for_each(MBB->begin(), MBB->end(), [MF](const MachineInstr &MI) {
+    if (MI.isCall(MachineInstr::IgnoreBundle))
+      MF->eraseCallSiteInfo(&MI);
+  });
   // Remove the block.
   MF->erase(MBB);
   EHScopeMembership.erase(MBB);

llvm/lib/CodeGen/IfConversion.cpp

@@ -1743,6 +1743,11 @@ bool IfConverter::IfConvertDiamondCommon(
       ++i;
   }
   while (NumDups1 != 0) {
+    // Since this instruction is going to be deleted, update call
+    // site info state if the instruction is call instruction.
+    if (DI2->isCall(MachineInstr::IgnoreBundle))
+      MBB2.getParent()->eraseCallSiteInfo(&*DI2);
+
     ++DI2;
     if (DI2 == MBB2.end())
       break;
@@ -1784,7 +1789,14 @@ bool IfConverter::IfConvertDiamondCommon(
     // NumDups2 only counted non-dbg_value instructions, so this won't
     // run off the head of the list.
     assert(DI1 != MBB1.begin());
+
     --DI1;
+
+    // Since this instruction is going to be deleted, update call
+    // site info state if the instruction is call instruction.
+    if (DI1->isCall(MachineInstr::IgnoreBundle))
+      MBB1.getParent()->eraseCallSiteInfo(&*DI1);
+
     // skip dbg_value instructions
     if (!DI1->isDebugInstr())
       ++i;
@@ -2069,6 +2081,10 @@ void IfConverter::CopyAndPredicateBlock(BBInfo &ToBBI, BBInfo &FromBBI,
       break;
 
     MachineInstr *MI = MF.CloneMachineInstr(&I);
+    // Make a copy of the call site info.
+    if (MI->isCall(MachineInstr::IgnoreBundle))
+      MF.copyCallSiteInfo(&I,MI);
+
     ToBBI.BB->insert(ToBBI.BB->end(), MI);
     ToBBI.NonPredSize++;
     unsigned ExtraPredCost = TII->getPredicationCost(I);

llvm/lib/CodeGen/InlineSpiller.cpp

@@ -867,7 +867,7 @@ foldMemoryOperand(ArrayRef<std::pair<MachineInstr *, unsigned>> Ops,
     --NumSpills;
   LIS.ReplaceMachineInstrInMaps(*MI, *FoldMI);
   if (MI->isCall())
-    MI->getMF()->updateCallSiteInfo(MI, FoldMI);
+    MI->getMF()->moveCallSiteInfo(MI, FoldMI);
   MI->eraseFromParent();
 
   // Insert any new instructions other than FoldMI into the LIS maps.

llvm/lib/CodeGen/LiveRangeEdit.cpp

@@ -232,7 +232,7 @@ bool LiveRangeEdit::foldAsLoad(LiveInterval *LI,
   LLVM_DEBUG(dbgs() << "                folded: " << *FoldMI);
   LIS.ReplaceMachineInstrInMaps(*UseMI, *FoldMI);
   if (UseMI->isCall())
-    UseMI->getMF()->updateCallSiteInfo(UseMI, FoldMI);
+    UseMI->getMF()->moveCallSiteInfo(UseMI, FoldMI);
   UseMI->eraseFromParent();
   DefMI->addRegisterDead(LI->reg, nullptr);
   Dead.push_back(DefMI);

llvm/lib/CodeGen/MachineFunction.cpp

@@ -835,20 +835,36 @@ void MachineFunction::addCodeViewHeapAllocSite(MachineInstr *I,
   CodeViewHeapAllocSites.push_back(std::make_tuple(BeginLabel, EndLabel, DI));
 }
 
-void MachineFunction::updateCallSiteInfo(const MachineInstr *Old,
-                                         const MachineInstr *New) {
-  if (!Target.Options.EnableDebugEntryValues || Old == New)
-    return;
+void MachineFunction::moveCallSiteInfo(const MachineInstr *Old,
+                                       const MachineInstr *New) {
+  assert(New->isCall() && "Call site info refers only to call instructions!");
 
-  assert(Old->isCall() && (!New || New->isCall()) &&
-         "Call site info referes only to call instructions!");
-  CallSiteInfoMap::iterator CSIt = CallSitesInfo.find(Old);
+  CallSiteInfoMap::iterator CSIt = getCallSiteInfo(Old);
   if (CSIt == CallSitesInfo.end())
     return;
+
   CallSiteInfo CSInfo = std::move(CSIt->second);
   CallSitesInfo.erase(CSIt);
-  if (New)
-    CallSitesInfo[New] = CSInfo;
+  CallSitesInfo[New] = CSInfo;
+}
+
+void MachineFunction::eraseCallSiteInfo(const MachineInstr *MI) {
+  CallSiteInfoMap::iterator CSIt = getCallSiteInfo(MI);
+  if (CSIt == CallSitesInfo.end())
+    return;
+  CallSitesInfo.erase(CSIt);
+}
+
+void MachineFunction::copyCallSiteInfo(const MachineInstr *Old,
+                                       const MachineInstr *New) {
+  assert(New->isCall() && "Call site info refers only to call instructions!");
+
+  CallSiteInfoMap::iterator CSIt = getCallSiteInfo(Old);
+  if (CSIt == CallSitesInfo.end())
+    return;
+
+  CallSiteInfo CSInfo = CSIt->second;
+  CallSitesInfo[New] = CSInfo;
 }
 
 /// \}

llvm/lib/CodeGen/MachineOutliner.cpp

@@ -1260,7 +1260,7 @@ bool MachineOutliner::outline(Module &M,
                   true /* isImp = true */));
           }
           if (MI.isCall())
-            MI.getMF()->updateCallSiteInfo(&MI);
+            MI.getMF()->eraseCallSiteInfo(&MI);
         };
         // Copy over the defs in the outlined range.
         // First inst in outlined range <-- Anything that's defined in this

llvm/lib/CodeGen/PeepholeOptimizer.cpp

@@ -1776,7 +1776,7 @@ bool PeepholeOptimizer::runOnMachineFunction(MachineFunction &MF) {
               LocalMIs.erase(DefMI);
               LocalMIs.insert(FoldMI);
               if (MI->isCall())
-                MI->getMF()->updateCallSiteInfo(MI, FoldMI);
+                MI->getMF()->moveCallSiteInfo(MI, FoldMI);
               MI->eraseFromParent();
               DefMI->eraseFromParent();
               MRI->markUsesInDebugValueAsUndef(FoldedReg);

llvm/lib/CodeGen/TargetInstrInfo.cpp

@@ -143,7 +143,7 @@ TargetInstrInfo::ReplaceTailWithBranchTo(MachineBasicBlock::iterator Tail,
   while (Tail != MBB->end()) {
     auto MI = Tail++;
     if (MI->isCall())
-      MBB->getParent()->updateCallSiteInfo(&*MI);
+      MBB->getParent()->eraseCallSiteInfo(&*MI);
     MBB->erase(MI);
   }
 

llvm/lib/CodeGen/UnreachableBlockElim.cpp

@@ -151,7 +151,7 @@ bool UnreachableMachineBlockElim::runOnMachineFunction(MachineFunction &F) {
     // Remove any call site information for calls in the block.
     for (auto &I : DeadBlocks[i]->instrs())
       if (I.isCall(MachineInstr::IgnoreBundle))
-        DeadBlocks[i]->getParent()->updateCallSiteInfo(&I);
+        DeadBlocks[i]->getParent()->eraseCallSiteInfo(&I);
 
     DeadBlocks[i]->eraseFromParent();
   }

llvm/lib/CodeGen/XRayInstrumentation.cpp

@@ -111,7 +111,7 @@ void XRayInstrumentation::replaceRetWithPatchableRet(
           MIB.add(MO);
         Terminators.push_back(&T);
         if (T.isCall())
-          MF.updateCallSiteInfo(&T);
+          MF.eraseCallSiteInfo(&T);
       }
     }
   }

llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp

@@ -1207,7 +1207,7 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
 
 
       // Update call site info and delete the pseudo instruction TCRETURN.
-      MBB.getParent()->updateCallSiteInfo(&MI, &*NewMI);
+      MBB.getParent()->moveCallSiteInfo(&MI, &*NewMI);
       MBB.erase(MBBI);
 
       MBBI = NewMI;
@@ -1439,7 +1439,7 @@ bool ARMExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
 
       MIB.cloneMemRefs(MI);
       TransferImpOps(MI, MIB, MIB);
-      MI.getMF()->updateCallSiteInfo(&MI, &*MIB);
+      MI.getMF()->moveCallSiteInfo(&MI, &*MIB);
       MI.eraseFromParent();
       return true;
     }

llvm/lib/Target/X86/X86ExpandPseudo.cpp

@@ -275,7 +275,7 @@ bool X86ExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
 
     MachineInstr &NewMI = *std::prev(MBBI);
     NewMI.copyImplicitOps(*MBBI->getParent()->getParent(), *MBBI);
-    MBB.getParent()->updateCallSiteInfo(&*MBBI, &NewMI);
+    MBB.getParent()->moveCallSiteInfo(&*MBBI, &NewMI);
 
     // Delete the pseudo instruction TCRETURN.
     MBB.erase(MBBI);

llvm/test/CodeGen/ARM/smml.ll

@@ -8,6 +8,13 @@
 ; RUN: llc -mtriple=thumbv7m-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-V4
 ; RUN: llc -mtriple=thumbv7em-eabi %s -o - | FileCheck %s -check-prefix=CHECK -check-prefix=CHECK-THUMBV6T2
 
+; Next test would previously trigger an assertion responsible for verification of
+; call site info state.
+; RUN: llc -stop-after=if-converter -debug-entry-values -mtriple=thumbv6t2-eabi %s -o -| FileCheck %s -check-prefix=CHECK-CALLSITE
+; CHECK-CALLSITE: name:  test_used_flags
+; CHECK-CALLSITE: callSites:
+
+
 define i32 @Test0(i32 %a, i32 %b, i32 %c) nounwind readnone ssp {
 entry:
 ; CHECK-LABEL: Test0

llvm/test/DebugInfo/MIR/ARM/if-coverter-call-site-info.mir

@@ -0,0 +1,165 @@
+# RUN: llc -mtriple=arm-linux-gnu -debug-entry-values -run-pass if-converter %s -o -| FileCheck %s
+
+# Vefify that the call site info will be updated after the optimization.
+# This test case would previously trigger an assertion when
+# deleting the call instruction.
+
+# Test case is generated from:
+# extern void
+# foo (int* seg, int subseg);
+# extern int* mri_common_symbol;
+#
+# void
+# baa (int* secptr, int subseg)
+# {
+#   if (! (secptr == 0 && subseg == 0))
+#     foo (secptr, subseg);
+#   mri_common_symbol = 0;
+# }
+#
+# With slight change of MIR - substitution of BL instruction with BL_pred 
+# in order to trigger optimization.
+# clang -target arm-linux-gnu -g -O2 -Xclang -femit-debug-entry-values 
+#       %s -stop-before=if-convert
+#
+# CHECK: callSites:
+# CHECK-NEXT: - { bb: {{.*}}, offset: {{.*}}, fwdArgRegs:
+# CHECK-NEXT:     - { arg: 0, reg: '$r0' }
+# CHECK-NEXT:     - { arg: 1, reg: '$r1' } }
+
+--- |
+  ; ModuleID = 'if-convert-call-site-info.c'
+  source_filename = "if-convert-call-site-info.c"
+  target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+  target triple = "armv6kz-unknown-linux-gnueabihf"
+  
+  @mri_common_symbol = external dso_local local_unnamed_addr global i32*, align 4
+  
+  ; Function Attrs: nounwind
+  define dso_local void @baa(i32* %secptr, i32 %subseg) local_unnamed_addr #0 !dbg !14 {
+  entry:
+    call void @llvm.dbg.value(metadata i32* %secptr, metadata !16, metadata !DIExpression()), !dbg !18
+    call void @llvm.dbg.value(metadata i32 %subseg, metadata !17, metadata !DIExpression()), !dbg !18
+    %cmp = icmp eq i32* %secptr, null, !dbg !19
+    %cmp1 = icmp eq i32 %subseg, 0, !dbg !21
+    %or.cond = and i1 %cmp, %cmp1, !dbg !22
+    br i1 %or.cond, label %if.end, label %if.then, !dbg !22
+  
+  if.then:                                          ; preds = %entry
+    tail call void @foo(i32* %secptr, i32 %subseg), !dbg !23
+    br label %if.end, !dbg !23
+  
+  if.end:                                           ; preds = %entry, %if.then
+    store i32* null, i32** @mri_common_symbol, align 4, !dbg !24, !tbaa !25
+    ret void, !dbg !29
+  }
+  
+  declare !dbg !4 dso_local void @foo(i32*, i32) local_unnamed_addr
+  
+  ; Function Attrs: nounwind readnone speculatable willreturn
+  declare void @llvm.dbg.value(metadata, metadata, metadata)
+  
+  ; Function Attrs: nounwind
+  declare void @llvm.stackprotector(i8*, i8**)
+  
+  attributes #0 = { "frame-pointer"="all" }
+  
+  !llvm.dbg.cu = !{!0}
+  !llvm.module.flags = !{!9, !10, !11, !12}
+  !llvm.ident = !{!13}
+  
+  !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 10.0.0 ", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None)
+  !1 = !DIFile(filename: "if-convert-call-site-info.c", directory: "/")
+  !2 = !{}
+  !3 = !{!4}
+  !4 = !DISubprogram(name: "foo", scope: !1, file: !1, line: 10, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2)
+  !5 = !DISubroutineType(types: !6)
+  !6 = !{null, !7, !8}
+  !7 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !8, size: 32)
+  !8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+  !9 = !{i32 2, !"Dwarf Version", i32 4}
+  !10 = !{i32 2, !"Debug Info Version", i32 3}
+  !11 = !{i32 1, !"wchar_size", i32 4}
+  !12 = !{i32 1, !"min_enum_size", i32 4}
+  !13 = !{!"clang version 10.0.0 "}
+  !14 = distinct !DISubprogram(name: "baa", scope: !1, file: !1, line: 14, type: !5, scopeLine: 15, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !15)
+  !15 = !{!16, !17}
+  !16 = !DILocalVariable(name: "secptr", arg: 1, scope: !14, file: !1, line: 14, type: !7, flags: DIFlagArgumentNotModified)
+  !17 = !DILocalVariable(name: "subseg", arg: 2, scope: !14, file: !1, line: 14, type: !8, flags: DIFlagArgumentNotModified)
+  !18 = !DILocation(line: 0, scope: !14)
+  !19 = !DILocation(line: 16, column: 17, scope: !20)
+  !20 = distinct !DILexicalBlock(scope: !14, file: !1, line: 16, column: 7)
+  !21 = !DILocation(line: 16, column: 32, scope: !20)
+  !22 = !DILocation(line: 16, column: 22, scope: !20)
+  !23 = !DILocation(line: 17, column: 5, scope: !20)
+  !24 = !DILocation(line: 18, column: 21, scope: !14)
+  !25 = !{!26, !26, i64 0}
+  !26 = !{!"any pointer", !27, i64 0}
+  !27 = !{!"omnipotent char", !28, i64 0}
+  !28 = !{!"Simple C/C++ TBAA"}
+  !29 = !DILocation(line: 19, column: 1, scope: !14)
+
+...
+---
+name:            baa
+alignment:       2
+tracksRegLiveness: true
+liveins:
+  - { reg: '$r0' }
+  - { reg: '$r1' }
+frameInfo:
+  stackSize:       8
+  maxAlignment:    4
+  adjustsStack:    true
+  hasCalls:        true
+  maxCallFrameSize: 0
+stack:
+  - { id: 0, type: spill-slot, offset: -4, size: 4, alignment: 4, callee-saved-register: '$lr', 
+      callee-saved-restored: false }
+  - { id: 1, type: spill-slot, offset: -8, size: 4, alignment: 4, callee-saved-register: '$r11' }
+callSites:
+  - { bb: 2, offset: 0, fwdArgRegs: 
+      - { arg: 0, reg: '$r0' }
+      - { arg: 1, reg: '$r1' } }
+constants:
+  - id:              0
+    value:           'i32** null'
+    alignment:       4
+machineFunctionInfo: {}
+body:             |
+  bb.0.entry:
+    successors: %bb.1(0x60000000), %bb.2(0x20000000)
+    liveins: $r0, $r1, $lr
+  
+    DBG_VALUE $r0, $noreg, !16, !DIExpression(), debug-location !18
+    DBG_VALUE $r0, $noreg, !16, !DIExpression(), debug-location !18
+    DBG_VALUE $r1, $noreg, !17, !DIExpression(), debug-location !18
+    DBG_VALUE $r1, $noreg, !17, !DIExpression(), debug-location !18
+    $sp = frame-setup STMDB_UPD $sp, 14, $noreg, killed $r11, killed $lr
+    frame-setup CFI_INSTRUCTION def_cfa_offset 8
+    frame-setup CFI_INSTRUCTION offset $lr, -4
+    frame-setup CFI_INSTRUCTION offset $r11, -8
+    $r11 = frame-setup MOVr killed $sp, 14, $noreg, $noreg
+    frame-setup CFI_INSTRUCTION def_cfa_register $r11
+    CMPri renamable $r0, 0, 14, $noreg, implicit-def $cpsr, debug-location !22
+    Bcc %bb.2, 1, killed $cpsr, debug-location !22
+  
+  bb.1.entry:
+    successors: %bb.3(0x55555555), %bb.2(0x2aaaaaab)
+    liveins: $r0, $r1
+  
+    CMPri renamable $r1, 0, 14, $noreg, implicit-def $cpsr, debug-location !22
+    Bcc %bb.3, 0, killed $cpsr, debug-location !22
+  
+  bb.2.if.then:
+    liveins: $r0, $r1
+  
+    BL_pred @foo, 14, $noreg, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit $r0, implicit $r1, implicit-def $sp, debug-location !23
+  
+  bb.3.if.end:
+    renamable $r0 = LDRi12 %const.0, 0, 14, $noreg, debug-location !24 :: (load 4 from constant-pool)
+    renamable $r1 = MOVi 0, 14, $noreg, $noreg
+    STRi12 killed renamable $r1, killed renamable $r0, 0, 14, $noreg, debug-location !24 :: (store 4 into @mri_common_symbol, !tbaa !25)
+    $sp = LDMIA_RET $sp, 14, $noreg, def $r11, def $pc, debug-location !29
+
+...