Hi folks. Over on my review here:

https://gem5-review.googlesource.com/c/public/gem5/+/13622

Giacomo kindly diagnosed a bug in my CL which has to do with the FloatReg
and FloatRegBits types and the accessors which get and set those values.

In a nut shell, I wanted to make FloatReg universally double, and
FloatRegBits universally uint64_t. That would mean that all the places that
use those types (generic register accessors, for instance) wouldn't need to
know what ISA was in use, since the types would be invariant. My idea was
that this would work out because if you wanted a uint32_t but got a
uint64_t, you'd just truncate it, and if you wanted a float but got a
double the compiler would trivially down convert it. The same sort of
conversions would happen in the other direction as well.

The problem arises from how the (read|set)FloatReg and
(read|set)FloatRegBits functions interact with each other.

Lets say you want to store a 32 bit floating point value in a register, and
then later read that out as a 32 bit bit pattern which represents that
value, maybe to interpret it as an int, or for whatever other purpose.

If you had floats and uint32_ts plumbed all the way through, you would
store that value as a float, and then get it's binary uint32_t
representation in a very straightforward way.

float -> uin32_t

With my new scheme however, there's an extra step in the middle which
doesn't fold away properly and gets you a different answer.

float -> double -> uint64_t -> uint32_t

The float to double conversion is the result of upconverting for the
setFloatReg function's argument. Then the "conversion" from double to
uin64_t is done by having both in a union and getting the uint64_t member
out. The final conversion from uint64_t to uint32_t is a truncation.

So in this version, what you get is a 32 bit truncated version of the
binary representation of the floating point value as a double. Or in
simpler words, the double chopped in half instead of the float.


I've thought about this a little bit, and, apart from giving up on the
whole thing which I'd rather not do, the nicest solution that I've thought
of would be to remove the accessors which work with floating point values
and only ever work with integer bitwise representations of those registers.
Also I would add some small, inline utility conversion functions to
translate between the bitwise and floating point representations at the
different widths.

If you wanted to read a floating point value as a float, you would
therefore read it as a uint64_t, and pass it through the converter to get a
float of the size you wanted. If you want to store a float, you would first
convert it to whatever sized uintxx_t was appropriate (float -> uint32_t,
double -> uint64_t) and then store that.

This does have the downside that extracting floating point values from the
floating point register file (what it's for, right?) becomes a bit more
cumbersome, but I think that's mitigated almost entirely by the fact that
those calls are almost all/all generated by the ISA parser.

On the plus side, it should make what I was originally trying to do work,
and it will also remove a raft of plumbing which pipes those values through
various interfaces


I'm going to start working on this and put up some CLs at some point.
Feedback welcome.

Gabe
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to