[PATCH] D53738: [Fixed Point Arithmetic] Fixed Point Addition

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D53738#1310212 <https://reviews.llvm.org/D53738#1310212>, @leonardchan wrote:

> In D53738#1309171 <https://reviews.llvm.org/D53738#1309171>, @ebevhan wrote:
>
> > In D53738#1308314 <https://reviews.llvm.org/D53738#1308314>, @leonardchan wrote:
> >
> > > > Generally I think it's good! One final note; I assume we could technically reuse/rename the EmitFixedPointAdd function and use it to emit other binops when those are added?
> > >
> > > Yes, but I imagine if we choose to keep the call to `EmitFixedPointConversion` to cast both operands to a common type, this wouldn't be reused for division or multiplication since I believe those do not require for the operands to be converted to a common type.
> >
> >
> > They don't? The example given by the spec is even `int * _Fract`.
>
>
> Oh, I meant that the scales of both operands won't need to be aligned before performing the operation. Since for multiplication, we can multiply fixed point numbers in any scale without shifting and only need to perform a shift on the result to convert to the destination type. Although this would only apply to non-saturating multiplication since to use the intrinsics, both operands would need to be in the same scale.


I see what you mean, but the fixed-point multiplication intrinsic requires the operands to be in the same scale.

It's certainly interesting to degenerate integer-with-fixedpoint to just a mul (since the scaling factor is 2^-n * 2^0, which is just 2^-n), but in the general case you can't avoid doing the scale alignment. Unless I'm missing something.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53738/new/

https://reviews.llvm.org/D53738