[PATCH] D69212: [RISCV] Sign-extend 32-bit integer inline assembly operands on RV64I

[Sam Elliott via Phabricator via llvm-commits]

lenary added a subscriber: luismarques.
lenary added a comment.

I think there are a few issues here (having read the freebsd ticket):

1. How do we choose to do something reasonable?
2. How do we tell users there are sharp corners here?

To provide opinionated answers, having had a chat with @luismarques before we saw the comment from @bsdjhb:

1. We need a sensible default here. Because GCC doesn't define any behaviour around inline assembly (and doesn't expect to maintain compatibility between versions), we can choose to do something principled and stick to that. Given the specification wording, I think sign-extending makes sense (given the specification), but if we want to do something different for signed vs unsigned types, this will be much more complex (LLVM IR doesn't have signed/unsigned types). I do think looking at the RISC-V specification and forming a reasonable behaviour based off that (without looking at other implementations) is the correct approach in this case.

2. I absolutely believe we need a warning if the operand type does not match the register size. This is important for users who want to avoid this sharp edge (users who don't care can turn off warnings). It doesn't need to be enabled by default, but warnings are a great way of encouraging developers to improve their code, and also help to point to potential code issues when switching compilers/versions which may have incompatibilities (as inline assembly does have, see comment about GCC in 1.). I would go so far as to say this warning should be cross-target, so other targets can benefit too. I do realise this could be hard to implement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69212