[PATCH] D74904: WIP: [CallSiteInfo] Erase call site info for a MI from bundle

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:158
+  if (MI->isCandidateForCallSiteEntry())
+    Parent->getParent()->eraseCallSiteInfo(MI);
   Parent->getParent()->DeleteMachineInstr(MI);
----------------
I don't think we should erase call site info in deleteNode. Part of the reason the assert in DeleteMachineInstr was added, IIUC, was to help identify the areas where llvm should move or copy call site info. I fear that if we change deleteNode like this, we'd lose the ability to do that.

Could we address the issue in P8199 by call eraseCallSiteInfo in TargetInstrInfo::ReplaceTailWithBranchTo instead?


================
Comment at: llvm/lib/CodeGen/MachineFunction.cpp:396
   // back trace will tell where to insert a call to updateCallSiteInfo().
-  assert((!MI->isCall(MachineInstr::IgnoreBundle) ||
+  assert((!MI->isCandidateForCallSiteEntry() ||
           CallSitesInfo.find(MI) == CallSitesInfo.end()) &&
----------------
This part lgtm.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74904/new/

https://reviews.llvm.org/D74904