Re: [musl] Re: [PATCH] Fix RISC-V a_cas inline asm operand sign extension

["Luís Marques" <luismarques@lowrisc.org>]

On Wed, Jan 22, 2020 at 2:46 PM Rich Felker <dalias@libc.org> wrote:
> Can you clarify why it's needed and why sign-extending is the right
> thing to do, though? Does lr.w sign-extend the loaded value (vs
> zero-extend or something else)?

LR.W does indeed sign-extend the value read from memory, before
storing it in the destination register (the ISA is quite consistent
about sign-extending). The branch operates on the whole 64-bits, so
the value to compare with must also be sign-extended. That's generally
not a problem because the ABI mandates the sign-extension of the
32-bit int to XLEN (64 bits on rv64), and that's something that the
compiler normally does automatically. The exception is the inline
assembler, which in this case becomes a problem when the function is
inlined.

> Thanks for pinging this again. It's unfortunate that clang doesn't do
> this right to begin with, but the patch is not terribly ugly and
> probably okay.

Yeah, this is a tricky issue. Unfortunately, it's a bit more
complicated than just "clang doesn't do the right thing". A patch was
proposed before to try to solve this issue [1], but this is an issue
that already existed for other architectures, and where Clang and GCC
were already inconsistent, both internally and compared to each other.
The difference is that this issue generally only manifested itself
with other C types (e.g. short). We (LLVM) could take a principled
stance where we guaranteed that the values would be properly
sign/zero-extended and truncated back (although my vague recollection
is that that would be tricky, if at all possible), but that wouldn't
guarantee compatibility with GCC, so you couldn't really rely on that
behavior anyway. My understanding is that GCC's sign-extension of the
value is an implementation quirk and not something guaranteed to
happen, and will in fact not happen in other not too dissimilar cases.
And the official stance of the GCC developers is that you can't rely
on it, and a lot of other stuff related to inline assembly. Still,
this is something that we want to improve in the future, because it's
indeed tricky. But the fix might just be to warn of the issue and
request that the value be explicitly casted.

[1] https://reviews.llvm.org/D69212