[PATCH] D83423: [MC, NVPTX] Add MCAsmPrinter support for unsigned-only data directives.

[Artem Belevich via Phabricator via llvm-commits]

tra marked an inline comment as done.
tra added inline comments.


================
Comment at: llvm/lib/MC/MCExpr.cpp:69
+    else if (MAI && !MAI->supportsSignedData())
+      OS << static_cast<uint64_t>(Value);
     else
----------------
hfinkel wrote:
> tra wrote:
> > hfinkel wrote:
> > > Will uint64_t always be correct here? Shouldn't this depend on SizeInBytes (like the hex printing does)?
> > MCConstantExpr::getValue() returns int64_t so casting it to uint64_t should be safe.
> > 
> > I guess I can find the matching unsigned type using std::make_unsigned. E.g:
> > ```
> >     using unsigned_t = typename std::make_unsigned<decltype(Value)>::type;
> >     OS <<  static_cast<unsigned_t>(v);
> > ```
> I'm not worried about the cast being unsafe, at the C++ level, I'm worried about it printing a number larger than the relevant directive actually accepts. In your test, the directive is .b64, which presumably takes a 64-bit integer, so everything's fine. What if it were .b8 and the printed argument were 18446744073709551613 (or whatever)?
> 
Got it. I've masked out the unwanted bits. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83423