[PATCH] D80665: [MachineLICM] Assert that locations from debug insts are not lost

[Vedant Kumar via Phabricator via llvm-commits]

vsk created this revision.
vsk added reviewers: aprantl, davide, chrisjackson, jmorse.
Herald added subscribers: asbirlea, hiraditya.
Herald added a project: LLVM.
davide added a comment.

As we discussed offline, I think the assertion makes sense, so LGTM. It's good to catch violations while we decide what to do with hoisting (what are the semantics et similia).
Do you think it's worth writing a verifier check to make sure that `DBG_VALUE`(s) when hoisted have always a location -- otherwise the MIR is considered illformed? I can do that, but I would like to have your opinion on it.


Assert that MachineLICM does not move a debug instruction and then drop
its debug location. Later passes require each debug instruction to have
a location.

Testing: check-llvm, clang stage2 RelWithDebInfo build (x86_64)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80665

Files:
  llvm/lib/CodeGen/MachineLICM.cpp


Index: llvm/lib/CodeGen/MachineLICM.cpp
===================================================================
--- llvm/lib/CodeGen/MachineLICM.cpp
+++ llvm/lib/CodeGen/MachineLICM.cpp
@@ -638,6 +638,7 @@
   // Since we are moving the instruction out of its basic block, we do not
   // retain its debug location. Doing so would degrade the debugging
   // experience and adversely affect the accuracy of profiling information.
+  assert(!MI->isDebugInstr() && "Cannot drop location from debug inst");
   MI->setDebugLoc(DebugLoc());
 
   // Add register to livein list to all the BBs in the current loop since a
@@ -841,6 +842,7 @@
 
     // The instruction is is moved from its basic block, so do not retain the
     // debug information.
+    assert(!I->isDebugInstr() && "Cannot drop location from debug inst");
     I->setDebugLoc(DebugLoc());
   }
 }
@@ -1536,6 +1538,7 @@
     // Since we are moving the instruction out of its basic block, we do not
     // retain its debug location. Doing so would degrade the debugging
     // experience and adversely affect the accuracy of profiling information.
+    assert(!MI->isDebugInstr() && "Cannot drop location from debug inst");
     MI->setDebugLoc(DebugLoc());
 
     // Update register pressure for BBs from header to this block.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D80665.266670.patch
Type: text/x-patch
Size: 1285 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200527/19d5e871/attachment-0001.bin>