[PATCH] D69889: [DebugInfo] Avoid creating entry values for clobbered registers

[David Stenberg via Phabricator via llvm-commits]

dstenb marked 2 inline comments as done.
dstenb added inline comments.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:1253
+/// Collect all register defines (including aliases) for the given instruction.
+static void collectRegDefs(const MachineInstr &MI, SmallSet<Register, 32> &Regs,
+                           const TargetRegisterInfo *TRI) {
----------------
aprantl wrote:
> I assume you checked that such a helper doesn't already exist?
Yes, I have not been able to find any functions that collects the defs in this way.


================
Comment at: llvm/lib/CodeGen/LiveDebugValues.cpp:1353
+    // entry value.
+    if (DefinedRegs.count(MI.getOperand(0).getReg()))
+      return false;
----------------
aprantl wrote:
> I'm assuming this is IsEntryValueCandidate()? There's no context..
> So we're saying any DBG_VALUE using a register that has been defined in the entry block is not an entry value candidate?
> I'm assuming this is IsEntryValueCandidate()? There's no context..

Sorry about not including the context! I have updated the diff.

> So we're saying any DBG_VALUE using a register that has been defined in the entry block is not an entry value candidate?

Yes, it will check any DBG_VALUE in the entry block. I think that "have we seen a preceding DBG_VALUE for this parameter which wasn't a viable candidate?" is a condition that is missing here (before this patch also), and in such cases we should not emit entry values for the parameter, Otherwise, I guess that we might emit incorrect entry values in cases like this:

```
void func(int foo, int bar);
```

```
entry:
DBG_VALUE $rdi, $noreg, "foo"
DBG_VALUE [some value other than a live-in register], "bar"
[instructions that do not clobber $rdi]
DBG_VALUE $rdi, $noreg, "bar"
```

I don't think such cases can happen now due to the `isNotModified()` guard, but as we're moving away from that, it's something to consider.

As a side note, I think it would be neat if there was some side-table mapping parameters to their corresponding registers, in the same as is done for the call sites with the `callSites` metadata, so that we would not have to find entry values by inspecting the entry block's debug values.


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

https://reviews.llvm.org/D69889