ucb-bar/fixedpoint

Disallow negative binary point

Opened this issue · 1 comments

Hi,

I have followed your progress on the FixedPoint type after initially opening chipsalliance/chisel#3161. Thanks a lot for taking that up!

I wanted to ask about "Disallow negative binary point" in one of your latest commits: cb42069. Is disallowing negative binary points necessary?

I know this is also not supported in the current FixedPoint of chisel-3.5.6, but it's actually a feature that I've wanted to use in the past. For example, val fp = FixedPoint(8.W, -8.BP) would represent a 16-bit integer fp * 2^8. This is useful when we're only interested in the most significant bits of an integer.

Is there any reason to explicitly disallow this in the current implementation?

Thanks,
Michiel

Hello,

The main reason for disallowing binary point is simply because Chisel disallowed it, this implementation closely followed the Chisel implementation, and yet this implementation wouldn't disallow it unless explicitly coded to do so. So, I just closed off the possibility of using negative binary point since there's no guarantee that it works properly in the current implementation. I didn't explore negative binary point since the team I work with didn't have a use for it in their modules, and I didn't see it as something that's being used in general.

There's also a motive of not adding too many new features, because the entire thing is harder to maintain the more features are provided. So, unless a feature is something that is actually commonly used with the fixed-point type and there's a real need for it, I would be against adding it. And I don't think negative binary point falls in the commonly-used category. Still, with that said, the current implementation does feel incomplete for not allowing negative binary point, and conceptually you should be able to have fixed-point numbers with negative binary point. So, if it doesn't over-complicate the fixed-point implementation, I won't be against adding negative binary point support. I don't currently have the incentive to get into it myself, though.

For anyone wanting to explore negative binary point with the current implementation, I suspect that most of the operations will work just fine with it. I would start with extending all relevant tests with test cases containing fixed-point numbers with negative binary point, and then trying to fix what doesn't work. A feature like this would definitely need to be covered well inside tests anyway.