[PATCH] D83560: [DebugInfo] Emit DW_OP_implicit_value for Floating point constants

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:233
+    for (int i = 0; i < NumBytes; ++i)
+      emitData1(Data[i]);
+    return;
----------------
bjope wrote:
> bjope wrote:
> > I'm working on a big-endian target (together with a debugger based on gdb 8.1.0).
> > I needed to reverse the order here (and in the loop at line 243), to get same result as before. 
> > 
> > Does that make sense? Should we care about endianess in DwarfExpression.cpp?
> > 
> One thing I noticed is that APFloat::bitcastToAPInt, according to comments in APFloat.cpp, is creating an APInt that is a bit map of the value as it would appear in memory. However, that would only be true if the APInt was stored as one value taking endianess into consideration. When looking at the rawBytes of the resulting APInt after bitcastToAPInt the bytes are in litte-endian order. So a byteSwap is generally needed for big-endian.
I don't know what the answer to the question, but I would encourage patches that hide the endianness problems as much as possible. I.e., the DwarfExpression API should have a high-level addConstantFP(const APFloat &Value) call, and DwarfExpression should know about endianness and do the right thing automatically (whatever the right thing is). What would be even better would be to hide endianness in AsmPrinter (instead of emitData1() use a high-level emitEndianCorrectedDataN() call — if such a thing makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83560