[PATCH] D77576: [globalisel] Add lost debug locations verifier

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LostDebugLocObserver.cpp:43
+    if (MI->getDebugLoc().getLine() == 0)
+      HasUnseenLineZeroLoc = true;
+  }
----------------
Why not delete HasUnseenLineZeroLoc and return early here?


================
Comment at: llvm/lib/CodeGen/GlobalISel/LostDebugLocObserver.cpp:83
+  if (MI.getOpcode() != TargetOpcode::G_CONSTANT &&
+      MI.getOpcode() != TargetOpcode::G_FCONSTANT) {
+    PotentialMIsForDebugLocs.erase(&MI);
----------------
Please have this return early, ideally like 'if (MI.isConstant()) return;'

I don't mean to suggest that you make tangential changes, it's just that it seems like there's a need for such a predicate (e.g. https://llvm.org/doxygen/AMDGPUInstructionSelector_8cpp_source.html#l01470 defines isConstant for MIs but leaves out G_FCONSTANT).


================
Comment at: llvm/lib/CodeGen/GlobalISel/LostDebugLocObserver.cpp:91
+void LostDebugLocObserver::changingInstr(MachineInstr &MI) {
+  if (MI.getOpcode() != TargetOpcode::G_CONSTANT &&
+      MI.getOpcode() != TargetOpcode::G_FCONSTANT) {
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77576