[PATCH] D106915: Add a DIExpression const-folder to prevent silly expressions

[Paul Robinson via Phabricator via llvm-commits]

probinson marked 2 inline comments as done.
probinson added a comment.

In D106915#2910725 <https://reviews.llvm.org/D106915#2910725>, @StephenTozer wrote:

> In D106915#2910494 <https://reviews.llvm.org/D106915#2910494>, @probinson wrote:
>
>> Looking at the rest of the infrastructure, I think the Constant is not optional, the DIExpression is the optional part.  Most of the time there isn't an expression at all, of course, just the initial operand (which might or might not be a Constant).  I suppose that could be redesigned but I think it goes beyond the scope of this patch.
>
> I agree - currently we can't support this, but it can go on the list of potential improvements to DIExpressions.

Hmmm currently the folder works on a single known-constant operand; if/when the non-variadic form of the intrinsic/DBG_VALUE is fully replaced by variadic intrinsic/DBG_VALUE_LIST, the operand could become a one-element operand list and we prepend a DW_OP_LLVM_arg to the expression.  That would unify how it all looks, I expect.  At that point the constant-folder could be modified to take the original operand list and expression (without the caller vetting it as a single constant int) and the folder would do the vetting itself.  A minor problem there is that DIExpression (and DebugInfoMetadata.h in general) don't know about operand lists, which appear to be stored in different ways for the IR intrinsic and the SDAG/MachineInstr form.  That would have to be unified, presumably within DebugInfoMetadata.

It wouldn't be a stretch, at that point, to say that the folder could import constant operands directly into the expression, and remove them from the operand list.  Right now that responsibility lies with the DWARF emitter.  We'd also have to look at how the Codeview side works, and make sure we don't break that or make it unreasonably complicated.


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

https://reviews.llvm.org/D106915