[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG53f5c8b4a14c: [AST] Add fixed-point multiplication constant
evaluation. (authored by ebevhan).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73186/new/
https://reviews.llvm.org/D73186
Files:
clang/include/clang/Basic/FixedPoint.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Basic/FixedPoint.cpp
clang/test/Frontend/fixed_point_mul.c
Index: clang/test/Frontend/fixed_point_mul.c
===
--- clang/test/Frontend/fixed_point_mul.c
+++ clang/test/Frontend/fixed_point_mul.c
@@ -1,6 +1,49 @@
// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+// Multiplication between different fixed point types
+short _Accum sa_const = 2.0hk * 2.0hk; // CHECK-DAG: @sa_const = {{.*}}global i16 512, align 2
+_Accum a_const = 3.0hk * 2.0k; // CHECK-DAG: @a_const = {{.*}}global i32 196608, align 4
+long _Accum la_const = 4.0hk * 2.0lk; // CHECK-DAG: @la_const = {{.*}}global i64 17179869184, align 8
+short _Accum sa_const2 = 0.5hr * 2.0hk; // CHECK-DAG: @sa_const2 = {{.*}}global i16 128, align 2
+short _Accum sa_const3 = 0.5r * 3.0hk; // CHECK-DAG: @sa_const3 = {{.*}}global i16 192, align 2
+short _Accum sa_const4 = 0.5lr * 4.0hk; // CHECK-DAG: @sa_const4 = {{.*}}global i16 256, align 2
+
+// Unsigned multiplication
+unsigned short _Accum usa_const = 1.0uhk * 2.0uhk;
+// CHECK-SIGNED-DAG: @usa_const = {{.*}}global i16 768, align 2
+// CHECK-UNSIGNED-DAG: @usa_const = {{.*}}global i16 384, align 2
+
+// Unsigned * signed
+short _Accum sa_const5 = 20.0uhk * 3.0hk;
+// CHECK-DAG: @sa_const5 = {{.*}}global i16 7680, align 2
+
+// Multiplication with negative number
+short _Accum sa_const6 = 0.5hr * (-2.0hk);
+// CHECK-DAG: @sa_const6 = {{.*}}global i16 -128, align 2
+
+// Int multiplication
+unsigned short _Accum usa_const2 = 5 * 10.5uhk;
+// CHECK-SIGNED-DAG: @usa_const2 = {{.*}}global i16 640, align 2
+// CHECK-UNSIGNED-DAG: @usa_const2 = {{.*}}global i16 320, align 2
+short _Accum sa_const7 = 3 * (-0.5hk); // CHECK-DAG: @sa_const7 = {{.*}}global i16 -192, align 2
+short _Accum sa_const8 = 100 * (-2.0hk); // CHECK-DAG: @sa_const8 = {{.*}}global i16 -25600, align 2
+long _Fract lf_const = -0.25lr * 3; // CHECK-DAG: @lf_const = {{.*}}global i32 -1610612736, align 4
+
+// Saturated multiplication
+_Sat short _Accum sat_sa_const = (_Sat short _Accum)128.0hk * 3.0hk;
+// CHECK-DAG: @sat_sa_const = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const = (_Sat unsigned short _Accum)128.0uhk * 128.0uhk;
+// CHECK-SIGNED-DAG: @sat_usa_const = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const = {{.*}}global i16 32767, align 2
+_Sat short _Accum sat_sa_const2 = (_Sat short _Accum)128.0hk * -128;
+// CHECK-DAG: @sat_sa_const2 = {{.*}}global i16 -32768, align 2
+_Sat unsigned short _Accum sat_usa_const2 = (_Sat unsigned short _Accum)128.0uhk * 30;
+// CHECK-SIGNED-DAG: @sat_usa_const2 = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const2 = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const3 = (_Sat unsigned short _Accum)0.5uhk * (-2);
+// CHECK-DAG: @sat_usa_const3 = {{.*}}global i16 0, align 2
+
void SignedMultiplication() {
// CHECK-LABEL: SignedMultiplication
short _Accum sa;
Index: clang/lib/Basic/FixedPoint.cpp
===
--- clang/lib/Basic/FixedPoint.cpp
+++ clang/lib/Basic/FixedPoint.cpp
@@ -197,6 +197,63 @@
return APFixedPoint(Result, CommonFXSema);
}
+APFixedPoint APFixedPoint::mul(const APFixedPoint ,
+ bool *Overflow) const {
+ auto CommonFXSema = Sema.getCommonSemantics(Other.getSemantics());
+ APFixedPoint ConvertedThis = convert(CommonFXSema);
+ APFixedPoint ConvertedOther = Other.convert(CommonFXSema);
+ llvm::APSInt ThisVal = ConvertedThis.getValue();
+ llvm::APSInt OtherVal = ConvertedOther.getValue();
+ bool Overflowed = false;
+
+ // Widen the LHS and RHS so we can perform a full multiplication.
+ unsigned Wide = CommonFXSema.getWidth() * 2;
+ if (CommonFXSema.isSigned()) {
+ThisVal = ThisVal.sextOrSelf(Wide);
+OtherVal = OtherVal.sextOrSelf(Wide);
+ } else {
+ThisVal = ThisVal.zextOrSelf(Wide);
+OtherVal = OtherVal.zextOrSelf(Wide);
+ }
+
+ // Perform the full multiplication and downscale to get the same scale.
+ //
+ // Note that the right shifts here perform an implicit downwards rounding.
+ // This rounding could discard bits that would technically place the result
+ // outside the representable range. We interpret the
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
leonardchan accepted this revision. leonardchan added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73186/new/ https://reviews.llvm.org/D73186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
ebevhan added a comment. The last patchset contains the comment about rounding, so I think I will consider this accepted. As a final addendum to the discussion on rounding and overflow... The last Appendix to the E-C TR does actually say: 2. In the first edition requires that overflow handling is done before rounding; for the second edition the order is changed: rounding should be done first, followed by overflow handling. Note that this change does not affect any result when the overflow mode is saturation. The wording in the main text could be a bit clearer about it being explicit, though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73186/new/ https://reviews.llvm.org/D73186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
ebevhan updated this revision to Diff 255976.
ebevhan added a comment.
Rebased.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73186/new/
https://reviews.llvm.org/D73186
Files:
clang/include/clang/Basic/FixedPoint.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Basic/FixedPoint.cpp
clang/test/Frontend/fixed_point_mul.c
Index: clang/test/Frontend/fixed_point_mul.c
===
--- clang/test/Frontend/fixed_point_mul.c
+++ clang/test/Frontend/fixed_point_mul.c
@@ -1,6 +1,49 @@
// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+// Multiplication between different fixed point types
+short _Accum sa_const = 2.0hk * 2.0hk; // CHECK-DAG: @sa_const = {{.*}}global i16 512, align 2
+_Accum a_const = 3.0hk * 2.0k; // CHECK-DAG: @a_const = {{.*}}global i32 196608, align 4
+long _Accum la_const = 4.0hk * 2.0lk; // CHECK-DAG: @la_const = {{.*}}global i64 17179869184, align 8
+short _Accum sa_const2 = 0.5hr * 2.0hk; // CHECK-DAG: @sa_const2 = {{.*}}global i16 128, align 2
+short _Accum sa_const3 = 0.5r * 3.0hk; // CHECK-DAG: @sa_const3 = {{.*}}global i16 192, align 2
+short _Accum sa_const4 = 0.5lr * 4.0hk; // CHECK-DAG: @sa_const4 = {{.*}}global i16 256, align 2
+
+// Unsigned multiplication
+unsigned short _Accum usa_const = 1.0uhk * 2.0uhk;
+// CHECK-SIGNED-DAG: @usa_const = {{.*}}global i16 768, align 2
+// CHECK-UNSIGNED-DAG: @usa_const = {{.*}}global i16 384, align 2
+
+// Unsigned * signed
+short _Accum sa_const5 = 20.0uhk * 3.0hk;
+// CHECK-DAG: @sa_const5 = {{.*}}global i16 7680, align 2
+
+// Multiplication with negative number
+short _Accum sa_const6 = 0.5hr * (-2.0hk);
+// CHECK-DAG: @sa_const6 = {{.*}}global i16 -128, align 2
+
+// Int multiplication
+unsigned short _Accum usa_const2 = 5 * 10.5uhk;
+// CHECK-SIGNED-DAG: @usa_const2 = {{.*}}global i16 640, align 2
+// CHECK-UNSIGNED-DAG: @usa_const2 = {{.*}}global i16 320, align 2
+short _Accum sa_const7 = 3 * (-0.5hk); // CHECK-DAG: @sa_const7 = {{.*}}global i16 -192, align 2
+short _Accum sa_const8 = 100 * (-2.0hk); // CHECK-DAG: @sa_const8 = {{.*}}global i16 -25600, align 2
+long _Fract lf_const = -0.25lr * 3; // CHECK-DAG: @lf_const = {{.*}}global i32 -1610612736, align 4
+
+// Saturated multiplication
+_Sat short _Accum sat_sa_const = (_Sat short _Accum)128.0hk * 3.0hk;
+// CHECK-DAG: @sat_sa_const = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const = (_Sat unsigned short _Accum)128.0uhk * 128.0uhk;
+// CHECK-SIGNED-DAG: @sat_usa_const = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const = {{.*}}global i16 32767, align 2
+_Sat short _Accum sat_sa_const2 = (_Sat short _Accum)128.0hk * -128;
+// CHECK-DAG: @sat_sa_const2 = {{.*}}global i16 -32768, align 2
+_Sat unsigned short _Accum sat_usa_const2 = (_Sat unsigned short _Accum)128.0uhk * 30;
+// CHECK-SIGNED-DAG: @sat_usa_const2 = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const2 = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const3 = (_Sat unsigned short _Accum)0.5uhk * (-2);
+// CHECK-DAG: @sat_usa_const3 = {{.*}}global i16 0, align 2
+
void SignedMultiplication() {
// CHECK-LABEL: SignedMultiplication
short _Accum sa;
Index: clang/lib/Basic/FixedPoint.cpp
===
--- clang/lib/Basic/FixedPoint.cpp
+++ clang/lib/Basic/FixedPoint.cpp
@@ -197,6 +197,63 @@
return APFixedPoint(Result, CommonFXSema);
}
+APFixedPoint APFixedPoint::mul(const APFixedPoint ,
+ bool *Overflow) const {
+ auto CommonFXSema = Sema.getCommonSemantics(Other.getSemantics());
+ APFixedPoint ConvertedThis = convert(CommonFXSema);
+ APFixedPoint ConvertedOther = Other.convert(CommonFXSema);
+ llvm::APSInt ThisVal = ConvertedThis.getValue();
+ llvm::APSInt OtherVal = ConvertedOther.getValue();
+ bool Overflowed = false;
+
+ // Widen the LHS and RHS so we can perform a full multiplication.
+ unsigned Wide = CommonFXSema.getWidth() * 2;
+ if (CommonFXSema.isSigned()) {
+ThisVal = ThisVal.sextOrSelf(Wide);
+OtherVal = OtherVal.sextOrSelf(Wide);
+ } else {
+ThisVal = ThisVal.zextOrSelf(Wide);
+OtherVal = OtherVal.zextOrSelf(Wide);
+ }
+
+ // Perform the full multiplication and downscale to get the same scale.
+ //
+ // Note that the right shifts here perform an implicit downwards rounding.
+ // This rounding could discard bits that would technically place the result
+ // outside the representable range. We interpret the spec as allowing us to
+ // perform the rounding step first, avoiding the overflow case that would
+ //
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
rjmccall added a comment. Since we've settled on not considering that to be overflow, yeah, I think the patch is fine. Might be worth being explicit about that at the point you do the shift: that it's known that this discards precision that could leave the true mathematical value outside of the expressible range, and that we are interpreting the spec as allowing us to round to avoid this formal overflow in order to avoid unnecessary UB. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73186/new/ https://reviews.llvm.org/D73186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
ebevhan added a comment. So, any more on this or are we in agreement? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73186/new/ https://reviews.llvm.org/D73186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
ebevhan added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + rjmccall wrote: > leonardchan wrote: > > ebevhan wrote: > > > rjmccall wrote: > > > > ebevhan wrote: > > > > > rjmccall wrote: > > > > > > leonardchan wrote: > > > > > > > ebevhan wrote: > > > > > > > > rjmccall wrote: > > > > > > > > > ebevhan wrote: > > > > > > > > > > ebevhan wrote: > > > > > > > > > > > rjmccall wrote: > > > > > > > > > > > > If the maximum expressible value is *k*, and the > > > > > > > > > > > > fully-precise multiplication yields *k+e* for some > > > > > > > > > > > > epsilon *e* that isn't representable in the result > > > > > > > > > > > > semantics, is that considered an overflow? If so, I > > > > > > > > > > > > think you need to do the shift after these bound > > > > > > > > > > > > checks, since the shift destroys the difference between > > > > > > > > > > > > *k* and *k+e*. That is, unless there's a compelling > > > > > > > > > > > > mathematical argument that it's not possible to > > > > > > > > > > > > overflow only in the fully-precision multiplication — > > > > > > > > > > > > but while I think that's possibly true of `_Fract` > > > > > > > > > > > > (since *k^2 < k*), it seems unlikely to be true of > > > > > > > > > > > > `_Accum`, although I haven't looked for a > > > > > > > > > > > > counter-example. And if there is a compelling > > > > > > > > > > > > argument, it should probably be at least alluded to in > > > > > > > > > > > > a comment. > > > > > > > > > > > > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage > > > > > > > > > > > > of the fact that `APFixedPointSemantics` doesn't have > > > > > > > > > > > > to correspond to a real type? You could probably just > > > > > > > > > > > > convert to a double-width common semantics, right? > > > > > > > > > > > > If the maximum expressible value is *k*, and the > > > > > > > > > > > > fully-precise multiplication yields *k+e* for some > > > > > > > > > > > > epsilon *e* that isn't representable in the result > > > > > > > > > > > > semantics, is that considered an overflow? If so, I > > > > > > > > > > > > think you need to do the shift after these bound > > > > > > > > > > > > checks, since the shift destroys the difference between > > > > > > > > > > > > *k* and *k+e*. > > > > > > > > > > > > > > > > > > > > > > I don't think I would consider that to be overflow; > > > > > > > > > > > that's precision loss. E-C considers these to be > > > > > > > > > > > different: > > > > > > > > > > > > > > > > > > > > > > > If the source value cannot be represented exactly by > > > > > > > > > > > > the fixed-point type, the source value is rounded to > > > > > > > > > > > > either the closest fixed-point value greater than the > > > > > > > > > > > > source value (rounded up) or to the closest fixed-point > > > > > > > > > > > > value less than the source value (rounded down). > > > > > > > > > > > > > > > > > > > > > > > > When the source value does not fit within the range of > > > > > > > > > > > > the fixed-point type, the conversion overflows. [...] > > > > > > > > > > > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > > > > > > > > > > If the result type of an arithmetic operation is a > > > > > > > > > > > > fixed-point type, [...] the calculated result is the > > > > > > > > > > > > mathematically exact result with overflow handling and > > > > > > > > > > > > rounding performed to the full precision of the result > > > > > > > > > > > > type as explained in 4.1.3. > > > > > > > > > > > > > > > > > > > > > > There is also no value of `e` that would affect > > > > > > > > > > > saturation. Any full precision calculation that gives > > > > > > > > > > > `k+e` must be `k` after downscaling, since the bits that > > > > > > > > > > > represent `e` must come from the extra precision range. > > > > > > > > > > > Even though `k+e` is technically larger than `k`, > > > > > > > > > > > saturation would still just give us `k` after truncating > > > > > > > > > > > out `e`, so the end result is the same. > > > > > > > > > > > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage > > > > > > > > > > > > of the fact that APFixedPointSemantics doesn't have to > > > > > > > > > > > > correspond to a real type? You could probably just > > > > > > > > > > > > convert to a double-width common semantics, right? > > > > > > > > > > > > > > > > > > > > > > It's likely possible to use APFixedPoint in the > > > > > > > > > > > calculations here, but I used APInt to make the behavior > > > > > > > > > > > explicit and not accidentally be dependent on the > > > > > > > > > > > behavior of APFixedPoint's conversions or operations. > > > > > > > > > > Although.,. I guess I see your point in that an > > > > > > > > > > intermediate result of k+e
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
rjmccall added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + leonardchan wrote: > ebevhan wrote: > > rjmccall wrote: > > > ebevhan wrote: > > > > rjmccall wrote: > > > > > leonardchan wrote: > > > > > > ebevhan wrote: > > > > > > > rjmccall wrote: > > > > > > > > ebevhan wrote: > > > > > > > > > ebevhan wrote: > > > > > > > > > > rjmccall wrote: > > > > > > > > > > > If the maximum expressible value is *k*, and the > > > > > > > > > > > fully-precise multiplication yields *k+e* for some > > > > > > > > > > > epsilon *e* that isn't representable in the result > > > > > > > > > > > semantics, is that considered an overflow? If so, I > > > > > > > > > > > think you need to do the shift after these bound checks, > > > > > > > > > > > since the shift destroys the difference between *k* and > > > > > > > > > > > *k+e*. That is, unless there's a compelling mathematical > > > > > > > > > > > argument that it's not possible to overflow only in the > > > > > > > > > > > fully-precision multiplication — but while I think that's > > > > > > > > > > > possibly true of `_Fract` (since *k^2 < k*), it seems > > > > > > > > > > > unlikely to be true of `_Accum`, although I haven't > > > > > > > > > > > looked for a counter-example. And if there is a > > > > > > > > > > > compelling argument, it should probably be at least > > > > > > > > > > > alluded to in a comment. > > > > > > > > > > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage of > > > > > > > > > > > the fact that `APFixedPointSemantics` doesn't have to > > > > > > > > > > > correspond to a real type? You could probably just > > > > > > > > > > > convert to a double-width common semantics, right? > > > > > > > > > > > If the maximum expressible value is *k*, and the > > > > > > > > > > > fully-precise multiplication yields *k+e* for some > > > > > > > > > > > epsilon *e* that isn't representable in the result > > > > > > > > > > > semantics, is that considered an overflow? If so, I think > > > > > > > > > > > you need to do the shift after these bound checks, since > > > > > > > > > > > the shift destroys the difference between *k* and *k+e*. > > > > > > > > > > > > > > > > > > > > I don't think I would consider that to be overflow; that's > > > > > > > > > > precision loss. E-C considers these to be different: > > > > > > > > > > > > > > > > > > > > > If the source value cannot be represented exactly by the > > > > > > > > > > > fixed-point type, the source value is rounded to either > > > > > > > > > > > the closest fixed-point value greater than the source > > > > > > > > > > > value (rounded up) or to the closest fixed-point value > > > > > > > > > > > less than the source value (rounded down). > > > > > > > > > > > > > > > > > > > > > > When the source value does not fit within the range of > > > > > > > > > > > the fixed-point type, the conversion overflows. [...] > > > > > > > > > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > > > > > > > > If the result type of an arithmetic operation is a > > > > > > > > > > > fixed-point type, [...] the calculated result is the > > > > > > > > > > > mathematically exact result with overflow handling and > > > > > > > > > > > rounding performed to the full precision of the result > > > > > > > > > > > type as explained in 4.1.3. > > > > > > > > > > > > > > > > > > > > There is also no value of `e` that would affect saturation. > > > > > > > > > > Any full precision calculation that gives `k+e` must be `k` > > > > > > > > > > after downscaling, since the bits that represent `e` must > > > > > > > > > > come from the extra precision range. Even though `k+e` is > > > > > > > > > > technically larger than `k`, saturation would still just > > > > > > > > > > give us `k` after truncating out `e`, so the end result is > > > > > > > > > > the same. > > > > > > > > > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage of > > > > > > > > > > > the fact that APFixedPointSemantics doesn't have to > > > > > > > > > > > correspond to a real type? You could probably just > > > > > > > > > > > convert to a double-width common semantics, right? > > > > > > > > > > > > > > > > > > > > It's likely possible to use APFixedPoint in the > > > > > > > > > > calculations here, but I used APInt to make the behavior > > > > > > > > > > explicit and not accidentally be dependent on the behavior > > > > > > > > > > of APFixedPoint's conversions or operations. > > > > > > > > > Although.,. I guess I see your point in that an intermediate > > > > > > > > > result of k+e technically "does not fit within the range of > > > > > > > > > the fixed-point type"... but I wonder if treating such cases > > > > > > > > > as overflow is particularly meaningful. I don't find there to > > > > > > > > > be much of a distinction
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
leonardchan added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + ebevhan wrote: > rjmccall wrote: > > ebevhan wrote: > > > rjmccall wrote: > > > > leonardchan wrote: > > > > > ebevhan wrote: > > > > > > rjmccall wrote: > > > > > > > ebevhan wrote: > > > > > > > > ebevhan wrote: > > > > > > > > > rjmccall wrote: > > > > > > > > > > If the maximum expressible value is *k*, and the > > > > > > > > > > fully-precise multiplication yields *k+e* for some epsilon > > > > > > > > > > *e* that isn't representable in the result semantics, is > > > > > > > > > > that considered an overflow? If so, I think you need to do > > > > > > > > > > the shift after these bound checks, since the shift > > > > > > > > > > destroys the difference between *k* and *k+e*. That is, > > > > > > > > > > unless there's a compelling mathematical argument that it's > > > > > > > > > > not possible to overflow only in the fully-precision > > > > > > > > > > multiplication — but while I think that's possibly true of > > > > > > > > > > `_Fract` (since *k^2 < k*), it seems unlikely to be true of > > > > > > > > > > `_Accum`, although I haven't looked for a counter-example. > > > > > > > > > > And if there is a compelling argument, it should probably > > > > > > > > > > be at least alluded to in a comment. > > > > > > > > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage of > > > > > > > > > > the fact that `APFixedPointSemantics` doesn't have to > > > > > > > > > > correspond to a real type? You could probably just convert > > > > > > > > > > to a double-width common semantics, right? > > > > > > > > > > If the maximum expressible value is *k*, and the > > > > > > > > > > fully-precise multiplication yields *k+e* for some epsilon > > > > > > > > > > *e* that isn't representable in the result semantics, is > > > > > > > > > > that considered an overflow? If so, I think you need to do > > > > > > > > > > the shift after these bound checks, since the shift > > > > > > > > > > destroys the difference between *k* and *k+e*. > > > > > > > > > > > > > > > > > > I don't think I would consider that to be overflow; that's > > > > > > > > > precision loss. E-C considers these to be different: > > > > > > > > > > > > > > > > > > > If the source value cannot be represented exactly by the > > > > > > > > > > fixed-point type, the source value is rounded to either the > > > > > > > > > > closest fixed-point value greater than the source value > > > > > > > > > > (rounded up) or to the closest fixed-point value less than > > > > > > > > > > the source value (rounded down). > > > > > > > > > > > > > > > > > > > > When the source value does not fit within the range of the > > > > > > > > > > fixed-point type, the conversion overflows. [...] > > > > > > > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > > > > > > If the result type of an arithmetic operation is a > > > > > > > > > > fixed-point type, [...] the calculated result is the > > > > > > > > > > mathematically exact result with overflow handling and > > > > > > > > > > rounding performed to the full precision of the result type > > > > > > > > > > as explained in 4.1.3. > > > > > > > > > > > > > > > > > > There is also no value of `e` that would affect saturation. > > > > > > > > > Any full precision calculation that gives `k+e` must be `k` > > > > > > > > > after downscaling, since the bits that represent `e` must > > > > > > > > > come from the extra precision range. Even though `k+e` is > > > > > > > > > technically larger than `k`, saturation would still just give > > > > > > > > > us `k` after truncating out `e`, so the end result is the > > > > > > > > > same. > > > > > > > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage of > > > > > > > > > > the fact that APFixedPointSemantics doesn't have to > > > > > > > > > > correspond to a real type? You could probably just convert > > > > > > > > > > to a double-width common semantics, right? > > > > > > > > > > > > > > > > > > It's likely possible to use APFixedPoint in the calculations > > > > > > > > > here, but I used APInt to make the behavior explicit and not > > > > > > > > > accidentally be dependent on the behavior of APFixedPoint's > > > > > > > > > conversions or operations. > > > > > > > > Although.,. I guess I see your point in that an intermediate > > > > > > > > result of k+e technically "does not fit within the range of the > > > > > > > > fixed-point type"... but I wonder if treating such cases as > > > > > > > > overflow is particularly meaningful. I don't find there to be > > > > > > > > much of a distinction between such a case and the case where > > > > > > > > the exact result lands inbetween two representable values. We > > > > > > > > just end up with a less precise result. > > > > > > >
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
ebevhan added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + rjmccall wrote: > ebevhan wrote: > > rjmccall wrote: > > > leonardchan wrote: > > > > ebevhan wrote: > > > > > rjmccall wrote: > > > > > > ebevhan wrote: > > > > > > > ebevhan wrote: > > > > > > > > rjmccall wrote: > > > > > > > > > If the maximum expressible value is *k*, and the > > > > > > > > > fully-precise multiplication yields *k+e* for some epsilon > > > > > > > > > *e* that isn't representable in the result semantics, is that > > > > > > > > > considered an overflow? If so, I think you need to do the > > > > > > > > > shift after these bound checks, since the shift destroys the > > > > > > > > > difference between *k* and *k+e*. That is, unless there's a > > > > > > > > > compelling mathematical argument that it's not possible to > > > > > > > > > overflow only in the fully-precision multiplication — but > > > > > > > > > while I think that's possibly true of `_Fract` (since *k^2 < > > > > > > > > > k*), it seems unlikely to be true of `_Accum`, although I > > > > > > > > > haven't looked for a counter-example. And if there is a > > > > > > > > > compelling argument, it should probably be at least alluded > > > > > > > > > to in a comment. > > > > > > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage of the > > > > > > > > > fact that `APFixedPointSemantics` doesn't have to correspond > > > > > > > > > to a real type? You could probably just convert to a > > > > > > > > > double-width common semantics, right? > > > > > > > > > If the maximum expressible value is *k*, and the > > > > > > > > > fully-precise multiplication yields *k+e* for some epsilon > > > > > > > > > *e* that isn't representable in the result semantics, is that > > > > > > > > > considered an overflow? If so, I think you need to do the > > > > > > > > > shift after these bound checks, since the shift destroys the > > > > > > > > > difference between *k* and *k+e*. > > > > > > > > > > > > > > > > I don't think I would consider that to be overflow; that's > > > > > > > > precision loss. E-C considers these to be different: > > > > > > > > > > > > > > > > > If the source value cannot be represented exactly by the > > > > > > > > > fixed-point type, the source value is rounded to either the > > > > > > > > > closest fixed-point value greater than the source value > > > > > > > > > (rounded up) or to the closest fixed-point value less than > > > > > > > > > the source value (rounded down). > > > > > > > > > > > > > > > > > > When the source value does not fit within the range of the > > > > > > > > > fixed-point type, the conversion overflows. [...] > > > > > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > > > > If the result type of an arithmetic operation is a > > > > > > > > > fixed-point type, [...] the calculated result is the > > > > > > > > > mathematically exact result with overflow handling and > > > > > > > > > rounding performed to the full precision of the result type > > > > > > > > > as explained in 4.1.3. > > > > > > > > > > > > > > > > There is also no value of `e` that would affect saturation. Any > > > > > > > > full precision calculation that gives `k+e` must be `k` after > > > > > > > > downscaling, since the bits that represent `e` must come from > > > > > > > > the extra precision range. Even though `k+e` is technically > > > > > > > > larger than `k`, saturation would still just give us `k` after > > > > > > > > truncating out `e`, so the end result is the same. > > > > > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage of the > > > > > > > > > fact that APFixedPointSemantics doesn't have to correspond to > > > > > > > > > a real type? You could probably just convert to a > > > > > > > > > double-width common semantics, right? > > > > > > > > > > > > > > > > It's likely possible to use APFixedPoint in the calculations > > > > > > > > here, but I used APInt to make the behavior explicit and not > > > > > > > > accidentally be dependent on the behavior of APFixedPoint's > > > > > > > > conversions or operations. > > > > > > > Although.,. I guess I see your point in that an intermediate > > > > > > > result of k+e technically "does not fit within the range of the > > > > > > > fixed-point type"... but I wonder if treating such cases as > > > > > > > overflow is particularly meaningful. I don't find there to be > > > > > > > much of a distinction between such a case and the case where the > > > > > > > exact result lands inbetween two representable values. We just > > > > > > > end up with a less precise result. > > > > > > Right, I was wondering if there was an accepted answer here. For > > > > > > saturating arithmetic, it's equivalent to truncate this extra > > > > > > precision down to *k* or to saturate to the
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
rjmccall added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + ebevhan wrote: > rjmccall wrote: > > leonardchan wrote: > > > ebevhan wrote: > > > > rjmccall wrote: > > > > > ebevhan wrote: > > > > > > ebevhan wrote: > > > > > > > rjmccall wrote: > > > > > > > > If the maximum expressible value is *k*, and the fully-precise > > > > > > > > multiplication yields *k+e* for some epsilon *e* that isn't > > > > > > > > representable in the result semantics, is that considered an > > > > > > > > overflow? If so, I think you need to do the shift after these > > > > > > > > bound checks, since the shift destroys the difference between > > > > > > > > *k* and *k+e*. That is, unless there's a compelling > > > > > > > > mathematical argument that it's not possible to overflow only > > > > > > > > in the fully-precision multiplication — but while I think > > > > > > > > that's possibly true of `_Fract` (since *k^2 < k*), it seems > > > > > > > > unlikely to be true of `_Accum`, although I haven't looked for > > > > > > > > a counter-example. And if there is a compelling argument, it > > > > > > > > should probably be at least alluded to in a comment. > > > > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage of the > > > > > > > > fact that `APFixedPointSemantics` doesn't have to correspond to > > > > > > > > a real type? You could probably just convert to a double-width > > > > > > > > common semantics, right? > > > > > > > > If the maximum expressible value is *k*, and the fully-precise > > > > > > > > multiplication yields *k+e* for some epsilon *e* that isn't > > > > > > > > representable in the result semantics, is that considered an > > > > > > > > overflow? If so, I think you need to do the shift after these > > > > > > > > bound checks, since the shift destroys the difference between > > > > > > > > *k* and *k+e*. > > > > > > > > > > > > > > I don't think I would consider that to be overflow; that's > > > > > > > precision loss. E-C considers these to be different: > > > > > > > > > > > > > > > If the source value cannot be represented exactly by the > > > > > > > > fixed-point type, the source value is rounded to either the > > > > > > > > closest fixed-point value greater than the source value > > > > > > > > (rounded up) or to the closest fixed-point value less than the > > > > > > > > source value (rounded down). > > > > > > > > > > > > > > > > When the source value does not fit within the range of the > > > > > > > > fixed-point type, the conversion overflows. [...] > > > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > > If the result type of an arithmetic operation is a fixed-point > > > > > > > > type, [...] the calculated result is the mathematically exact > > > > > > > > result with overflow handling and rounding performed to the > > > > > > > > full precision of the result type as explained in 4.1.3. > > > > > > > > > > > > > > There is also no value of `e` that would affect saturation. Any > > > > > > > full precision calculation that gives `k+e` must be `k` after > > > > > > > downscaling, since the bits that represent `e` must come from the > > > > > > > extra precision range. Even though `k+e` is technically larger > > > > > > > than `k`, saturation would still just give us `k` after > > > > > > > truncating out `e`, so the end result is the same. > > > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage of the > > > > > > > > fact that APFixedPointSemantics doesn't have to correspond to a > > > > > > > > real type? You could probably just convert to a double-width > > > > > > > > common semantics, right? > > > > > > > > > > > > > > It's likely possible to use APFixedPoint in the calculations > > > > > > > here, but I used APInt to make the behavior explicit and not > > > > > > > accidentally be dependent on the behavior of APFixedPoint's > > > > > > > conversions or operations. > > > > > > Although.,. I guess I see your point in that an intermediate result > > > > > > of k+e technically "does not fit within the range of the > > > > > > fixed-point type"... but I wonder if treating such cases as > > > > > > overflow is particularly meaningful. I don't find there to be much > > > > > > of a distinction between such a case and the case where the exact > > > > > > result lands inbetween two representable values. We just end up > > > > > > with a less precise result. > > > > > Right, I was wondering if there was an accepted answer here. For > > > > > saturating arithmetic, it's equivalent to truncate this extra > > > > > precision down to *k* or to saturate to the maximum representable > > > > > value, since by assumption that was just *k*; but for non-saturating > > > > > arithmetic, it affects whether the operation has UB. All else being > > > > > the same,
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
ebevhan added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + rjmccall wrote: > leonardchan wrote: > > ebevhan wrote: > > > rjmccall wrote: > > > > ebevhan wrote: > > > > > ebevhan wrote: > > > > > > rjmccall wrote: > > > > > > > If the maximum expressible value is *k*, and the fully-precise > > > > > > > multiplication yields *k+e* for some epsilon *e* that isn't > > > > > > > representable in the result semantics, is that considered an > > > > > > > overflow? If so, I think you need to do the shift after these > > > > > > > bound checks, since the shift destroys the difference between *k* > > > > > > > and *k+e*. That is, unless there's a compelling mathematical > > > > > > > argument that it's not possible to overflow only in the > > > > > > > fully-precision multiplication — but while I think that's > > > > > > > possibly true of `_Fract` (since *k^2 < k*), it seems unlikely to > > > > > > > be true of `_Accum`, although I haven't looked for a > > > > > > > counter-example. And if there is a compelling argument, it > > > > > > > should probably be at least alluded to in a comment. > > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage of the fact > > > > > > > that `APFixedPointSemantics` doesn't have to correspond to a real > > > > > > > type? You could probably just convert to a double-width common > > > > > > > semantics, right? > > > > > > > If the maximum expressible value is *k*, and the fully-precise > > > > > > > multiplication yields *k+e* for some epsilon *e* that isn't > > > > > > > representable in the result semantics, is that considered an > > > > > > > overflow? If so, I think you need to do the shift after these > > > > > > > bound checks, since the shift destroys the difference between *k* > > > > > > > and *k+e*. > > > > > > > > > > > > I don't think I would consider that to be overflow; that's > > > > > > precision loss. E-C considers these to be different: > > > > > > > > > > > > > If the source value cannot be represented exactly by the > > > > > > > fixed-point type, the source value is rounded to either the > > > > > > > closest fixed-point value greater than the source value (rounded > > > > > > > up) or to the closest fixed-point value less than the source > > > > > > > value (rounded down). > > > > > > > > > > > > > > When the source value does not fit within the range of the > > > > > > > fixed-point type, the conversion overflows. [...] > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > If the result type of an arithmetic operation is a fixed-point > > > > > > > type, [...] the calculated result is the mathematically exact > > > > > > > result with overflow handling and rounding performed to the full > > > > > > > precision of the result type as explained in 4.1.3. > > > > > > > > > > > > There is also no value of `e` that would affect saturation. Any > > > > > > full precision calculation that gives `k+e` must be `k` after > > > > > > downscaling, since the bits that represent `e` must come from the > > > > > > extra precision range. Even though `k+e` is technically larger than > > > > > > `k`, saturation would still just give us `k` after truncating out > > > > > > `e`, so the end result is the same. > > > > > > > > > > > > > Would this algorithm be simpler if you took advantage of the fact > > > > > > > that APFixedPointSemantics doesn't have to correspond to a real > > > > > > > type? You could probably just convert to a double-width common > > > > > > > semantics, right? > > > > > > > > > > > > It's likely possible to use APFixedPoint in the calculations here, > > > > > > but I used APInt to make the behavior explicit and not accidentally > > > > > > be dependent on the behavior of APFixedPoint's conversions or > > > > > > operations. > > > > > Although.,. I guess I see your point in that an intermediate result > > > > > of k+e technically "does not fit within the range of the fixed-point > > > > > type"... but I wonder if treating such cases as overflow is > > > > > particularly meaningful. I don't find there to be much of a > > > > > distinction between such a case and the case where the exact result > > > > > lands inbetween two representable values. We just end up with a less > > > > > precise result. > > > > Right, I was wondering if there was an accepted answer here. For > > > > saturating arithmetic, it's equivalent to truncate this extra precision > > > > down to *k* or to saturate to the maximum representable value, since by > > > > assumption that was just *k*; but for non-saturating arithmetic, it > > > > affects whether the operation has UB. All else being the same, it's > > > > better to have fewer corner-case sources of UB. > > > > > > > > My read is that Embedded C is saying there's a sequence here: compute > > > > the exact mathematical
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
rjmccall added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + leonardchan wrote: > ebevhan wrote: > > rjmccall wrote: > > > ebevhan wrote: > > > > ebevhan wrote: > > > > > rjmccall wrote: > > > > > > If the maximum expressible value is *k*, and the fully-precise > > > > > > multiplication yields *k+e* for some epsilon *e* that isn't > > > > > > representable in the result semantics, is that considered an > > > > > > overflow? If so, I think you need to do the shift after these > > > > > > bound checks, since the shift destroys the difference between *k* > > > > > > and *k+e*. That is, unless there's a compelling mathematical > > > > > > argument that it's not possible to overflow only in the > > > > > > fully-precision multiplication — but while I think that's possibly > > > > > > true of `_Fract` (since *k^2 < k*), it seems unlikely to be true of > > > > > > `_Accum`, although I haven't looked for a counter-example. And if > > > > > > there is a compelling argument, it should probably be at least > > > > > > alluded to in a comment. > > > > > > > > > > > > Would this algorithm be simpler if you took advantage of the fact > > > > > > that `APFixedPointSemantics` doesn't have to correspond to a real > > > > > > type? You could probably just convert to a double-width common > > > > > > semantics, right? > > > > > > If the maximum expressible value is *k*, and the fully-precise > > > > > > multiplication yields *k+e* for some epsilon *e* that isn't > > > > > > representable in the result semantics, is that considered an > > > > > > overflow? If so, I think you need to do the shift after these bound > > > > > > checks, since the shift destroys the difference between *k* and > > > > > > *k+e*. > > > > > > > > > > I don't think I would consider that to be overflow; that's precision > > > > > loss. E-C considers these to be different: > > > > > > > > > > > If the source value cannot be represented exactly by the > > > > > > fixed-point type, the source value is rounded to either the closest > > > > > > fixed-point value greater than the source value (rounded up) or to > > > > > > the closest fixed-point value less than the source value (rounded > > > > > > down). > > > > > > > > > > > > When the source value does not fit within the range of the > > > > > > fixed-point type, the conversion overflows. [...] > > > > > > > > > > > > [...] > > > > > > > > > > > > If the result type of an arithmetic operation is a fixed-point > > > > > > type, [...] the calculated result is the mathematically exact > > > > > > result with overflow handling and rounding performed to the full > > > > > > precision of the result type as explained in 4.1.3. > > > > > > > > > > There is also no value of `e` that would affect saturation. Any full > > > > > precision calculation that gives `k+e` must be `k` after downscaling, > > > > > since the bits that represent `e` must come from the extra precision > > > > > range. Even though `k+e` is technically larger than `k`, saturation > > > > > would still just give us `k` after truncating out `e`, so the end > > > > > result is the same. > > > > > > > > > > > Would this algorithm be simpler if you took advantage of the fact > > > > > > that APFixedPointSemantics doesn't have to correspond to a real > > > > > > type? You could probably just convert to a double-width common > > > > > > semantics, right? > > > > > > > > > > It's likely possible to use APFixedPoint in the calculations here, > > > > > but I used APInt to make the behavior explicit and not accidentally > > > > > be dependent on the behavior of APFixedPoint's conversions or > > > > > operations. > > > > Although.,. I guess I see your point in that an intermediate result of > > > > k+e technically "does not fit within the range of the fixed-point > > > > type"... but I wonder if treating such cases as overflow is > > > > particularly meaningful. I don't find there to be much of a distinction > > > > between such a case and the case where the exact result lands inbetween > > > > two representable values. We just end up with a less precise result. > > > Right, I was wondering if there was an accepted answer here. For > > > saturating arithmetic, it's equivalent to truncate this extra precision > > > down to *k* or to saturate to the maximum representable value, since by > > > assumption that was just *k*; but for non-saturating arithmetic, it > > > affects whether the operation has UB. All else being the same, it's > > > better to have fewer corner-case sources of UB. > > > > > > My read is that Embedded C is saying there's a sequence here: compute the > > > exact mathematical result; round that to the precision of the result > > > type; the operation overflows if the rounded result is not representable > > > in the result type. Is the rounding direction
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
leonardchan added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + ebevhan wrote: > rjmccall wrote: > > ebevhan wrote: > > > ebevhan wrote: > > > > rjmccall wrote: > > > > > If the maximum expressible value is *k*, and the fully-precise > > > > > multiplication yields *k+e* for some epsilon *e* that isn't > > > > > representable in the result semantics, is that considered an > > > > > overflow? If so, I think you need to do the shift after these bound > > > > > checks, since the shift destroys the difference between *k* and > > > > > *k+e*. That is, unless there's a compelling mathematical argument > > > > > that it's not possible to overflow only in the fully-precision > > > > > multiplication — but while I think that's possibly true of `_Fract` > > > > > (since *k^2 < k*), it seems unlikely to be true of `_Accum`, although > > > > > I haven't looked for a counter-example. And if there is a compelling > > > > > argument, it should probably be at least alluded to in a comment. > > > > > > > > > > Would this algorithm be simpler if you took advantage of the fact > > > > > that `APFixedPointSemantics` doesn't have to correspond to a real > > > > > type? You could probably just convert to a double-width common > > > > > semantics, right? > > > > > If the maximum expressible value is *k*, and the fully-precise > > > > > multiplication yields *k+e* for some epsilon *e* that isn't > > > > > representable in the result semantics, is that considered an > > > > > overflow? If so, I think you need to do the shift after these bound > > > > > checks, since the shift destroys the difference between *k* and *k+e*. > > > > > > > > I don't think I would consider that to be overflow; that's precision > > > > loss. E-C considers these to be different: > > > > > > > > > If the source value cannot be represented exactly by the fixed-point > > > > > type, the source value is rounded to either the closest fixed-point > > > > > value greater than the source value (rounded up) or to the closest > > > > > fixed-point value less than the source value (rounded down). > > > > > > > > > > When the source value does not fit within the range of the > > > > > fixed-point type, the conversion overflows. [...] > > > > > > > > > > [...] > > > > > > > > > > If the result type of an arithmetic operation is a fixed-point type, > > > > > [...] the calculated result is the mathematically exact result with > > > > > overflow handling and rounding performed to the full precision of the > > > > > result type as explained in 4.1.3. > > > > > > > > There is also no value of `e` that would affect saturation. Any full > > > > precision calculation that gives `k+e` must be `k` after downscaling, > > > > since the bits that represent `e` must come from the extra precision > > > > range. Even though `k+e` is technically larger than `k`, saturation > > > > would still just give us `k` after truncating out `e`, so the end > > > > result is the same. > > > > > > > > > Would this algorithm be simpler if you took advantage of the fact > > > > > that APFixedPointSemantics doesn't have to correspond to a real type? > > > > > You could probably just convert to a double-width common semantics, > > > > > right? > > > > > > > > It's likely possible to use APFixedPoint in the calculations here, but > > > > I used APInt to make the behavior explicit and not accidentally be > > > > dependent on the behavior of APFixedPoint's conversions or operations. > > > Although.,. I guess I see your point in that an intermediate result of > > > k+e technically "does not fit within the range of the fixed-point > > > type"... but I wonder if treating such cases as overflow is particularly > > > meaningful. I don't find there to be much of a distinction between such a > > > case and the case where the exact result lands inbetween two > > > representable values. We just end up with a less precise result. > > Right, I was wondering if there was an accepted answer here. For > > saturating arithmetic, it's equivalent to truncate this extra precision > > down to *k* or to saturate to the maximum representable value, since by > > assumption that was just *k*; but for non-saturating arithmetic, it affects > > whether the operation has UB. All else being the same, it's better to have > > fewer corner-case sources of UB. > > > > My read is that Embedded C is saying there's a sequence here: compute the > > exact mathematical result; round that to the precision of the result type; > > the operation overflows if the rounded result is not representable in the > > result type. Is the rounding direction completely unspecified, down to > > being potentially operand-specific? If so, we could just say that we > > always round to avoid overflow if possible. The main consideration here > > is that we need to give the
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
ebevhan added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + rjmccall wrote: > ebevhan wrote: > > ebevhan wrote: > > > rjmccall wrote: > > > > If the maximum expressible value is *k*, and the fully-precise > > > > multiplication yields *k+e* for some epsilon *e* that isn't > > > > representable in the result semantics, is that considered an overflow? > > > > If so, I think you need to do the shift after these bound checks, since > > > > the shift destroys the difference between *k* and *k+e*. That is, > > > > unless there's a compelling mathematical argument that it's not > > > > possible to overflow only in the fully-precision multiplication — but > > > > while I think that's possibly true of `_Fract` (since *k^2 < k*), it > > > > seems unlikely to be true of `_Accum`, although I haven't looked for a > > > > counter-example. And if there is a compelling argument, it should > > > > probably be at least alluded to in a comment. > > > > > > > > Would this algorithm be simpler if you took advantage of the fact that > > > > `APFixedPointSemantics` doesn't have to correspond to a real type? You > > > > could probably just convert to a double-width common semantics, right? > > > > If the maximum expressible value is *k*, and the fully-precise > > > > multiplication yields *k+e* for some epsilon *e* that isn't > > > > representable in the result semantics, is that considered an overflow? > > > > If so, I think you need to do the shift after these bound checks, since > > > > the shift destroys the difference between *k* and *k+e*. > > > > > > I don't think I would consider that to be overflow; that's precision > > > loss. E-C considers these to be different: > > > > > > > If the source value cannot be represented exactly by the fixed-point > > > > type, the source value is rounded to either the closest fixed-point > > > > value greater than the source value (rounded up) or to the closest > > > > fixed-point value less than the source value (rounded down). > > > > > > > > When the source value does not fit within the range of the fixed-point > > > > type, the conversion overflows. [...] > > > > > > > > [...] > > > > > > > > If the result type of an arithmetic operation is a fixed-point type, > > > > [...] the calculated result is the mathematically exact result with > > > > overflow handling and rounding performed to the full precision of the > > > > result type as explained in 4.1.3. > > > > > > There is also no value of `e` that would affect saturation. Any full > > > precision calculation that gives `k+e` must be `k` after downscaling, > > > since the bits that represent `e` must come from the extra precision > > > range. Even though `k+e` is technically larger than `k`, saturation would > > > still just give us `k` after truncating out `e`, so the end result is the > > > same. > > > > > > > Would this algorithm be simpler if you took advantage of the fact that > > > > APFixedPointSemantics doesn't have to correspond to a real type? You > > > > could probably just convert to a double-width common semantics, right? > > > > > > It's likely possible to use APFixedPoint in the calculations here, but I > > > used APInt to make the behavior explicit and not accidentally be > > > dependent on the behavior of APFixedPoint's conversions or operations. > > Although.,. I guess I see your point in that an intermediate result of k+e > > technically "does not fit within the range of the fixed-point type"... but > > I wonder if treating such cases as overflow is particularly meaningful. I > > don't find there to be much of a distinction between such a case and the > > case where the exact result lands inbetween two representable values. We > > just end up with a less precise result. > Right, I was wondering if there was an accepted answer here. For saturating > arithmetic, it's equivalent to truncate this extra precision down to *k* or > to saturate to the maximum representable value, since by assumption that was > just *k*; but for non-saturating arithmetic, it affects whether the operation > has UB. All else being the same, it's better to have fewer corner-case > sources of UB. > > My read is that Embedded C is saying there's a sequence here: compute the > exact mathematical result; round that to the precision of the result type; > the operation overflows if the rounded result is not representable in the > result type. Is the rounding direction completely unspecified, down to being > potentially operand-specific? If so, we could just say that we always round > to avoid overflow if possible. The main consideration here is that we need > to give the operation the same semantics statically and dynamically, and I > don't know if there's any situation where those semantics would affect the > performance of the operation when done dynamically. > For
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
rjmccall added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + ebevhan wrote: > ebevhan wrote: > > rjmccall wrote: > > > If the maximum expressible value is *k*, and the fully-precise > > > multiplication yields *k+e* for some epsilon *e* that isn't representable > > > in the result semantics, is that considered an overflow? If so, I think > > > you need to do the shift after these bound checks, since the shift > > > destroys the difference between *k* and *k+e*. That is, unless there's a > > > compelling mathematical argument that it's not possible to overflow only > > > in the fully-precision multiplication — but while I think that's possibly > > > true of `_Fract` (since *k^2 < k*), it seems unlikely to be true of > > > `_Accum`, although I haven't looked for a counter-example. And if there > > > is a compelling argument, it should probably be at least alluded to in a > > > comment. > > > > > > Would this algorithm be simpler if you took advantage of the fact that > > > `APFixedPointSemantics` doesn't have to correspond to a real type? You > > > could probably just convert to a double-width common semantics, right? > > > If the maximum expressible value is *k*, and the fully-precise > > > multiplication yields *k+e* for some epsilon *e* that isn't representable > > > in the result semantics, is that considered an overflow? If so, I think > > > you need to do the shift after these bound checks, since the shift > > > destroys the difference between *k* and *k+e*. > > > > I don't think I would consider that to be overflow; that's precision loss. > > E-C considers these to be different: > > > > > If the source value cannot be represented exactly by the fixed-point > > > type, the source value is rounded to either the closest fixed-point value > > > greater than the source value (rounded up) or to the closest fixed-point > > > value less than the source value (rounded down). > > > > > > When the source value does not fit within the range of the fixed-point > > > type, the conversion overflows. [...] > > > > > > [...] > > > > > > If the result type of an arithmetic operation is a fixed-point type, > > > [...] the calculated result is the mathematically exact result with > > > overflow handling and rounding performed to the full precision of the > > > result type as explained in 4.1.3. > > > > There is also no value of `e` that would affect saturation. Any full > > precision calculation that gives `k+e` must be `k` after downscaling, since > > the bits that represent `e` must come from the extra precision range. Even > > though `k+e` is technically larger than `k`, saturation would still just > > give us `k` after truncating out `e`, so the end result is the same. > > > > > Would this algorithm be simpler if you took advantage of the fact that > > > APFixedPointSemantics doesn't have to correspond to a real type? You > > > could probably just convert to a double-width common semantics, right? > > > > It's likely possible to use APFixedPoint in the calculations here, but I > > used APInt to make the behavior explicit and not accidentally be dependent > > on the behavior of APFixedPoint's conversions or operations. > Although.,. I guess I see your point in that an intermediate result of k+e > technically "does not fit within the range of the fixed-point type"... but I > wonder if treating such cases as overflow is particularly meaningful. I don't > find there to be much of a distinction between such a case and the case where > the exact result lands inbetween two representable values. We just end up > with a less precise result. Right, I was wondering if there was an accepted answer here. For saturating arithmetic, it's equivalent to truncate this extra precision down to *k* or to saturate to the maximum representable value, since by assumption that was just *k*; but for non-saturating arithmetic, it affects whether the operation has UB. All else being the same, it's better to have fewer corner-case sources of UB. My read is that Embedded C is saying there's a sequence here: compute the exact mathematical result; round that to the precision of the result type; the operation overflows if the rounded result is not representable in the result type. Is the rounding direction completely unspecified, down to being potentially operand-specific? If so, we could just say that we always round to avoid overflow if possible. The main consideration here is that we need to give the operation the same semantics statically and dynamically, and I don't know if there's any situation where those semantics would affect the performance of the operation when done dynamically. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73186/new/ https://reviews.llvm.org/D73186
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
ebevhan added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + ebevhan wrote: > rjmccall wrote: > > If the maximum expressible value is *k*, and the fully-precise > > multiplication yields *k+e* for some epsilon *e* that isn't representable > > in the result semantics, is that considered an overflow? If so, I think > > you need to do the shift after these bound checks, since the shift destroys > > the difference between *k* and *k+e*. That is, unless there's a compelling > > mathematical argument that it's not possible to overflow only in the > > fully-precision multiplication — but while I think that's possibly true of > > `_Fract` (since *k^2 < k*), it seems unlikely to be true of `_Accum`, > > although I haven't looked for a counter-example. And if there is a > > compelling argument, it should probably be at least alluded to in a comment. > > > > Would this algorithm be simpler if you took advantage of the fact that > > `APFixedPointSemantics` doesn't have to correspond to a real type? You > > could probably just convert to a double-width common semantics, right? > > If the maximum expressible value is *k*, and the fully-precise > > multiplication yields *k+e* for some epsilon *e* that isn't representable > > in the result semantics, is that considered an overflow? If so, I think you > > need to do the shift after these bound checks, since the shift destroys the > > difference between *k* and *k+e*. > > I don't think I would consider that to be overflow; that's precision loss. > E-C considers these to be different: > > > If the source value cannot be represented exactly by the fixed-point type, > > the source value is rounded to either the closest fixed-point value greater > > than the source value (rounded up) or to the closest fixed-point value less > > than the source value (rounded down). > > > > When the source value does not fit within the range of the fixed-point > > type, the conversion overflows. [...] > > > > [...] > > > > If the result type of an arithmetic operation is a fixed-point type, [...] > > the calculated result is the mathematically exact result with overflow > > handling and rounding performed to the full precision of the result type as > > explained in 4.1.3. > > There is also no value of `e` that would affect saturation. Any full > precision calculation that gives `k+e` must be `k` after downscaling, since > the bits that represent `e` must come from the extra precision range. Even > though `k+e` is technically larger than `k`, saturation would still just give > us `k` after truncating out `e`, so the end result is the same. > > > Would this algorithm be simpler if you took advantage of the fact that > > APFixedPointSemantics doesn't have to correspond to a real type? You could > > probably just convert to a double-width common semantics, right? > > It's likely possible to use APFixedPoint in the calculations here, but I used > APInt to make the behavior explicit and not accidentally be dependent on the > behavior of APFixedPoint's conversions or operations. Although.,. I guess I see your point in that an intermediate result of k+e technically "does not fit within the range of the fixed-point type"... but I wonder if treating such cases as overflow is particularly meaningful. I don't find there to be much of a distinction between such a case and the case where the exact result lands inbetween two representable values. We just end up with a less precise result. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73186/new/ https://reviews.llvm.org/D73186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
ebevhan added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + rjmccall wrote: > If the maximum expressible value is *k*, and the fully-precise multiplication > yields *k+e* for some epsilon *e* that isn't representable in the result > semantics, is that considered an overflow? If so, I think you need to do the > shift after these bound checks, since the shift destroys the difference > between *k* and *k+e*. That is, unless there's a compelling mathematical > argument that it's not possible to overflow only in the fully-precision > multiplication — but while I think that's possibly true of `_Fract` (since > *k^2 < k*), it seems unlikely to be true of `_Accum`, although I haven't > looked for a counter-example. And if there is a compelling argument, it > should probably be at least alluded to in a comment. > > Would this algorithm be simpler if you took advantage of the fact that > `APFixedPointSemantics` doesn't have to correspond to a real type? You could > probably just convert to a double-width common semantics, right? > If the maximum expressible value is *k*, and the fully-precise multiplication > yields *k+e* for some epsilon *e* that isn't representable in the result > semantics, is that considered an overflow? If so, I think you need to do the > shift after these bound checks, since the shift destroys the difference > between *k* and *k+e*. I don't think I would consider that to be overflow; that's precision loss. E-C considers these to be different: > If the source value cannot be represented exactly by the fixed-point type, > the source value is rounded to either the closest fixed-point value greater > than the source value (rounded up) or to the closest fixed-point value less > than the source value (rounded down). > > When the source value does not fit within the range of the fixed-point type, > the conversion overflows. [...] > > [...] > > If the result type of an arithmetic operation is a fixed-point type, [...] > the calculated result is the mathematically exact result with overflow > handling and rounding performed to the full precision of the result type as > explained in 4.1.3. There is also no value of `e` that would affect saturation. Any full precision calculation that gives `k+e` must be `k` after downscaling, since the bits that represent `e` must come from the extra precision range. Even though `k+e` is technically larger than `k`, saturation would still just give us `k` after truncating out `e`, so the end result is the same. > Would this algorithm be simpler if you took advantage of the fact that > APFixedPointSemantics doesn't have to correspond to a real type? You could > probably just convert to a double-width common semantics, right? It's likely possible to use APFixedPoint in the calculations here, but I used APInt to make the behavior explicit and not accidentally be dependent on the behavior of APFixedPoint's conversions or operations. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73186/new/ https://reviews.llvm.org/D73186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
ebevhan updated this revision to Diff 239797.
ebevhan added a comment.
Rebased.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73186/new/
https://reviews.llvm.org/D73186
Files:
clang/include/clang/Basic/FixedPoint.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Basic/FixedPoint.cpp
clang/test/Frontend/fixed_point_mul.c
Index: clang/test/Frontend/fixed_point_mul.c
===
--- clang/test/Frontend/fixed_point_mul.c
+++ clang/test/Frontend/fixed_point_mul.c
@@ -1,6 +1,49 @@
// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+// Multiplication between different fixed point types
+short _Accum sa_const = 2.0hk * 2.0hk; // CHECK-DAG: @sa_const = {{.*}}global i16 512, align 2
+_Accum a_const = 3.0hk * 2.0k; // CHECK-DAG: @a_const = {{.*}}global i32 196608, align 4
+long _Accum la_const = 4.0hk * 2.0lk; // CHECK-DAG: @la_const = {{.*}}global i64 17179869184, align 8
+short _Accum sa_const2 = 0.5hr * 2.0hk; // CHECK-DAG: @sa_const2 = {{.*}}global i16 128, align 2
+short _Accum sa_const3 = 0.5r * 3.0hk; // CHECK-DAG: @sa_const3 = {{.*}}global i16 192, align 2
+short _Accum sa_const4 = 0.5lr * 4.0hk; // CHECK-DAG: @sa_const4 = {{.*}}global i16 256, align 2
+
+// Unsigned multiplication
+unsigned short _Accum usa_const = 1.0uhk * 2.0uhk;
+// CHECK-SIGNED-DAG: @usa_const = {{.*}}global i16 768, align 2
+// CHECK-UNSIGNED-DAG: @usa_const = {{.*}}global i16 384, align 2
+
+// Unsigned * signed
+short _Accum sa_const5 = 20.0uhk * 3.0hk;
+// CHECK-DAG: @sa_const5 = {{.*}}global i16 7680, align 2
+
+// Multiplication with negative number
+short _Accum sa_const6 = 0.5hr * (-2.0hk);
+// CHECK-DAG: @sa_const6 = {{.*}}global i16 -128, align 2
+
+// Int multiplication
+unsigned short _Accum usa_const2 = 5 * 10.5uhk;
+// CHECK-SIGNED-DAG: @usa_const2 = {{.*}}global i16 640, align 2
+// CHECK-UNSIGNED-DAG: @usa_const2 = {{.*}}global i16 320, align 2
+short _Accum sa_const7 = 3 * (-0.5hk); // CHECK-DAG: @sa_const7 = {{.*}}global i16 -192, align 2
+short _Accum sa_const8 = 100 * (-2.0hk); // CHECK-DAG: @sa_const8 = {{.*}}global i16 -25600, align 2
+long _Fract lf_const = -0.25lr * 3; // CHECK-DAG: @lf_const = {{.*}}global i32 -1610612736, align 4
+
+// Saturated multiplication
+_Sat short _Accum sat_sa_const = (_Sat short _Accum)128.0hk * 3.0hk;
+// CHECK-DAG: @sat_sa_const = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const = (_Sat unsigned short _Accum)128.0uhk * 128.0uhk;
+// CHECK-SIGNED-DAG: @sat_usa_const = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const = {{.*}}global i16 32767, align 2
+_Sat short _Accum sat_sa_const2 = (_Sat short _Accum)128.0hk * -128;
+// CHECK-DAG: @sat_sa_const2 = {{.*}}global i16 -32768, align 2
+_Sat unsigned short _Accum sat_usa_const2 = (_Sat unsigned short _Accum)128.0uhk * 30;
+// CHECK-SIGNED-DAG: @sat_usa_const2 = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const2 = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const3 = (_Sat unsigned short _Accum)0.5uhk * (-2);
+// CHECK-DAG: @sat_usa_const3 = {{.*}}global i16 0, align 2
+
void SignedMultiplication() {
// CHECK-LABEL: SignedMultiplication
short _Accum sa;
Index: clang/lib/Basic/FixedPoint.cpp
===
--- clang/lib/Basic/FixedPoint.cpp
+++ clang/lib/Basic/FixedPoint.cpp
@@ -197,6 +197,57 @@
return APFixedPoint(Result, CommonFXSema);
}
+APFixedPoint APFixedPoint::mul(const APFixedPoint ,
+ bool *Overflow) const {
+ auto CommonFXSema = Sema.getCommonSemantics(Other.getSemantics());
+ APFixedPoint ConvertedThis = convert(CommonFXSema);
+ APFixedPoint ConvertedOther = Other.convert(CommonFXSema);
+ llvm::APSInt ThisVal = ConvertedThis.getValue();
+ llvm::APSInt OtherVal = ConvertedOther.getValue();
+ bool Overflowed = false;
+
+ // Widen the LHS and RHS so we can perform a full multiplication.
+ unsigned Wide = CommonFXSema.getWidth() * 2;
+ if (CommonFXSema.isSigned()) {
+ThisVal = ThisVal.sextOrSelf(Wide);
+OtherVal = OtherVal.sextOrSelf(Wide);
+ } else {
+ThisVal = ThisVal.zextOrSelf(Wide);
+OtherVal = OtherVal.zextOrSelf(Wide);
+ }
+
+ // Perform the full multiplication and downscale to get the same scale.
+ llvm::APSInt Result;
+ if (CommonFXSema.isSigned())
+Result = ThisVal.smul_ov(OtherVal, Overflowed)
+.ashr(CommonFXSema.getScale());
+ else
+Result = ThisVal.umul_ov(OtherVal, Overflowed)
+.lshr(CommonFXSema.getScale());
+ assert(!Overflowed && "Full multiplication cannot
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
rjmccall added inline comments. Comment at: clang/lib/Basic/FixedPoint.cpp:242 + } else +Overflowed = Result < Min || Result > Max; + If the maximum expressible value is *k*, and the fully-precise multiplication yields *k+e* for some epsilon *e* that isn't representable in the result semantics, is that considered an overflow? If so, I think you need to do the shift after these bound checks, since the shift destroys the difference between *k* and *k+e*. That is, unless there's a compelling mathematical argument that it's not possible to overflow only in the fully-precision multiplication — but while I think that's possibly true of `_Fract` (since *k^2 < k*), it seems unlikely to be true of `_Accum`, although I haven't looked for a counter-example. And if there is a compelling argument, it should probably be at least alluded to in a comment. Would this algorithm be simpler if you took advantage of the fact that `APFixedPointSemantics` doesn't have to correspond to a real type? You could probably just convert to a double-width common semantics, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73186/new/ https://reviews.llvm.org/D73186 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.
ebevhan created this revision.
ebevhan added reviewers: rjmccall, leonardchan, bjope.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D73186
Files:
clang/include/clang/Basic/FixedPoint.h
clang/lib/AST/ExprConstant.cpp
clang/lib/Basic/FixedPoint.cpp
clang/test/Frontend/fixed_point_mul.c
Index: clang/test/Frontend/fixed_point_mul.c
===
--- clang/test/Frontend/fixed_point_mul.c
+++ clang/test/Frontend/fixed_point_mul.c
@@ -1,6 +1,49 @@
// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -fpadding-on-unsigned-fixed-point -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,UNSIGNED
+// Multiplication between different fixed point types
+short _Accum sa_const = 2.0hk * 2.0hk; // CHECK-DAG: @sa_const = {{.*}}global i16 512, align 2
+_Accum a_const = 3.0hk * 2.0k; // CHECK-DAG: @a_const = {{.*}}global i32 196608, align 4
+long _Accum la_const = 4.0hk * 2.0lk; // CHECK-DAG: @la_const = {{.*}}global i64 17179869184, align 8
+short _Accum sa_const2 = 0.5hr * 2.0hk; // CHECK-DAG: @sa_const2 = {{.*}}global i16 128, align 2
+short _Accum sa_const3 = 0.5r * 3.0hk; // CHECK-DAG: @sa_const3 = {{.*}}global i16 192, align 2
+short _Accum sa_const4 = 0.5lr * 4.0hk; // CHECK-DAG: @sa_const4 = {{.*}}global i16 256, align 2
+
+// Unsigned multiplication
+unsigned short _Accum usa_const = 1.0uhk * 2.0uhk;
+// CHECK-SIGNED-DAG: @usa_const = {{.*}}global i16 768, align 2
+// CHECK-UNSIGNED-DAG: @usa_const = {{.*}}global i16 384, align 2
+
+// Unsigned * signed
+short _Accum sa_const5 = 20.0uhk * 3.0hk;
+// CHECK-DAG: @sa_const5 = {{.*}}global i16 7680, align 2
+
+// Multiplication with negative number
+short _Accum sa_const6 = 0.5hr * (-2.0hk);
+// CHECK-DAG: @sa_const6 = {{.*}}global i16 -128, align 2
+
+// Int multiplication
+unsigned short _Accum usa_const2 = 5 * 10.5uhk;
+// CHECK-SIGNED-DAG: @usa_const2 = {{.*}}global i16 640, align 2
+// CHECK-UNSIGNED-DAG: @usa_const2 = {{.*}}global i16 320, align 2
+short _Accum sa_const7 = 3 * (-0.5hk); // CHECK-DAG: @sa_const7 = {{.*}}global i16 -192, align 2
+short _Accum sa_const8 = 100 * (-2.0hk); // CHECK-DAG: @sa_const8 = {{.*}}global i16 -25600, align 2
+long _Fract lf_const = -0.25lr * 3; // CHECK-DAG: @lf_const = {{.*}}global i32 -1610612736, align 4
+
+// Saturated multiplication
+_Sat short _Accum sat_sa_const = (_Sat short _Accum)128.0hk * 3.0hk;
+// CHECK-DAG: @sat_sa_const = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const = (_Sat unsigned short _Accum)128.0uhk * 128.0uhk;
+// CHECK-SIGNED-DAG: @sat_usa_const = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const = {{.*}}global i16 32767, align 2
+_Sat short _Accum sat_sa_const2 = (_Sat short _Accum)128.0hk * -128;
+// CHECK-DAG: @sat_sa_const2 = {{.*}}global i16 -32768, align 2
+_Sat unsigned short _Accum sat_usa_const2 = (_Sat unsigned short _Accum)128.0uhk * 30;
+// CHECK-SIGNED-DAG: @sat_usa_const2 = {{.*}}global i16 65535, align 2
+// CHECK-UNSIGNED-DAG: @sat_usa_const2 = {{.*}}global i16 32767, align 2
+_Sat unsigned short _Accum sat_usa_const3 = (_Sat unsigned short _Accum)0.5uhk * (-2);
+// CHECK-DAG: @sat_usa_const3 = {{.*}}global i16 0, align 2
+
void SignedMultiplication() {
// CHECK-LABEL: SignedMultiplication
short _Accum sa;
Index: clang/lib/Basic/FixedPoint.cpp
===
--- clang/lib/Basic/FixedPoint.cpp
+++ clang/lib/Basic/FixedPoint.cpp
@@ -197,6 +197,57 @@
return APFixedPoint(Result, CommonFXSema);
}
+APFixedPoint APFixedPoint::mul(const APFixedPoint ,
+ bool *Overflow) const {
+ auto CommonFXSema = Sema.getCommonSemantics(Other.getSemantics());
+ APFixedPoint ConvertedThis = convert(CommonFXSema);
+ APFixedPoint ConvertedOther = Other.convert(CommonFXSema);
+ llvm::APSInt ThisVal = ConvertedThis.getValue();
+ llvm::APSInt OtherVal = ConvertedOther.getValue();
+ bool Overflowed = false;
+
+ // Widen the LHS and RHS so we can perform a full multiplication.
+ unsigned Wide = CommonFXSema.getWidth() * 2;
+ if (CommonFXSema.isSigned()) {
+ThisVal = ThisVal.sextOrSelf(Wide);
+OtherVal = OtherVal.sextOrSelf(Wide);
+ } else {
+ThisVal = ThisVal.zextOrSelf(Wide);
+OtherVal = OtherVal.zextOrSelf(Wide);
+ }
+
+ // Perform the full multiplication and downscale to get the same scale.
+ llvm::APSInt Result;
+ if (CommonFXSema.isSigned())
+Result = ThisVal.smul_ov(OtherVal, Overflowed)
+.ashr(CommonFXSema.getScale());
+ else
+Result = ThisVal.umul_ov(OtherVal, Overflowed)
+.lshr(CommonFXSema.getScale());
+ assert(!Overflowed && "Full multiplication
