Fix RV64F compilation, simplify fmv implementation, and make nan boxing functions generic #594
Conversation
| assert(xlen >= 64); | ||
| let rs1_val_D = F(rs1)[63..0]; | ||
| let rd_val_X : xlenbits = sign_extend(rs1_val_D); | ||
| X(rd) = rd_val_X; |
There was a problem hiding this comment.
Please keep the extends/truncates, they generalise the code to support xlen != 64 and flen != 64 (RV128 or RVQ), whereas yours assumes xlen == flen == 64.
There was a problem hiding this comment.
That is, the only code changes needed should be to add && flen >= 64 to the asserts.
There was a problem hiding this comment.
Agreed. This will need to be reverted in #445, so no reason to remove it here. Simplifying it to all happen in-line like you did with F seems cleaner though.
There was a problem hiding this comment.
Hmm good point! I think it was technically missing the nan_box, presumably because the Q version wasn't available here so I added it, and pulled the code from #448 to make the nan boxing functions support all of the float sizes.
| let rd_val_X : xlenbits = sign_extend(rs1_val_S); | ||
| X(rd) = rd_val_X; | ||
| // A narrower n-bit transfer out of the floating-point registers will | ||
| // transfer the lower n bits of the register ignoring the upper FLEN-n bits. |
There was a problem hiding this comment.
I don't see the point of these comments? These behave exactly as one would expect, and are entirely consistent with the rest of the ISA.
There was a problem hiding this comment.
That is, reading a 32-bit value reads that 32-bit sub register, and writing one either sign-extends, in the case of integer destinations, or nan boxes, in the case of float destinations.
There was a problem hiding this comment.
Yeah it wasn't meant to contradict any expectations - it's a direct quote from the spec; just explain why the code was doing what it is (because it's not obvious IMO unless you are an expert). I can remove it if you like. Or maybe put it in quotes?
Timmmm
changed the title
This simplifies the code and allows easily supporting the Q extension.
Simplify the implementations with fewer intermediate variables, and fix compilation of RV64F. I also added relevant quote from the spec because the spec for these instructions is very confusing. This is a prime candidate for getting Sail code into the spec.
Timmmm
force-pushed
the
user/timh/flen_fixes
branch
from
60448aa
to
feb2e2d
Compare
Timmmm
changed the title
Simplify the implementations with fewer intermediate variables, and fix compilation of RV64F.
I also added relevant quote from the spec because the spec for these instructions is very confusing. This is a prime candidate for getting Sail code into the spec.
Fixes #556