[PATCH] D56900: [Fixed Point Arithmetic] Fixed Point and Integer Conversions

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.


================
Comment at: clang/test/Frontend/fixed_point_conversions.c:437
+  // DEFAULT-NEXT: [[RES:%[a-z0-9]+]] = trunc i39 [[SATMIN]] to i16
+  // DEFAULT-NEXT: store i16 [[RES]], i16* %sat_sa, align 2
+
----------------
leonardchan wrote:
> ebevhan wrote:
> > Conversions like this are a bit odd. There shouldn't be a need to upsize/upscale the container before the saturation, should there?
> I see. You're saying that we can just check directly if the value exceeds 255 (or goes under -256) since the range of target values can always fit in the range of source values. Therefore we do not need to cast up since the only reason we would need to is if converting to a type with a greater source range.
> 
> In this case, we could technically have a special case for integers where I think we can perform the saturation checks without the initial sign extension. I think it might be simpler though if in `EmitFixedPointConversion`, we treat an integer as a 'zero-scale fixed point number'. I think the reason the upsizing occurs in the first place is so that we satisfy the first FX conversion rule of calculating the result with full precision of both operands.
> I see. You're saying that we can just check directly if the value exceeds 255 (or goes under -256) since the range of target values can always fit in the range of source values. Therefore we do not need to cast up since the only reason we would need to is if converting to a type with a greater source range.

That's right. Though, for integer to fixed-point conversion, I don't think you ever need to upcast first; either the integer is larger than or equal to the integral part of the fixed-point type, in which case we can check the magnitude in the type as is, or it's smaller, and we don't have to do any saturation.

> I think it might be simpler though if in `EmitFixedPointConversion`, we treat an integer as a 'zero-scale fixed point number'.

Isn't this already the case? The semantics for an integer type are fetched as a zero scale fixed-point type and used that way (except when the target is an integer due to the rounding requirement).


Repository:
  rC Clang

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

https://reviews.llvm.org/D56900