[llvm] [SPIRV] Emit NonSemantic DebugValue (PR #224158)

[Diego Novillo via llvm-commits]

================
@@ -1533,12 +1904,104 @@ SPIRVNonSemanticDebugHandler::resolveDebugLocTarget(const MachineInstr *MI) {
     // above skips it.
     MI = findAdjacentEmittedInstruction(MI, MAI, true);
     assert(MI && "Merge instruction must be followed by a terminator");
+    // The assert is compiled out in a release build, and returning MI there
+    // would give an engaged optional holding null.
+    if (!MI)
+      return std::nullopt;
   }
 
   return MI;
 }
 
-void SPIRVNonSemanticDebugHandler::emitDebugScopeForInstruction(
+void SPIRVNonSemanticDebugHandler::resolveDebugRecord(
+    const MachineInstr &MI, const MachineInstr *LastEmitted) {
+  SPIRV::ModuleAnalysisInfo &MAI = *CurrentMAI;
+  const MachineFunction &MF = *MI.getMF();
+
+  // A record sits at its own position, except in a merge region: nothing may
+  // be emitted between a merge and its terminator, so it belongs ahead of the
+  // merge.
+  const MachineInstr *Anchor =
+      LastEmitted && isMergeInstruction(LastEmitted->getOpcode()) ? LastEmitted
+                                                                  : &MI;
+
+  // A #dbg_declare reaches Machine IR as an indirect DBG_VALUE. Its storage is
+  // an OpVariable in the entry block, so it is in the output by the time any
+  // record naming it is reached.
+  if (std::optional<Register> LocReg = getDebugDeclareStorageReg(MI)) {
+    MCRegister StorageReg = MAI.getRegisterAlias(&MF, *LocReg);
+    if (StorageReg.isValid())
+      placeRecord(SPIRV::NonSemanticExtInst::DebugDeclare, MI, StorageReg,
+                  Anchor);
+    return;
+  }
+
+  // A constant assignment names the id emitted for it at module scope, which
+  // every function body follows.
+  if (std::optional<MCRegister> ConstReg = getConstantValueReg(MI)) {
+    placeRecord(SPIRV::NonSemanticExtInst::DebugValue, MI, *ConstReg, Anchor);
+    return;
+  }
+
+  std::optional<Register> ValReg = getDebugValueReg(MI);
+  if (!ValReg)
+    return;
+  MCRegister ValueReg = MAI.getRegisterAlias(&MF, *ValReg);
+  if (!ValueReg.isValid())
+    return;
+
+  // NonSemantic.Shader.DebugInfo states that "Forward references are not
----------------
dnovillo wrote:

> I was wondering whether a frontend could emit cases where the dominance check fails. I mean how "real" this scenario could be.

Not from the frontend, but optimization produces them. For example, code sinking can move the definition to a successor without moving the associated debug record. Nothing flags it, since the IR verifier only checks dominance for instruction operands and a debug record's value is not one.

> The cases that we reject due to the dominance check, are they cases of "stale" DI after optimisations?

Mostly not. `rewriteDebugUsers` salvages or kills a debug user not dominated by the replacement. This makes the salvager fall back to. The two I found are deliberate: the inliner remaps debug record operands late, and `MachineSink` forwards non-dominated debug users of a sunk `COPY`.

> If there are valid cases (no stale DI, and correct DI), what would require supporting them?

A definition that does not dominate the record means that the value only reaches it on some paths. Describing that needs a location that is valid over a range, which is what DWARF location lists do. NSDI has nothing equivalent: a `DebugValue` marks a point, with no way to say where it stops holding. Emitting one anyway would claim the value is live where it is not, so we need to drop the record in that case.


https://github.com/llvm/llvm-project/pull/224158