[PATCH] D89838: [DebugInfo] Fix legacy ZExt emission when FromBits >= 64 (PR47927)

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

In D89838#2344609 <https://reviews.llvm.org/D89838#2344609>, @probinson wrote:

> Ummm the default expression-stack type is address-sized, and I'm not aware that we support machines with address sizes > 64 bits?
> (DWARF 5 does have a typed stack, but IIUC this path is not used for DWARF 5.)

@probinson thanks for catching this. Would it be appropriate to use two DW_OP_bit_pieces to implement the zext, like: `DW_OP_breg5 RDI+0, DW_OP_bit_piece 64 0, DW_OP_lit0, DW_OP_bit_piece 64 64`?



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:658
+    emitUnsigned((1ULL << FromBits) - 1);
+  } else {
+    emitOp(dwarf::DW_OP_lit1);
----------------
markus wrote:
> I think this is a good middle ground solution. 
> 
> Perhaps if one wanted to be more generic and do something about the current inability to encode arbitrarily large constants in the Dwarf expression one could make a generic DwarfExpression method that takes an APInt and encodes it with a  shift and or sequence.
Yes. Fwiw, I did try adding an `emitUnsigned(APInt)` API to DwarfExpression, but decided to not pursue that direction because the patch become very large. Ideally there'd be a simpler fix that serves.


================
Comment at: llvm/test/DebugInfo/X86/legacy-zext.ll:9
+define void @t(i64 %x) !dbg !6 {
+  call void @llvm.dbg.value(metadata i64 %x, metadata !9,
+                            metadata !DIExpression(DW_OP_LLVM_convert, 64, DW_ATE_unsigned,
----------------
markus wrote:
> I guess it does not matter much for what we want to test here but shouldn't !9 be a 128-bit type for this to make sense?
Yes, !9 should be a 128-bit type. I'll fix this in the next revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89838