[PATCH] D150707: [DebugInfo] Handle undefined values for DBG_PHIs in InstrRef-LiveDebugValues

Wed May 17 03:57:38 PDT 2023

jmorse added a comment.

This works around the reported crash, which is great. However, there are going to be `ValueIDNum`s with slightly different interpretations as a result of the `DebugPHIRecord` not being updated. I don't *think* (90% sure) that's going to lead to incorrect variable locations, as the machine-location problem being solved just removes un-necessary PHIs. Thus, not updating the `ValueIDNum` which will have previously referred to a live-in value (aka a PHI):

- If the PHI is eliminated, the `ValueIDNum` will refer to a value that's never defined, therefore any variables using this value will be optimized out (which is fine for now),
- If the PHI is retained, then `ValueIDNum` will refer to it , and it will be correct (I think?).

Would you agree?

Could you stick a FIXME in the code-comment you've added indicating that we should find a better overall solution than just not updating the live-in `ValueIDNum`.

================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:3720
+    ValueIDNum ResolvedValue = MInLocs[BlockNo][LocNo.asU64()];
+    // If the resolved value for this Phi is the EmptyValue, then there is never
+    // a def for that location that reaches this block (likely due to this block
----------------
NB, PHI not Phi

================
Comment at: llvm/test/DebugInfo/X86/instr-ref-unreachable.mir:1-8
+# RUN: llc --run-pass=livedebugvalues %s -o - -experimental-debug-variable-locations | FileCheck %s
+
+# CHECK-DAG: ![[UNDEFVAR:[0-9]+]] = !DILocalVariable(name: "undef"
+# CHECK-DAG: ![[DEFVAR:[0-9]+]] = !DILocalVariable(name: "def"
+
+# CHECK-LABEL: bb.1.bb1
+# CHECK: DBG_VALUE_LIST ![[UNDEFVAR]], {{.+}}, $noreg
----------------
File-comment indicating what's the intention behind the test is please

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150707