[PATCH] D105657: [InstrRef][X86] Drop debug instruction numbers from x87 instructions

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added a comment.

In D105657#2887180 <https://reviews.llvm.org/D105657#2887180>, @jmorse wrote:

> (Possibly, this should end up on the mailing list),

+1

> In D105657#2886802 <https://reviews.llvm.org/D105657#2886802>, @djtodoro wrote:
>
>> Thanks for this. It looks reasonable, but I was wondering if there is an automatic way for ensuring that when deletion of an operand occurs, there is no dangling debug ref to it. It seems like a good candidate for an assertion somehow?
>
> That definitely makes sense for things like RemoveOperand -- it's going to cause all other operands to be re-numbered, thus breaking many connections between DBG_INSTR_REFs and the instruction. I'll try putting a dropDebugNumber into RemoveOperand and see if it affects variable coverage. (I think this patch will still be sufficient).

Thanks!

> NB: This particular patch was generated by an assertion firing, while building a large codebase InstrRefBasedLDV followed a chain of substitutions back to an operand that wasn't a register def, and asserted. This won't catch everything, but at least caught this.
>
>> I guess we should be aware that this feature will be used for some downstream targets as well, so we need to provide some kind of mechanism for them in order to keep things valid, any thoughts?
>
> Downstream targets are inevitably going to need some modifications to be correct, as with this patch; I should add something to SourceLevelDebugging.rst to document what the update rules are for debug-instr-numbers. I think the extra instrumentation is necessary to improve past what DBG_VALUEs can currently provide.
>
> Unfortunately I don't think there's any way of proving or ensuring the debug-info is always correct, because proving that two functions are identical is generally undecidable. There's always going to be a small chance of an un-instrumented optimisation out there subtly modifying an instruction in a way that makes it calculate a different value, and where debug-info should be dropped, but isn't. I think we have to accept this as a good trade-off in risk: right now, register coalescing causes large numbers of DBG_VALUEs to point at the wrong value, and there's no good way to fix it, wheras with debug-instr-number there's always a safe option of dropping.

I agree and good documentation is the right way to proceed with this.


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

https://reviews.llvm.org/D105657