rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Add an RFC for fixed point types.

Open zyp opened this issue 1 year ago • 44 comments

Rendered

zyp avatar Dec 22 '23 23:12 zyp

I feel like implicit conversions from floats are potentially a source of bugs severe enough that maybe we shouldn't have them at all.

I think for making constants of a specific shape it's reasonable to have implicit float conversion. As the other argument to binary operators it's probably reasonable to require an explicit conversion to a constant first.

  • How does one perform computation on fixed point values during compilation? I think "you just use floats" is an unsatisfying answer.

An adjacent question is «what will FixedPointValue.get() in the simulator return?» We should probably add a separate FixedPointConstant type.

zyp avatar Dec 23 '23 00:12 zyp

We should probably add a separate FixedPointConstant type.

Are there good fixed point libraries for Python we could use here?

whitequark avatar Dec 23 '23 00:12 whitequark

I have nothing concrete here, just minddumping stuff I wrote down in text files locally.

I assume that __div__ is deliberately not implemented. Thanks to the curse of rational numbers you'd need infinite bits to represent many divisions without loss, such as 1/5. Amaranth extends width so values can be completely represented, but this is impossible for fixed point division.

You can get as much precision as you'd like from dividing fixed point numbers by left-shifting a numerator (increasing its f_width) before dividing. Consider dividing e.g. 1*10^-6/999999*10^-6:

  • 1*10^-6/999999*10^-6 = int(1/999999) * (10^-6/10^-6) = 0*10^0.
    • Q(0,6)/Q(0,6) yields Q(6,0), yields 0 due to not enough precision.

On the other hand, we can left-shift the integer part to get a non-zero result from this divide. :

  • 1*10^-6/999999*10^-6 = 1*10^-6*(10^7*10^-7)/999999*10^-6 = int(10000000/999999)*(10^-13/10^-6) = 10*10^-7
    • Q(0,13)/Q(0,6) yields Q(6,7), yields a non-zero result (close enough).

I arbitrarily chose to shift by 7 decimal places (replace with binary for this RFC), but it's not obvious to me that any given left shift will be universally good for all use cases to prevent divides from returning 0 for non-zero numerators.

Additionally, you may only want the extra precision during the divide; e.g. I could truncate the Q(6,7) in the second example back down to Q(0,7), saving 6 bits, and still have a perfectly valid result to pipe to the rest of a design. Whether/how much to truncate depends on the dynamic range of fixed point values you expect your design to see.

Will fixed point divide be rare enough that divide should be completely ignored? Maybe there should be a divide function that allows the user to tweak:

  • Extra precision used during the integer divide portion.
  • The Q of the output.

I do not have a concrete use case in mind right now (I could probably shoehorn division into my Celsius to Fahrenheit conversion experiments). Just something I mulled over today.

cr1901 avatar Feb 27 '24 01:02 cr1901

You could always multiply by reciprocal for an application like unit conversion.

whitequark avatar Feb 27 '24 07:02 whitequark

Multiply-by-reciprocal only works if you can calculate it ahead of time- i.e. one of the multiply inputs is constant. Otherwise you'd have to to find the reciprocal while your design runs before doing the multiply, which... requires a divide. The thing you're trying to avoid.

cr1901 avatar Feb 27 '24 14:02 cr1901

This is exactly the case for Celsius to Fahrenheit conversion, no?

whitequark avatar Feb 27 '24 14:02 whitequark

Indeed, I will need to find a different way to shoehorn a division into my Celsius to Fahrenheit experiments :).

Divide by non-constant factor for fixed point is probably uncommon. But I hesitate to say it's "so uncommon it's not worth supporting in some way at all, even if a __div__ impl is impossible".

cr1901 avatar Feb 27 '24 14:02 cr1901

The question of whether __div__ makes sense to have is whether it's feasible for synthesis to infer a divider.

Independent of that, there's already nothing that prevents you from making a divider component with two fixedpoint inputs and a fixedpoint output.

zyp avatar Feb 27 '24 14:02 zyp

The question of whether div makes sense to have is whether it's feasible for synthesis to infer a divider.

Do FPGAs have divider IPs? I thought they only had multiplier IP. I assumed amaranth provides floor division/mod for simulation purposes, and not something you'd want to synthesize.

cr1901 avatar Feb 27 '24 14:02 cr1901

is whether it's feasible for synthesis to infer a divider.

Yosys will actually synthesize a divider for you if you do a // b in Amaranth. Here's the size for a 8/8=8 combinational divider for iCE40:

   Number of cells:                114
     SB_CARRY                       64
     SB_LUT4                        50

whitequark avatar Feb 27 '24 14:02 whitequark

The question of whether div makes sense to have

Even if dividers synthesize, I don't think __div__ makes sense to have for fixed point; in integer division, "d doesn't divide n evenly" is handled by remainder. Therefore all values can be represented in the output without loss, which is great since Amaranth never overflows.

In fixed point, there's no remainder, so if "d doesn't evenly divide n" we keep adding bits to n until d evenly divides our modified n. This can require infinite number of extra added bits, and if we keep with "Amaranth arithmetic never overflows" semantics, this is impossible to satisfy.

Even if __div__ doesn't make sense, I think there should be something for divides (a function instead of an overloaded operator?) Last night was me minddumping my thoughts on what fixed point divide should handle.

cr1901 avatar Feb 27 '24 14:02 cr1901

If a perfect __div__ is desirable to have, instead of a fixed.Value, we could simply have it return a ratio object containing the two operands. Once such a ratio is passed to e.g. fixed.Value.eq(), the actual division could be performed with the precision of the assignment target.

To avoid scope creep, I'm inclined to leave inferred division out of this RFC. We could instead do a separate RFC later for that.

zyp avatar Feb 27 '24 15:02 zyp

To avoid scope creep, I'm inclined to leave inferred division out of this RFC. We could instead do a separate RFC later for that.

That's fine, mind putting your comment re: ratio objects under Future Possibilities/making a mention that division is out of scope?

cr1901 avatar Feb 27 '24 15:02 cr1901