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

Jeremy Morse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 18 07:35:57 PDT 2019


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;
----------------
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


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

https://reviews.llvm.org/D63429





More information about the llvm-commits mailing list