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

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX marked 4 inline comments as done.
SouraVX added a comment.

For cases other than those gracefully handled(``float, double, long double`)  for `DW_OP_implicit_value` I'm considering a hard error sort of `assert/llvm_unreachable` or 2nd option could be just bail out early.

And for other consumer targets, still using `_stack_value` based expression, I'm considering bailing out early for `variables > 64 bits` instead of incorrectly representing it.

@aprantl do you any suggestions/objection WRT this. Please share!



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:231
+    for (int i = 0; i < NumBytes; ++i)
+      emitData1(Data[i]);
+  }
----------------
aprantl wrote:
> `return;`
Ah, Sorry will take care of that.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:233
+  }
+  if (NumBytes == 10 /*long double*/) {
+    // long double IEEE representation uses 80 bits(10 bytes).
----------------
aprantl wrote:
> Why would we special-case Float80 instead of just emitting everything that we didn't emit in the above block here unconditionally?
Initially thought of implementing that way, but due padding issues this cannot be done.
For instance, for `long double` bit width will be 80 bits(10 bytes) and emit as
```
emitUnsigned(NumBytes);
for(...)
emitData();
```
This will ends up in expression of `NumBytes(10)` in this case, however `DW_AT_byte_size` for `long double` type is `16` bytes -- this conflict resulting in following error in `GDB` 
```
(gdb) p ld
access outside bounds of object referenced via synthetic pointer.
```
For addressing padding issues and another distant issue(some other Floating point constant with different IEEE representation should not be emitted erronously.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83560