[PATCH] D63429: [DebugInfo] Avoid adding too much indirection to pointer-valued variables

[Jeremy Morse via Phabricator via llvm-commits]

jmorse marked an inline comment as done.
jmorse added inline comments.


================
Comment at: lib/CodeGen/PrologEpilogInserter.cpp:1191
+        unsigned PrependFlags = DIExpression::ApplyOffset;
+        if (!MI.isIndirectDebugValue() && DIExpr->getNumElements() == 0)
+          PrependFlags |= DIExpression::StackValue;
----------------
jmorse wrote:
> aprantl wrote:
> > The only thing I'm concerned about is the `DIExpr->getNumElements() == 0`. This sounds like it would also reject other cases where we'd want this to apply, such as an expression that only contains a `DW_OP_LLVM_fragment`. Is there a better condition? Perhaps by inverting it? What's an example of an expression where we don't want this to happen?
> Oooofff, indeed, that definitely would cause problems. The current test that DwarfExpression makes [0] to see whether something is a simple register location or not, is whether the expression is:
>  * Empty, or
>  * Only a fragment
> 
> Anything else involves computing values on the expression stack: that makes it a memory location unless DW_OP_stack_value is added. I've added a method to perform that check in isComplex, and use that in PrologEpilog. I can't replace the DwarfExpression test with that method unfortunately, as it's passing around expr_operand iterators rather than DIExpressions.
> 
> [0] https://github.com/llvm/llvm-project/blob/fb9ce100d19be130d004d03088ccd4af295f3435/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp#L229
Hmmn, isComplex will also have to account for DW_OP_LLVM_tag_offset that's just been added. I'll need some time to pull that in and rebuild, will have that patch up shortly.


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

https://reviews.llvm.org/D63429