[PATCH] D61321: [globalisel] Allow SrcOp to convert an APInt and render it as an immediate operand (MO.isImm() == true)

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D61321#1485092 <https://reviews.llvm.org/D61321#1485092>, @dsanders wrote:

> In D61321#1484604 <https://reviews.llvm.org/D61321#1484604>, @arsenm wrote:
>
> > For the use cases, I think this should just be int64_t instead of APInt
>
>
> int64_t was my first thought for it but I got rather worried about the potential mistakes due to registers being `unsigned`. For example, is `1` a register or an immediate? It turned out that my worries were moot as the compiler doesn't allow both an unsigned and an int64_t overload anyway (https://godbolt.org/z/zO8LhX)




In D61321#1485092 <https://reviews.llvm.org/D61321#1485092>, @dsanders wrote:

> In D61321#1484604 <https://reviews.llvm.org/D61321#1484604>, @arsenm wrote:
>
> > For the use cases, I think this should just be int64_t instead of APInt
>
>
> int64_t was my first thought for it but I got rather worried about the potential mistakes due to registers being `unsigned`. For example, is `1` a register or an immediate? It turned out that my worries were moot as the compiler doesn't allow both an unsigned and an int64_t overload anyway (https://godbolt.org/z/zO8LhX)


I vaguely remember trying to do something like this for SrcOp before, and hit this problem but didn't continue working on it. If you disambiguate the constant type, it works: https://godbolt.org/z/aLyc1T
I don't think using needing to use INT64_C is really worse than having to construct an APInt. I really despise using a plain unsigned for registers, but fixing that would be a hopelessly large project


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61321