[PATCH] D102220: [NFC, CFI] Use unsigned type for CFI register values in parser/streamer code

[Scott Linder via Phabricator via llvm-commits]

scott.linder added a comment.

Would it be reasonable to just clean up all the types in these functions? I noted the ones I identified.



================
Comment at: llvm/include/llvm/MC/MCStreamer.h:983
+  virtual void emitCFIOffset(uint64_t Register, int64_t Offset);
   virtual void emitCFIPersonality(const MCSymbol *Sym, unsigned Encoding);
   virtual void emitCFILsda(const MCSymbol *Sym, unsigned Encoding);
----------------
Should this just be `uint64_t`?


================
Comment at: llvm/include/llvm/MC/MCStreamer.h:984
   virtual void emitCFIPersonality(const MCSymbol *Sym, unsigned Encoding);
   virtual void emitCFILsda(const MCSymbol *Sym, unsigned Encoding);
   virtual void emitCFIRememberState();
----------------
Same


================
Comment at: llvm/include/llvm/MC/MCStreamer.h:993
+  virtual void emitCFIReturnColumn(uint64_t Register);
   virtual void emitCFIGnuArgsSize(int64_t Size);
   virtual void emitCFISignalFrame();
----------------
Same


================
Comment at: llvm/lib/MC/MCParser/AsmParser.cpp:1534
+
+  Res = static_cast<uint64_t>(Value);
+
----------------
The `Absolute` in this case is just referring to "not relative", right? The `Res` value from `evaluateAsAbsolute` may be negative, and this cast will result in some large unsigned value for that case.

Maybe we just add an `assert(Value > 0)`? Or as you note a variant of `evaluateAsAbsolute` for `unsigned` which does the check could work.


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:1844
+
+  Res = static_cast<uint64_t>(Value);
+
----------------
Same


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102220