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

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

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

> I'm currently rebasing this patch switching APInt to uint64_t and one thing I'm a bit worried about is the case where the implicit conversion from Register to unsigned causes registers to be passed where immediates are expected. At the moment our plan guards against the case where Register is correctly used but not against trivial misuse like:
>
>   unsigned Reg = Register();
>   SrcOp(Reg)
>
>
> and with the current unsigned constructor, this:
>
>   SrcOp(1)
>
>
> will be treated as a register.
>
> I had a look into the feasibility of eliminating the implicit conversions and that's way too big a task at the moment to block this on fixing that (I'll post some patches that propagate Register a bit more tomorrow, they were a side-effect of looking into it). I have a potential alternative though. We can add some deleted constructors like in https://godbolt.org/z/0zQOJX. The downside of this is plain constants like `0` won't work, you need to use `0ul`. Does that limitation sound ok if it makes mixing up registers and immediates harder?


Of course, the moment I post I notice a case I'd missed. AArch64::WZR is an unsigned, we'd have to pass it as `Register(AArch64::WZR)` to avoid hitting the deleted constructor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61321