Add an RFC for fixed point types.

[zyp]

Rendered

[whitequark]

General comments:

• We designed lib.data and lib.wiring to be imported as entire modules: from amaranth.lib import data, wiring. This library should be designed (naming-wise) to work that way too. fixed.Shape and fixed.Value are one option, though I can see why others may object to it.
• I feel like implicit conversions from floats are potentially a source of bugs severe enough that maybe we shouldn't have them at all.
• How does one perform computation on fixed point values during compilation? I think "you just use floats" is an unsatisfying answer.

[whitequark]

These two lines seem mutually incompatible.

[zyp]

The idea is that the underlying implementation would be FixedPoint(i_or_f_width, f_width = None, /, *, signed), but I found it clearer to express it like that. Consider how you can do range(stop) and range(start, stop).

[whitequark]

Makes sense. I wasn't sure if it was that or a typo. Does the first constructor add a sign bit when signed is true, and otherwise create a number whose range spans from 0 to some value below 1?

[zyp]

Yes, although the exact semantics wrt. the sign bit depends on which Q notation we settle on.

[samimia-swks]

The dominant notation in the industry, at least in the audio ASIC world, is to include the sign bit in the number of integer bits. For example, -1 to 1 is represented as a Q1.23. That would be my vote.

[whitequark]

Bikeshed: lib.fixed, lib.fixnum.

[whitequark]

What is the semantics of this operation?

[zyp]

I just updated it to .cast(shape, f_width=0) locally. The idea is to determine i_width automatically, so FixedPoint.cast(unsigned(8)) would result in UQ(8) and FixedPoint.cast(unsigned(8), f_width = 4) would result in UQ(4, 4).

[whitequark]

Why UQ(4,4)?

[zyp]

FixedPoint.cast() is a building block for FixedPointValue.cast(). The idea is to be able to say «here's a value with n fractional bits, please turn it into an appropriately sized FixedPointValue». An unsigned(8) with four fractional bits would have four integer bits left and thus cast to UQ(4, 4).

[whitequark]

This API seems confusing and difficult to use. Should we expose it at all?

[whitequark]

Decimal support as well?

[zyp]

And probably Fraction as well.

[whitequark]

I feel like the i_width and f_width names are difficult enough to read that it's of more importance than bikeshedding to come up with something more readable.

.int_bits, .frac_bits?

cursed option: int, frac = x.width?

[samimia-swks]

+1 on int_bits, frac_bits...

[whitequark]

I understand Q-notation is an established one, however since Amaranth defaults to unsigned, I feel that having SQ and UQ (without having just Q) would serve Amaranth better.

[whitequark]

What happens if you assign 1024 to a Q8.8 value?

[zyp]

Signal(Q(8, 8)).eq(1024) would effectively be Signal(signed(17)).eq(1024 << 8). (Or signed(16) depending on Q notation.)

Speaking of overflow, I figure fixed point overflow should behave like regular integer overflow. Saturating math feels orthogonal to this RFC and could be added through a later RFC that handles both integer and fixed point values.

[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.

[whitequark]

We should probably add a separate FixedPointConstant type.

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

[whitequark]

OK, this is a compelling argument for support of floats. I am convinced that it is a good idea to have floats supported in at least some way. But we need to be very careful around overflows.

[whitequark]

I wonder if we could leave it unspecified and implementation-defined until we can get a numerics expert to chime in.

[samimia-swks]

In most DSP applications, simple truncating is done (bit picking, which is equivalent to a floor()) because it's free. I would vote for that being the default behavior at least.

[ld-cd]

Truncating can also be a big foot gun in some DSP applications that is hard to track down, you can end up with a DC spur that grows over time or over your pipeline with no obvious explanation. A middle ground is round towards zero or symmetrically towards positive/negative infinity (this is nearly free on the output of Xilinx DSP48E1/2 and I believe cheap/free on ECP5 and friends DSPs). The way I personally would handle it would be to make the rounding mode part of fixed.Shape type making it fixed.Shape(i_width, f_width, /, *, signed, rounding_mode). Truncate is still a reasonable default for most applications though.

[whitequark]

Hm, making rounding_mode part of the signature is a pretty big action, what happens if you add two numbers with differing rounding modes for example?

[ld-cd]

To be clear the semantics I'm imagining here are if you have a and b and b has more fractional bits than a then when you assign a.eq(b) the rounding mode that is used is that of a so that downstream DSP modules can enforce a rounding mode on their inputs.

The scenarios where I can imagine what you propose causing issues are (I'm guessing there are more outside the bounds of my imagination)

1. User adds a and b and then .rounds them
2. A user adds a and b and utilizes the result as a port on their module (I believe this is allowed but still learning the language)

Solutions that solve 1. But not two in no particular order:

1. Don't actually resolve the round until a signal is assigned to something with a concrete value (this seems like a bad idea and probably doesn't solve the problem)
2. Make the rounding mode undefined and require an explicit rounding mode to .round, but this doesn't solve the problem of getting a top level port with an undefined rounding mode

Solutions that I believe would solve both:

1. Choose the rounding mode of the left term
2. Choose the less footgunny rounding mode (TOEVEN,TOODD>SYMZERO,SYMINF>TRUNC) with ties going to either a defined order or the left term
3. Ban arithmetic operations between numbers with different rounding modes and require an explicit cast of one of the arguments with something like .with_mode(other.rounding_mode)
4. Make this undefined behavior and pick one of the above for now

My naive preference inclination would be 3, it seems like a fairly rare scenario and if you end up in it it's probably worth making the user think about what they want the result to be, but I'm not a language designer so take that with a grain of salt

[ld-cd]

Further thoughts on 3 as an option:

Operations with a constant fixed point value should probably inherit the rounding mode of the non-constant value without requiring an explicit cast because I think the alternative is annoying to deal with (requiring a shape to be passed any time a constant is declared).

[whitequark]

This means that fixed.Shape(7, 0, signed=True) has the same width as signed(8), which seems counterintuitive.

[ld-cd]

I just managed to hit this myself even though I've read through this a few times

[zyp]

I've thought about this since I wrote the last draft and concluded that this is probably the wrong decision, so I'm intending to change it when I find time to revise the RFC again.

[whitequark]

What happens if it's not representable? At least we need to have c.as_float(exact=True) which gives an error in that case.

[whitequark]

This is inconsistent with how Const() works.

[cr1901]

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.

[whitequark]

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

[cr1901]

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.

[whitequark]

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

[cr1901]

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".

[zyp]

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.

[cr1901]

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.

[whitequark]

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

[cr1901]

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.

[zyp]

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.

[cr1901]

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?

[ld-cd]

Is having i_width positive and f_width negative or the converse in scope for this RFC? If it is then I think .round should take i_width as an optional argument

[whitequark]

What would that mean?

[ld-cd]

Two scenarios where I think you could reasonably end up in this position with the current RFC

1. You start off with say UQ0.16 [0,1) and right shift by 2 the natural representation is still 16 bits but with the range [0, 0.25) and I believe the representation is going to end up being UQ-2.18 in this notation?
2. This is less reasonable but say you throw a 4096 (1<<12) point FFT at values with the format SQ7.10, if you are naive about the scaling and keep the bit width to 18 bits, you end up with SQ19.-2 (ie a step of 4 between every point). This is equivalent in terms of the bits on the wire to starting with SQ0.17 and ending with SQ12.5 but it feels more silly and you can end up there naturally so it should probably be supported

I'm currently working on tracking down some DSP bugs in a project at work that nobody every simulated, in my fixed point emulation code the number format I chose was basically n_bits, exponent which handles these cases more naturally but is probably overall less intelligible. I'd imagine this looking like fixed.Shape(shape, exponent) in amaranth (IE fixed.Shape(signed(18), 2) for SQ19.-2) but that would be a pretty large change I'm not sure it would be a net positive anyways (also don't want to bikeshed here).

[ld-cd]

Ok I've played with what the implementation does here, this might be a bit long; The implementation does allow the creation of the above types of representations (IE Q-4.20, Q20.-4) and appears to operate on them correctly, however it for the most part won't generate them on its own. Left shift and right shift preserve the number of bits until either i_width or f_width would go negative at which point it pads things out:

Right shift works as expected:

In [2]: fixed.SQ(8, 8), fixed.SQ(-4, 20), Signal(fixed.SQ(8, 8)) >> 8, Signal(fixed.SQ(8, 8)) >> 12
Out[2]:
(fixed.Shape(8, 8, signed=True),
 fixed.Shape(-4, 20, signed=True),
 (fixedpoint SQ0.16 (sig $signal)),
 (fixedpoint SQ0.20 (sig $signal)))

Left shift seems to get converted to unsigned which I think is not the correct behavior:

In [4]: Signal(fixed.SQ(8, 8)) << 8, Signal(fixed.SQ(8, 8)) << 9, Signal(fixed.SQ(8, 8)) << 10, Signal(fixed.SQ(8, 8)) << 12
Out[4]:
((fixedpoint SQ16.0 (sig $signal)),
 (fixedpoint UQ18.0 (cat (const 1'd0) (sig $signal))),
 (fixedpoint UQ19.0 (cat (const 2'd0) (sig $signal))),
 (fixedpoint UQ21.0 (cat (const 4'd0) (sig $signal))))

I personally think the rounding semantics are a bit difficult to reason about as they stand:

In [5]: (Signal(fixed.SQ(8, 8)) << 12).round(-4), (Signal(fixed.SQ(8, 8)) >> 12).round(16)
Out[5]:
((fixedpoint UQ21.-4 (+ (slice (cat (const 4'd0) (sig $signal)) 4:21) (slice (cat (const 4'd0) (sig $signal)) 3:4))),
 (fixedpoint SQ0.16 (+ (slice (sig $signal) 4:17) (slice (sig $signal) 3:4))))

Rounding with negative fractional bits produces the sensible result (wrt signed to unsigned bug), but rounding off the fractional bits in a right shift looses precision when it does not necessarily need too. I think this is a decent argument for either always preserving the number of bits in a (constant) shift, or including an i_width argument to round. Personally I would advocate for always preserving the number of bits because I think that that is easier to reason about and the argument to round can just be width not i_width or f_width.

Addition between a number with no fractional bits and one with no integer bits also produces a number 2 bits wider than I believe is necessary:

In [7]: Signal(fixed.SQ(-4, 20)) + Signal(fixed.SQ(20, -4))
Out[7]: (fixedpoint SQ22.20 (+ (sig $signal) (cat (const 24'd0) (sig $signal))))

[ld-cd]

would a .max() and .min() method or property make sense here to get a const with the max/min representable value make sense here or is that outside the scope of the Shape API given that the base signed and unsigned types don't have that?

[ld-cd]

In the implementation at least the semantics of these seem to be different from underlying amaranth values. If the shift amount is an integer these act like .shift_left() and .shift_right(). Those operators should probably be added here and the semantics of __lshift__ and __rshift__ adjusted to match those on unsigned() and signed().

Note: I may be wrong here, still learning the language

[ld-cd]

For the comparison operators the rounding behavior seems under defined when the types don't have the same precision, the options seem to be:

1. Round to the precision of the left argument (what the draft implementation does) EDIT: Draft implementation does not have an opinion on this
2. Round to the precision of the least precise argument (easiest for the hardware)
3. Round to the most precise argument (excluding constants maybe?)
4. Ban comparison between values (but maybe not const and value?) with different precision

[ld-cd]

4. seems bad, I'm going to implement 3 for my FFT testing and see if it ends up being a problem

[ld-cd]

.as_value() returns a value with a signdedness that doesn't depend on the signdedness of the fixedpoint shape, should there be a .lower() or .numerator() method that returns .as_value() cast to the appropriate signdedness?