[PATCH] D78135: [MachineDebugify] Insert synthetic DBG_VALUE instructions

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

> There is essentially no attempt made to match up DBG_VALUE register
>  operands with the local variables they ought to correspond to. I'm not
>  sure how to improve the situation.

Yeah, and it gets harder the further down the pipeline you go. Especially since the pipeline is fairly free form and even early passes are allowed to select target instructions. I don't think it really matters too much if the IR-level dbg.value maps to the 'correct' MIR-level vreg as we're really looking for places the DBG_VALUE causes crashes or causes CodeGen changes. Eventually we might expand into checking that DBG_VALUE's are handled as opposed to accidentally lost. Even then though we're more concerned with preserving MIR level information rather than the actual mapping to IR-level



================
Comment at: llvm/lib/CodeGen/MachineDebugify.cpp:52-61
+  if (DbgValF) {
+    for (const Use &U : DbgValF->uses()) {
+      if (auto *DbgValInst = dyn_cast<DbgValueInst>(U.getUser())) {
+        Line2Var[DbgValInst->getDebugLoc().getLine()] =
+            DbgValInst->getVariable();
+        if (!Expr)
+          Expr = DbgValInst->getExpression();
----------------
I notice that this only generates enough information to cover the line numbers in the IR-level. That limits the vregs covered by DBG_VALUE's to the first few as there's typically more MIR instructions than IR instructions. Is it possible to cover more of the MIR vregs?


================
Comment at: llvm/test/CodeGen/Generic/MIRDebugify/locations.mir:1
-# RUN: llc -run-pass=mir-debugify -o - %s | FileCheck --check-prefixes=ALL,VALUE %s
+# RUN: llc -run-pass=mir-debugify -o - %s | FileCheck --check-prefixes=ALL,VALUE --implicit-check-not=DBG_VALUE %s
 # RUN: llc -run-pass=mir-debugify -debugify-level=locations -o - %s | FileCheck --check-prefixes=ALL --implicit-check-not=dbg.value %s
----------------
We ought to rename this test if it's going to cover the DBG_VALUE case too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78135