[PATCH] D82363: [DebugInfo] Add new instruction and expression operator for variadic debug values

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added inline comments.


================
Comment at: llvm/docs/SourceLevelDebugging.rst:581
+
+The first form, ``DBG_VALUE``, appears thus:
 
----------------
djtodoro wrote:
> aprantl wrote:
> > Since we — contrary to LLVM bitcode — don't bother with compatibility for MIR — should we just always use DBG_VALUE_LIST instead or turn DBG_VALUE into DBG_VALUE_LIST to keep the code simpler?
> +1 for this, but do we want to keep the old one (DBG_VALUE) for the legacy (for some time) or it is too expensive?
This is being discussed on the mailing list right now, although the conversation somewhat died down without a firm resolution. I'm leaning towards removing the old form (it was suggested that replacing it with DBG_VALUE_OLD for a transition period before outright removal would be a good idea) and using DBG_VALUE_LIST everywhere.

The main point of discussion that needed resolution was how to represent the different location types with DBG_VALUE_LIST, since this current version only cares about stack values. My most recent proposal was the addition of a new expression operator, DW_OP_LLVM_direct, that could be used primarily to differentiate between the register location `DW_OP_reg0` and the memory location `DW_OP_breg0`, since this distinction doesn't exist in LLVM (we use the indirect flag to do so, which DBG_VALUE_LIST doesn't have).

I'm prepared to implement whatever solution we agree upon; and although we haven't got a firm agreement on any particular outcome, there seems to be more or less unanimous agreement that removing the old form is the right move. Given that we're keeping the behaviour of the old instruction and that we may want to keep it around in some legacy form for a time (DBG_VALUE_OLD?) as was suggested on the mailing list, it will probably be quite a small patch to implement the replacement. I'd also like that patch to be one of the last in this stack, so that we don't end up with a broken intermediate patch state (replacing the existing DBG_VALUE's uses before the new DBG_VALUE is fully functional).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82363