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

[John Baldwin via Phabricator via llvm-commits]

bsdjhb added a comment.

In D69212#1716991 <https://reviews.llvm.org/D69212#1716991>, @jrtc27 wrote:

> I was not aware of that aside in the spec, which does indeed suggest this specific case should sign-extend, and I guess gives us a reason why this isn't a slippery slope. However, even if this is the case, I still feel code shouldn't be relying on this, simply because it's unclear (and perhaps surprising to those who don't know this particular note from the spec). One might perfectly reasonably assume that both signed and unsigned i32 types (in the original source) are zero-extended, or (probably the default position of people) that the signed type is sign-extended and the unsigned type is zero-extended (which would require passing the extension information through via the asm call instruction's operands).


FWIW, even though I had ready this part of the spec previously, I didn't really notice it until I went back and re-read it while investigating this specific issue (I was actually hunting for an unsigned variant of lw.r when I found this section instead).  I think having a warning makes sense as this behavior is certainly not common across platforms.  I also find it odd that this comment is commentary in the spec rather than formal language in the spec as it seems to assert that this behavior is required of the compiler rather than optional.  For FreeBSD's atomic.h I'm going to add explicit casts to force sign-extension which I think would also satisfy the proposed warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69212