riscvarchive / riscv-binutils-gdb

RISC-V backports for binutils-gdb. Development is done upstream at the FSF.
GNU General Public License v2.0
147 stars 233 forks source link

Truncation of shift amounts - immediate vs. indirect #198

Open gsauthof opened 4 years ago

gsauthof commented 4 years ago

With sll, srl and sra the shift amount is truncated by the CPU to 6 bits (RV64) or 5 bits (RV32), i.e. only the lower 6 or 5 bits are used as shift amount.

That means it's perfectly fine to shift by -1, meaning shift by 63 or 31 bits - because of the truncation.

In contrast to this, the GNU as is very strict when dealing with shift amounts of the immediate shift instructions slli, srli and srai.

Using a shift amount thus doesn't fit into 6 or 5 bits (such as -1) yields the following error:

Error: Improper shift amount

Arguably, this asymmetry is surprising and unnecessary.

Suggestion: also silently truncate shift amounts in GNU as in the processing of the immediate shift-instructions to align the behavior to what happens in the CPU when executing the indirect variants.

Use-case: Besides the argument of symmetry, this would simplify code that is XLEN agnostic because it relies on shift amount truncation (example: immediate rotation).

jim-wilson commented 4 years ago

I believe it is standard practice for assemblers to warn when a operand is out of range, even when it is an operand that will be truncated. Just doing a quick check, I see that arm, mips, and sparc assemblers also give warnings for out of range shift counts. The mips and sparc truncates like riscv, and arm truncates for rotate which is what I tested.

If we don't warn for out of range shift counts, then it would be easy for an accidental bug to turn into a silent error that only shows up at run-time which is even worse.

For portability, there is already other stuff you need to do for 32/64 bit support like handle add and addw. It is easy enough to handle shift counts the same way. You can do this with macros, or conditional compilation for instance.

gsauthof commented 4 years ago

I believe it is standard practice for assemblers to warn when a operand is out of range, even when it is an operand that will be truncated. Just doing a quick check, I see that arm, mips, and sparc assemblers also give warnings for out of range shift counts. The mips and sparc truncates like riscv, and arm truncates for rotate which is what I tested.

I don't understand. Current RISC-V binutils as does not truncate. It writes an error message and then terminates with exit status 1. It doesn't produce an object file.

Emitting a warning is one thing, but handling this as hard error is a different story. I'm arguing about classifying this as an error.

I did a quick check with x86-64 GNU as and shl $-1, %r15 and shl $63, %r15 are assembled without an error (and yield the same result during execution) and GNU as doesn't even generate a warning.

If we don't warn for out of range shift counts, then it would be easy for an accidental bug to turn into a silent error that only shows up at run-time which is even worse.

I'm not arguing against warning about this. Ideally such a warning could be disabled.

For portability, there is already other stuff you need to do for 32/64 bit support like handle add and addw. It is easy enough to handle shift counts the same way. You can do this with macros, or conditional compilation for instance.

There is always other tricky stuff, but this issue would be easy to fix.

What kind of macro would I want to use to mask the 6 or 5 lowest bits depending on assembling for RV64G or RV32G?

jim-wilson commented 4 years ago

I meant the mips/sparc/arm hardware truncates like riscv hardware does. And the mips/sparc/arm assemblers give errors like the riscv assembler does. The x86_64 assembler behavior is probably historical error which can't be fixed without breaking old code bases.