riscvarchive / riscv-binutils-gdb

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

binutils doesn't understand rm operand of some fcvt instructions #139

Closed alexrp closed 6 years ago

alexrp commented 6 years ago

In particular:

fcvt.d.s f0, f1, rne
fcvt.d.w f0, x1, rne
fcvt.d.wu f0, x1, rne

Results in:

test.s: Assembler messages:
test.s:1: Error: illegal operands `fcvt.d.s f0,f1,rne'
test.s:2: Error: illegal operands `fcvt.d.w f0,x1,rne'
test.s:3: Error: illegal operands `fcvt.d.wu f0,x1,rne'

Disassembling these instructions (if emitted manually with .word) just results in raw data being printed as well.

This is despite the ISA specification saying these instructions do have an rm operand.

aswaterman commented 6 years ago

These instructions do not round. Their result is always exact.

alexrp commented 6 years ago

So then this would be a specification bug?

aswaterman commented 6 years ago

I believe the spec says (in English, not in the diagrams) that these instructions don’t round and should have the rm field set to 0.

alexrp commented 6 years ago

Indeed, but as I was writing C code to generate RISC-V instructions, I was primarily using the instruction set listings as a reference. Those indicate that these instructions have an rm field, rather than specifying that it should be zeroed. Perhaps those should be updated to reflect this?

aswaterman commented 6 years ago

There’s a tension there,too... As written, it’s confusing for your use case. If we change it to have zeros, it’ll be confusing for hardware implementors, because the hardware should not enforce that the rm field contains 0. (The trapping behavior is the same as for instructions that actually do round. This simplifies the logic that detects illegal instructions.)

alexrp commented 6 years ago

Hmm, fair enough. I would say that since it is the user-level ISA spec (i.e. software developers will likely be looking at it), a small note somewhere in the spec that developers should zero the field out would probably be in order.

In any case, that's probably a discussion more appropriate for the spec repo, so I'll close this.