[PATCH] D94670: [DebugInfo][NFC] add a new DIE type to represent label + offset

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:583
   /// implicitly uses .set if it is available.
-  void emitLabelPlusOffset(const MCSymbol *Label, uint64_t Offset,
+  void emitLabelPlusOffset(const MCSymbol *Label, int64_t Offset,
                            unsigned Size, bool IsSectionRelative = false) const;
----------------
ikudrin wrote:
> Can this change be separated and its own justification be added?
Hmm, it should be ok to set `Offset` as `int64_t` or `uint64_t`. Two users in `emitLabelPlusOffset` for `Offset`. One is `EmitCOFFSecRel32(Label, Offset)`(unsigned user) and the other is `MCConstantExpr::create(Offset...)`(signed user). 

I need a negative offset for XCOFF dwarf, so even the offset is not implicitly converted as `emitLabelPlusOffset` parameter, it will be converted to `int64_t` in `MCConstantExpr::create(Offset...)`. For now I left it as unchanged.

I originally changed it because I think it makes more sense to use a signed integer for an offset. Do you think should we change it to signed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94670