[PATCH] D83560: [DebugInfo] Added support for DW_OP_implicit_value in llvm

Tue Jul 21 16:23:07 PDT 2020

aprantl added inline comments.

================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2492
     APInt RawBytes = Value.getConstantFP()->getValueAPF().bitcastToAPInt();
-    DwarfExpr.addUnsignedConstant(RawBytes);
+    if (AP.getDwarfVersion() >= 4 && RawBytes.getBitWidth() == 80)
+      // FIXME: For now, we only support long double.
----------------
This restriction seems overly strict and also partially wrong.

First off, the condition should be sunk into DwarfExpression. That's were we do the low-level DWARF expression encoding. Maybe we should add a DwarfExpression::addFPConstant(APFloat) function?

An then in the new function, since we cannot represent anything > 64 bit without DW_OP_implicit_value, we should error out if it's not supported, and not emit a broken constant like we did before.

================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h:300
+  /// Emit DW_OP_implicit_value.
+  void addImplicitValue(const APInt &Value);
+
----------------
See my comment above. It might be better to have something that takes an APFloat?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83560