[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
This revision was automatically updated to reflect the committed changes.
Closed by commit rL344530: [Fixed Point Arithmetic] FixedPointCast (authored by
leonardchan, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D50616?vs=169714&id=169716#toc
Repository:
rL LLVM
https://reviews.llvm.org/D50616
Files:
cfe/trunk/include/clang/AST/OperationKinds.def
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGExprAgg.cpp
cfe/trunk/lib/CodeGen/CGExprComplex.cpp
cfe/trunk/lib/CodeGen/CGExprConstant.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/Edit/RewriteObjCFoundationAPI.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/test/Frontend/fixed_point_conversions.c
cfe/trunk/test/Frontend/fixed_point_unknown_conversions.c
Index: cfe/trunk/include/clang/AST/Type.h
===
--- cfe/trunk/include/clang/AST/Type.h
+++ cfe/trunk/include/clang/AST/Type.h
@@ -2066,7 +2066,8 @@
STK_Integral,
STK_Floating,
STK_IntegralComplex,
-STK_FloatingComplex
+STK_FloatingComplex,
+STK_FixedPoint
};
/// Given that this is a scalar type, classify it.
Index: cfe/trunk/include/clang/AST/OperationKinds.def
===
--- cfe/trunk/include/clang/AST/OperationKinds.def
+++ cfe/trunk/include/clang/AST/OperationKinds.def
@@ -197,6 +197,10 @@
///float f = i;
CAST_OPERATION(IntegralToFloating)
+/// CK_FixedPointCast - Fixed point to fixed point.
+///(_Accum) 0.5r
+CAST_OPERATION(FixedPointCast)
+
/// CK_FloatingToIntegral - Floating point to integral. Rounds
/// towards zero, discarding any fractional component.
///(int) f
Index: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
@@ -172,6 +172,8 @@
"this value is too large for this fixed point type">;
def err_fixed_point_not_enabled : Error<"compile with "
"'-ffixed-point' to enable fixed point types">;
+def err_unimplemented_conversion_with_fixed_point_type : Error<
+ "conversion between fixed point and %0 is not yet supported">;
// SEH
def err_seh_expected_handler : Error<
Index: cfe/trunk/test/Frontend/fixed_point_conversions.c
===
--- cfe/trunk/test/Frontend/fixed_point_conversions.c
+++ cfe/trunk/test/Frontend/fixed_point_conversions.c
@@ -0,0 +1,283 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+
+void TestFixedPointCastSameType() {
+ _Accum a = 2.5k;
+ _Accum a2 = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+
+ a2 = (_Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+}
+
+void TestFixedPointCastDown() {
+ long _Accum la = 2.5lk;
+ _Accum a = la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[ACCUM_AS_I64:%[0-9]+]] = ashr i64 [[LACCUM]], 16
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = trunc i64 [[ACCUM_AS_I64]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a, align 4
+
+ a = (_Accum)la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[ACCUM_AS_I64:%[0-9]+]] = ashr i64 [[LACCUM]], 16
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = trunc i64 [[ACCUM_AS_I64]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a, align 4
+
+ short _Accum sa = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: [[SACCUM_AS_I32:%[0-9]+]] = ashr i32 [[ACCUM]], 8
+ // DEFAULT-NEXT: [[SACCUM:%[0-9]+]] = trunc i32 [[SACCUM_AS_I32]] to i16
+ // DEFAULT-NEXT: store i16 [[SACCUM]], i16* %sa, align 2
+
+ sa = (short _Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: [[SACCUM_AS_I32:%[0-9]+]] = ashr i32 [[ACCUM]], 8
+ // DEFAULT-NEXT: [[SACCUM:%[0-9]+]] = trunc i32 [[SACCUM_AS_I32]] to i16
+ // DEFAULT-NEXT: store i16 [[SACCUM]], i16* %sa, align 2
+}
+
+void TestFixedPointCastUp() {
+ short _Accum sa = 2.5hk;
+ _Accum a = sa;
+ // DEFAULT: [[SACCUM:%[0-9]+]] = load i16, i16* %sa, align 2
+ // DEFAULT-NEXT: [[SACCUM_BUFF:%[0-9]+]] = sext i16 [[SACCUM]] to i32
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = shl i32 [[S
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan updated this revision to Diff 169714.
leonardchan marked an inline comment as done.
Repository:
rC Clang
https://reviews.llvm.org/D50616
Files:
include/clang/AST/OperationKinds.def
include/clang/AST/Type.h
include/clang/Basic/DiagnosticCommonKinds.td
lib/AST/Expr.cpp
lib/AST/ExprConstant.cpp
lib/AST/Type.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprComplex.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/Edit/RewriteObjCFoundationAPI.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExpr.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
test/Frontend/fixed_point_conversions.c
test/Frontend/fixed_point_unknown_conversions.c
Index: test/Frontend/fixed_point_unknown_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_unknown_conversions.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -verify -ffixed-point %s
+
+void func() {
+ _Bool b;
+ char c;
+ int i;
+ float f;
+ double d;
+ double _Complex dc;
+ int _Complex ic;
+ struct S {
+int i;
+ } s;
+ enum E {
+A
+ } e;
+ int *ptr;
+ typedef int int_t;
+ int_t i2;
+
+ _Accum accum;
+ _Fract fract = accum; // ok
+ _Accum *accum_ptr;
+
+ accum = b; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = f; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ accum = d; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ accum = dc; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ accum = ic; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ accum = s; // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
+ accum = e; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ accum = ptr; // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
+ accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
+ accum = i2; // expected-error{{conversion between fixed point and 'int_t' (aka 'int') is not yet supported}}
+
+ b = accum; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ c = accum; // expected-error{{conversion between fixed point and 'char' is not yet supported}}
+ i = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ f = accum; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ d = accum; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ dc = accum; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ ic = accum; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ s = accum; // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
+ e = accum; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ ptr = accum; // expected-error{{assigning to 'int *' from incompatible type '_Accum'}}
+ ptr = accum_ptr; // expected-warning{{incompatible pointer types assigning to 'int *' from '_Accum *'}}
+ i2 = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+}
Index: test/Frontend/fixed_point_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_conversions.c
@@ -0,0 +1,283 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+
+void TestFixedPointCastSameType() {
+ _Accum a = 2.5k;
+ _Accum a2 = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+
+ a2 = (_Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+}
+
+void TestFixedPointCastDown() {
+ long _Accum la = 2.5lk;
+ _Accum a = la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[ACCUM_AS_I64:%[0-9]+]] = ashr i64 [[LACCUM]], 16
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = trunc i64 [[ACCUM_AS_I64]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a, align 4
+
+ a = (_Accum)la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ //
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
ebevhan added inline comments. Comment at: lib/CodeGen/CGExprScalar.cpp:1019 + assert(!SrcType->isFixedPointType() && !DstType->isFixedPointType() && + "Use the TargetCodeGenInfo::emitFixedPoint family functions for " + "handling conversions involving fixed point types."); It's not in TargetCodeGenInfo any more. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan updated this revision to Diff 169500.
leonardchan added a comment.
- Removed target hook
Repository:
rC Clang
https://reviews.llvm.org/D50616
Files:
include/clang/AST/OperationKinds.def
include/clang/AST/Type.h
include/clang/Basic/DiagnosticCommonKinds.td
lib/AST/Expr.cpp
lib/AST/ExprConstant.cpp
lib/AST/Type.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprComplex.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/Edit/RewriteObjCFoundationAPI.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExpr.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
test/Frontend/fixed_point_conversions.c
test/Frontend/fixed_point_unknown_conversions.c
Index: test/Frontend/fixed_point_unknown_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_unknown_conversions.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -verify -ffixed-point %s
+
+void func() {
+ _Bool b;
+ char c;
+ int i;
+ float f;
+ double d;
+ double _Complex dc;
+ int _Complex ic;
+ struct S {
+int i;
+ } s;
+ enum E {
+A
+ } e;
+ int *ptr;
+ typedef int int_t;
+ int_t i2;
+
+ _Accum accum;
+ _Fract fract = accum; // ok
+ _Accum *accum_ptr;
+
+ accum = b; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = f; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ accum = d; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ accum = dc; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ accum = ic; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ accum = s; // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
+ accum = e; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ accum = ptr; // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
+ accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
+ accum = i2; // expected-error{{conversion between fixed point and 'int_t' (aka 'int') is not yet supported}}
+
+ b = accum; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ c = accum; // expected-error{{conversion between fixed point and 'char' is not yet supported}}
+ i = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ f = accum; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ d = accum; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ dc = accum; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ ic = accum; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ s = accum; // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
+ e = accum; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ ptr = accum; // expected-error{{assigning to 'int *' from incompatible type '_Accum'}}
+ ptr = accum_ptr; // expected-warning{{incompatible pointer types assigning to 'int *' from '_Accum *'}}
+ i2 = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+}
Index: test/Frontend/fixed_point_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_conversions.c
@@ -0,0 +1,283 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+
+void TestFixedPointCastSameType() {
+ _Accum a = 2.5k;
+ _Accum a2 = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+
+ a2 = (_Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+}
+
+void TestFixedPointCastDown() {
+ long _Accum la = 2.5lk;
+ _Accum a = la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[ACCUM_AS_I64:%[0-9]+]] = ashr i64 [[LACCUM]], 16
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = trunc i64 [[ACCUM_AS_I64]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a, align 4
+
+ a = (_Accum)la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan updated this revision to Diff 169495.
Repository:
rC Clang
https://reviews.llvm.org/D50616
Files:
include/clang/AST/OperationKinds.def
include/clang/AST/Type.h
include/clang/Basic/DiagnosticCommonKinds.td
lib/AST/Expr.cpp
lib/AST/ExprConstant.cpp
lib/AST/Type.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprComplex.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/TargetInfo.cpp
lib/Edit/RewriteObjCFoundationAPI.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExpr.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
test/Frontend/fixed_point_conversions.c
test/Frontend/fixed_point_unknown_conversions.c
Index: test/Frontend/fixed_point_unknown_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_unknown_conversions.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -verify -ffixed-point %s
+
+void func() {
+ _Bool b;
+ char c;
+ int i;
+ float f;
+ double d;
+ double _Complex dc;
+ int _Complex ic;
+ struct S {
+int i;
+ } s;
+ enum E {
+A
+ } e;
+ int *ptr;
+ typedef int int_t;
+ int_t i2;
+
+ _Accum accum;
+ _Fract fract = accum; // ok
+ _Accum *accum_ptr;
+
+ accum = b; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = f; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ accum = d; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ accum = dc; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ accum = ic; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ accum = s; // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
+ accum = e; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ accum = ptr; // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
+ accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
+ accum = i2; // expected-error{{conversion between fixed point and 'int_t' (aka 'int') is not yet supported}}
+
+ b = accum; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ c = accum; // expected-error{{conversion between fixed point and 'char' is not yet supported}}
+ i = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ f = accum; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ d = accum; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ dc = accum; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ ic = accum; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ s = accum; // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
+ e = accum; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ ptr = accum; // expected-error{{assigning to 'int *' from incompatible type '_Accum'}}
+ ptr = accum_ptr; // expected-warning{{incompatible pointer types assigning to 'int *' from '_Accum *'}}
+ i2 = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+}
Index: test/Frontend/fixed_point_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_conversions.c
@@ -0,0 +1,283 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+
+void TestFixedPointCastSameType() {
+ _Accum a = 2.5k;
+ _Accum a2 = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+
+ a2 = (_Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+}
+
+void TestFixedPointCastDown() {
+ long _Accum la = 2.5lk;
+ _Accum a = la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[ACCUM_AS_I64:%[0-9]+]] = ashr i64 [[LACCUM]], 16
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = trunc i64 [[ACCUM_AS_I64]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a, align 4
+
+ a = (_Accum)la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
ebevhan added a comment. I agree with John, after that I think it's fine for me. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
rjmccall added a comment. In https://reviews.llvm.org/D50616#1259769, @leonardchan wrote: > @ebevhan @rjmccall Seeing that the saturation intrinsic in > https://reviews.llvm.org/D52286 should be put on hold for now, would it be > fine to submit this patch as is? Then if the intrinsic is finished, come back > to this and update this patch to use the intrinsic? Okay, but please un-target-hook it. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan added a comment. @ebevhan @rjmccall Seeing that the saturation intrinsic in https://reviews.llvm.org/D52286 should be put on hold for now, would it be fine to submit this patch as is? Then if the intrinsic is finished, come back to this and update this patch to use the intrinsic? Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:419
+ case CK_LValueBitCast:
+ case CK_FixedPointCast: {
state =
a.sidorin wrote:
> Should we consider this construction as unsupported rather than supported as
> a normal cast?
Uhm, this code seems to be held together by magic. We squeeze all sorts of
casts (eg., float casts) into a subroutine that deals with casts of //lvalues//
(!?) Fortunately, it dissolves into `SValBuilder::evalCast()` pretty quickly,
so we don't really get punished for that. So it's not this patch's fault but
our technical debt. I guess this change on its own doesn't make things worse,
so i'm ok with it.
Repository:
rC Clang
https://reviews.llvm.org/D50616
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
a.sidorin added subscribers: NoQ, a.sidorin.
a.sidorin added a comment.
Accidentally noticed about this review via cfe-commits. @NoQ, the change in the
ExprEngine looks a bit dangerous to me. Could you please check?
Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:419
+ case CK_LValueBitCast:
+ case CK_FixedPointCast: {
state =
Should we consider this construction as unsupported rather than supported as a
normal cast?
Repository:
rC Clang
https://reviews.llvm.org/D50616
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
rjmccall added a comment. In https://reviews.llvm.org/D50616#1206181, @leonardchan wrote: > I made a post on llvm-dev > (http://lists.llvm.org/pipermail/llvm-dev/2018-August/125433.html) to see if > other people have opinions on this. In the meantime, do you think I should > make a separate patch that moves this into an LLVM intrinsic function? Yeah, I think that even if LLVM decides they don't want to include first-class IR support for fixed-point types, it makes more sense to provide standard intrinsics for these operations than to do all of the lowering in the frontend. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan added a comment. I made a post on llvm-dev (http://lists.llvm.org/pipermail/llvm-dev/2018-August/125433.html) to see if other people have opinions on this. In the meantime, do you think I should make a separate patch that moves this into an LLVM intrinsic function? Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
ebevhan added a comment. In https://reviews.llvm.org/D50616#1204588, @leonardchan wrote: > Would it be simpler instead just to have the logic contained in the virtual > function for `TargetCodeGenInfo` as opposed to returning `nullptr` since any > custom target will end up overriding it anyway and ideally not return > `nullptr`? I guess that's also possible. I figured it would be clearer to have the generic logic in the more obvious location (CGExprScalar) and fall back to the custom handling if needed. Many of the handlers in TargetCodeGenInfo are empty. > I haven't brought this up to llvm-dev, but the main reason for why we decided > to only implement this in clang was because we thought it would be a lot > harder pushing a new type into upstream llvm, especially since a lot of the > features can be supported on the frontend using clang's existing > infrastructure. We are open to adding fixed point types to LLVM IR in the > future though once all the frontend stuff is laid out and if the community is > willing to accept it. I know there are multiple threads on llvm-dev asking about fixed-point support (some of them from us) and the verdict usually seems to be "no new types, use integers and intrinsics". This has worked fairly well for us, so it should be possible to adapt the design for upstream if there is a desire to have it. I do not think a new type is required. As there are no in-tree architectures with native fixed-point support (as far as I know? Does AVR or ARM support it, perhaps? Probably not enough to cover the entire E-C spec, though.), there isn't really much to warrant adding a new type (or even new IR operators) for it. Furthermore, many of the operations on fixed-point are simple integer operations (except ones like saturation/saturating operations, multiplication, division etc) so those would be better off implemented in terms of those operators rather than inventing new ones that do pretty much the same thing. In https://reviews.llvm.org/D50616#1204754, @rjmccall wrote: > Very few things in LLVM actually try to exhaustively handle all operations. > There are a > couple of generic predicates on `Instruction` like `mayHaveSideEffects`, > there's > serialization/`.ll`-writing, and there's code-gen. The first two are not > hard at all > to implement, and it'd be quite simple to write a legalization pass in > code-gen that > turns all these operations into integer operations and which could easily be > customized > to support targets that want to do something more precise. I certainly support the idea of representing some (not all) fixed-point operations as intrinsics (or instructions, although I suspect that's more work than one might think) and converting them into integer instructions for targets which do not care to support the specific operation natively. A type is overkill; fixed-point values **are** 'bags of bits' like LLVM integers, just with a different interpretation of the bits. It's no different than the distinction between signed and unsigned, and we model that on the LLVM level through the operations rather than through the types. Creating instruction/intrinsic definitions of fixed-point operations does come with problems. If an IR producer wants to emit fixed-point operations with slightly different semantics than what our instructions/intrinsics have, they're out of luck. It might also be a problem for targets that have fixed-point support, but cannot perform the operations in exactly the same manner as the IR operation specifies. Making the semantics of the operation too tight could cause this to happen, but making them too loose makes it hard to really do anything with it. This is another argument for frontend customization. Every operation also introduces a bit of optimization/codegen work, unlike pure integer operations which the passes already know. Still, I think intrinsics for the complicated operations would provide a good balance. > The advantages of having real IR support include: > > - It's significant simpler for frontends. All of this logic in Clang will > need to be reimplemented in any other frontend that wants to support > fixed-point types. As I mentioned, this only simplifies things if the semantics of the operations work for the other frontend/language/target. > > > - It's much easier to test. The frontend needs to handle a lot of different > code patterns that ultimately turn into the same small set of basic > operations, like compound arithmetic and atomic operations and a million > different things that are supposed to trigger implicit conversions. It's ver > frustrating to write tests for these things when constants aren't readable > and the generated IR for every operation is a ten-instruction sequence. I think this is primarily a problem for saturating operations, as those require a whole bunch of conditional assignment. Nonsaturating multiplication is only about 5 inst
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
rjmccall added a comment. In https://reviews.llvm.org/D50616#1203772, @lebedev.ri wrote: > In https://reviews.llvm.org/D50616#1203751, @rjmccall wrote: > > > In https://reviews.llvm.org/D50616#1203692, @ebevhan wrote: > > > > > > > > > > > Has anyone actually asked LLVM whether they would accept fixed-point types > > into IR? I'm just a frontend guy, but it seems to me that there are > > advantages to directly representing these operations in a portable way even > > if there are no in-tree targets providing special support for them. And > > there are certainly in-tree targets that could provide such support if > > someone was motivated to do it. > > > Even just adding one new LLVM IR instruction (well, intrinsic too, ideally) > already 'requires' you to to > then go around and make sure it is properly handled wrt all the other > instructions, optimizations, codegen. > > Adding a whole new type, i suspect, would be *much* more impactful. > And since it can already be represented via existing operations on existing > integer type, > it isn't obvious why that would be the right way forward. Very few things in LLVM actually try to exhaustively handle all operations. There are a couple of generic predicates on `Instruction` like `mayHaveSideEffects`, there's serialization/`.ll`-writing, and there's code-gen. The first two are not hard at all to implement, and it'd be quite simple to write a legalization pass in code-gen that turns all these operations into integer operations and which could easily be customized to support targets that want to do something more precise. The advantages of having real IR support include: - It's significant simpler for frontends. All of this logic in Clang will need to be reimplemented in any other frontend that wants to support fixed-point types. - It's much easier to test. The frontend needs to handle a lot of different code patterns that ultimately turn into the same small set of basic operations, like compound arithmetic and atomic operations and a million different things that are supposed to trigger implicit conversions. It's ver frustrating to write tests for these things when constants aren't readable and the generated IR for every operation is a ten-instruction sequence. - It's much easier to analyze and optimize. I'm sure some fixed-point optimizations can be done generically over the underlying integer ops, but many others would be much more difficult if not impossible — e.g. I would assume that the lowering of padded unsigned values exploits an assumption that the padding bit is zero, which a generic optimization cannot know. - All those ten-instruction sequences add up in their compile-time impact on every stage of the pipeline. I'm not saying it's an open-and-shut case, but LLVM is supposed to be an abstraction layer over more than just the concerns of code-generation, and even those concerns don't obviously point towards frontend expansion. Like I said, if this is just a hobby and we don't really care about supporting this as a feature beyond just checking a box, frontend expansion is definitely the easier approach for checking that box. But if we want a high-quality implementation of fixed-point types, we should represent them directly in LLVM IR. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
rjmccall added a comment. >> Has anyone actually asked LLVM whether they would accept fixed-point types >> into IR? I'm just a frontend guy, but it seems to me that there are >> advantages to directly representing these operations in a portable way even >> if there are no in-tree targets providing special support for them. And >> there are certainly in-tree targets that could provide such support if >> someone was motivated to do it. > > I haven't brought this up to llvm-dev, but the main reason for why we decided > to only implement this in clang was because we thought it would be a lot > harder pushing a new type into upstream llvm, especially since a lot of the > features can be supported on the frontend using clang's existing > infrastructure. We are open to adding fixed point types to LLVM IR in the > future though once all the frontend stuff is laid out and if the community is > willing to accept it. Mostly I'm reluctant to add a ton of customization points to get more optimizable IR patterns from the frontend when I think it's really clear that the right thing to do is to represent these operations semantically through LLVM IR. If LLVM people don't want to add an `llvm::FixedPointType` then we should at least add portable intrinsics for them instead of target intrinsics, in which case there's still no point in customizing this in the frontend. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan added a comment. In https://reviews.llvm.org/D50616#1203692, @ebevhan wrote: > In https://reviews.llvm.org/D50616#1203446, @leonardchan wrote: > > > Sorry I forgot to address this also. Just to make sure I understand this > > correctly since I haven't used these before: target hooks are essentially > > for emitting target specific code for some operations right? Does this mean > > that the `EmitFixedPointConversion` function should be moved to a virtual > > method under `TargetCodeGenInfo` that can be overridden and this is what > > get's called instead during conversion? > > > Yes, the thought I had was to have a virtual function in TargetCodeGenInfo > that would be called first thing in EmitFixedPointConversion, and if it > returns non-null it uses that value instead. It's a bit unfortunate in this > instance as the only thing that doesn't work for us is the saturation, but > letting you configure *parts* of the emission is a bit too specific. Would it be simpler instead just to have the logic contained in the virtual function for `TargetCodeGenInfo` as opposed to returning `nullptr` since any custom target will end up overriding it anyway and ideally not return `nullptr`? In https://reviews.llvm.org/D50616#1203751, @rjmccall wrote: > In https://reviews.llvm.org/D50616#1203692, @ebevhan wrote: > > > In https://reviews.llvm.org/D50616#1203446, @leonardchan wrote: > > > > > Sorry I forgot to address this also. Just to make sure I understand this > > > correctly since I haven't used these before: target hooks are essentially > > > for emitting target specific code for some operations right? Does this > > > mean that the `EmitFixedPointConversion` function should be moved to a > > > virtual method under `TargetCodeGenInfo` that can be overridden and this > > > is what get's called instead during conversion? > > > > > > Yes, the thought I had was to have a virtual function in TargetCodeGenInfo > > that would be called first thing in EmitFixedPointConversion, and if it > > returns non-null it uses that value instead. It's a bit unfortunate in this > > instance as the only thing that doesn't work for us is the saturation, but > > letting you configure *parts* of the emission is a bit too specific. > > > > In https://reviews.llvm.org/D50616#1203635, @rjmccall wrote: > > > > > If this is more than just a hobby — like if there's a backend that wants > > > to do serious work with matching fixed-point operations to hardware > > > support — we should just add real LLVM IR support for fixed-point types > > > instead of adding a bunch of frontend customization hooks. I don't know > > > what better opportunity we're expecting might come along to justify that, > > > and I don't think it's some incredibly onerous task to add a new leaf to > > > the LLVM type hierarchy. Honestly, LLVM programmers across the board > > > have become far too accustomed to pushing things opaquely through an > > > uncooperative IR with an obscure muddle of intrinsics. > > > > > > In the meantime, we can continue working on this code. Even if there's > > > eventually real IR support for fixed-point types, this code will still be > > > useful; it'll just become the core of some legalization pass in the > > > generic backend. > > > > > > It definitely is; our downstream target requires it. As far as I know > > there's no real use case upstream, which is why it's likely quite hard to > > add any extensions to LLVM proper. Our implementation is done through > > intrinsics rather than an extension of the type system, as fixed-point > > numbers are really just integers under the hood. We've always wanted to > > upstream our fixed-point support (or some reasonable adaptation of it) but > > never gotten the impression that it would be possible since there's no > > upstream user. > > > > A mail I sent to the mailing list a while back has more details on our > > implementation, including what intrinsics we've added: > > http://lists.llvm.org/pipermail/cfe-dev/2018-May/058019.html We also have > > an LLVM pass that converts these intrinsics into pure IR, but it's likely > > that it won't work properly with some parts of this upstream design. > > > > Leonard's original proposal for this work has always been Clang-centric and > > never planned for implementing anything on the LLVM side, so we need to > > make due with what we get, but we would prefer as small a diff from > > upstream as possible. > > > Has anyone actually asked LLVM whether they would accept fixed-point types > into IR? I'm just a frontend guy, but it seems to me that there are > advantages to directly representing these operations in a portable way even > if there are no in-tree targets providing special support for them. And > there are certainly in-tree targets that could provide such support if > someone was motivated to do it. I haven't brought this up to llvm-dev, but the main reason for why we decided to only
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan updated this revision to Diff 161298.
leonardchan marked 4 inline comments as done.
Repository:
rC Clang
https://reviews.llvm.org/D50616
Files:
include/clang/AST/OperationKinds.def
include/clang/AST/Type.h
include/clang/Basic/DiagnosticCommonKinds.td
lib/AST/Expr.cpp
lib/AST/ExprConstant.cpp
lib/AST/Type.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprComplex.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/CodeGen/TargetInfo.cpp
lib/CodeGen/TargetInfo.h
lib/Edit/RewriteObjCFoundationAPI.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExpr.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
test/Frontend/fixed_point_conversions.c
test/Frontend/fixed_point_unknown_conversions.c
Index: test/Frontend/fixed_point_unknown_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_unknown_conversions.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -verify -ffixed-point %s
+
+void func() {
+ _Bool b;
+ char c;
+ int i;
+ float f;
+ double d;
+ double _Complex dc;
+ int _Complex ic;
+ struct S {
+int i;
+ } s;
+ enum E {
+A
+ } e;
+ int *ptr;
+ typedef int int_t;
+ int_t i2;
+
+ _Accum accum;
+ _Fract fract = accum; // ok
+ _Accum *accum_ptr;
+
+ accum = b; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = f; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ accum = d; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ accum = dc; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ accum = ic; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ accum = s; // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
+ accum = e; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ accum = ptr; // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
+ accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
+ accum = i2; // expected-error{{conversion between fixed point and 'int_t' (aka 'int') is not yet supported}}
+
+ b = accum; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ c = accum; // expected-error{{conversion between fixed point and 'char' is not yet supported}}
+ i = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ f = accum; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ d = accum; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ dc = accum; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ ic = accum; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ s = accum; // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
+ e = accum; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ ptr = accum; // expected-error{{assigning to 'int *' from incompatible type '_Accum'}}
+ ptr = accum_ptr; // expected-warning{{incompatible pointer types assigning to 'int *' from '_Accum *'}}
+ i2 = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+}
Index: test/Frontend/fixed_point_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_conversions.c
@@ -0,0 +1,283 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+
+void TestFixedPointCastSameType() {
+ _Accum a = 2.5k;
+ _Accum a2 = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+
+ a2 = (_Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+}
+
+void TestFixedPointCastDown() {
+ long _Accum la = 2.5lk;
+ _Accum a = la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[ACCUM_AS_I64:%[0-9]+]] = ashr i64 [[LACCUM]], 16
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = trunc i64 [[ACCUM_AS_I64]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a, align 4
+
+ a = (_Accum)la;
+ // DEFAULT:
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
lebedev.ri added a comment. In https://reviews.llvm.org/D50616#1203751, @rjmccall wrote: > In https://reviews.llvm.org/D50616#1203692, @ebevhan wrote: > > > > > > Has anyone actually asked LLVM whether they would accept fixed-point types > into IR? I'm just a frontend guy, but it seems to me that there are > advantages to directly representing these operations in a portable way even > if there are no in-tree targets providing special support for them. And > there are certainly in-tree targets that could provide such support if > someone was motivated to do it. Even just adding one new LLVM IR instruction (well, intrinsic too, ideally) already 'requires' you to to then go around and make sure it is properly handled wrt all the other instructions, optimizations, codegen. Adding a whole new type, i suspect, would be *much* more impactful. And since it can already be represented via existing operations on existing integer type, it isn't obvious why that would be the right way forward. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
rjmccall added a comment. In https://reviews.llvm.org/D50616#1203692, @ebevhan wrote: > In https://reviews.llvm.org/D50616#1203446, @leonardchan wrote: > > > Sorry I forgot to address this also. Just to make sure I understand this > > correctly since I haven't used these before: target hooks are essentially > > for emitting target specific code for some operations right? Does this mean > > that the `EmitFixedPointConversion` function should be moved to a virtual > > method under `TargetCodeGenInfo` that can be overridden and this is what > > get's called instead during conversion? > > > Yes, the thought I had was to have a virtual function in TargetCodeGenInfo > that would be called first thing in EmitFixedPointConversion, and if it > returns non-null it uses that value instead. It's a bit unfortunate in this > instance as the only thing that doesn't work for us is the saturation, but > letting you configure *parts* of the emission is a bit too specific. > > In https://reviews.llvm.org/D50616#1203635, @rjmccall wrote: > > > If this is more than just a hobby — like if there's a backend that wants to > > do serious work with matching fixed-point operations to hardware support — > > we should just add real LLVM IR support for fixed-point types instead of > > adding a bunch of frontend customization hooks. I don't know what better > > opportunity we're expecting might come along to justify that, and I don't > > think it's some incredibly onerous task to add a new leaf to the LLVM type > > hierarchy. Honestly, LLVM programmers across the board have become far too > > accustomed to pushing things opaquely through an uncooperative IR with an > > obscure muddle of intrinsics. > > > > In the meantime, we can continue working on this code. Even if there's > > eventually real IR support for fixed-point types, this code will still be > > useful; it'll just become the core of some legalization pass in the generic > > backend. > > > It definitely is; our downstream target requires it. As far as I know there's > no real use case upstream, which is why it's likely quite hard to add any > extensions to LLVM proper. Our implementation is done through intrinsics > rather than an extension of the type system, as fixed-point numbers are > really just integers under the hood. We've always wanted to upstream our > fixed-point support (or some reasonable adaptation of it) but never gotten > the impression that it would be possible since there's no upstream user. > > A mail I sent to the mailing list a while back has more details on our > implementation, including what intrinsics we've added: > http://lists.llvm.org/pipermail/cfe-dev/2018-May/058019.html We also have an > LLVM pass that converts these intrinsics into pure IR, but it's likely that > it won't work properly with some parts of this upstream design. > > Leonard's original proposal for this work has always been Clang-centric and > never planned for implementing anything on the LLVM side, so we need to make > due with what we get, but we would prefer as small a diff from upstream as > possible. Has anyone actually asked LLVM whether they would accept fixed-point types into IR? I'm just a frontend guy, but it seems to me that there are advantages to directly representing these operations in a portable way even if there are no in-tree targets providing special support for them. And there are certainly in-tree targets that could provide such support if someone was motivated to do it. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
ebevhan added a comment. In https://reviews.llvm.org/D50616#1203446, @leonardchan wrote: > Sorry I forgot to address this also. Just to make sure I understand this > correctly since I haven't used these before: target hooks are essentially for > emitting target specific code for some operations right? Does this mean that > the `EmitFixedPointConversion` function should be moved to a virtual method > under `TargetCodeGenInfo` that can be overridden and this is what get's > called instead during conversion? Yes, the thought I had was to have a virtual function in TargetCodeGenInfo that would be called first thing in EmitFixedPointConversion, and if it returns non-null it uses that value instead. It's a bit unfortunate in this instance as the only thing that doesn't work for us is the saturation, but letting you configure *parts* of the emission is a bit too specific. In https://reviews.llvm.org/D50616#1203635, @rjmccall wrote: > If this is more than just a hobby — like if there's a backend that wants to > do serious work with matching fixed-point operations to hardware support — we > should just add real LLVM IR support for fixed-point types instead of adding > a bunch of frontend customization hooks. I don't know what better > opportunity we're expecting might come along to justify that, and I don't > think it's some incredibly onerous task to add a new leaf to the LLVM type > hierarchy. Honestly, LLVM programmers across the board have become far too > accustomed to pushing things opaquely through an uncooperative IR with an > obscure muddle of intrinsics. > > In the meantime, we can continue working on this code. Even if there's > eventually real IR support for fixed-point types, this code will still be > useful; it'll just become the core of some legalization pass in the generic > backend. It definitely is; our downstream target requires it. As far as I know there's no real use case upstream, which is why it's likely quite hard to add any extensions to LLVM proper. Our implementation is done through intrinsics rather than an extension of the type system, as fixed-point numbers are really just integers under the hood. We've always wanted to upstream our fixed-point support (or some reasonable adaptation of it) but never gotten the impression that it would be possible since there's no upstream user. A mail I sent to the mailing list a while back has more details on our implementation, including what intrinsics we've added: http://lists.llvm.org/pipermail/cfe-dev/2018-May/058019.html We also have an LLVM pass that converts these intrinsics into pure IR, but it's likely that it won't work properly with some parts of this upstream design. Leonard's original proposal for this work has always been Clang-centric and never planned for implementing anything on the LLVM side, so we need to make due with what we get, but we would prefer as small a diff from upstream as possible. Comment at: lib/CodeGen/CGExprScalar.cpp:331 + SourceLocation Loc); + /// Emit a conversion from the specified complex type to the specified leonardchan wrote: > ebevhan wrote: > > What's the plan for the other conversions (int<->fix, float<->fix)? > > Functions for those as well? > > > > What about `EmitScalarConversion`? If it cannot handle conversions of > > fixed-point values it should probably be made to assert, since it will > > likely mess up. > Ideally, my plan was to have separate functions for each cast since it seems > the logic for each of them is unique, other than saturation which may be > abstracted to its own function and be used by the others. > > I wasn't sure if I should also place the logic for these casts in > `EmitScalarConversion` since `EmitScalarConversion` seemed pretty large > enough and thought it was easier if we instead had separate, self contained > functions for each fixed point conversion. But I'm open for hearing ideas on > different ways on implementing them. > > Will add the assertion. Yes, you have a point. Our EmitScalarConversion does all of the fixed-point stuff at once and it's a pretty big mess. It might be a bit confusing for users if they use EmitScalarConversion, expecting it to work on fixed-point scalars as well but then it doesn't work properly. So long as it asserts to tell them that, it should be fine but someone else might have an opinion on the expected behavior of the function. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
rjmccall added a comment. In https://reviews.llvm.org/D50616#1203446, @leonardchan wrote: > In https://reviews.llvm.org/D50616#1202034, @ebevhan wrote: > > > I think I've mentioned this before, but I would like to discuss the > > possibility of adding a target hook(s) for some of these operations > > (conversions, arithmetic when it comes). Precisely matching the emitted > > saturation operation is virtually impossible for a target. > > > Sorry I forgot to address this also. Just to make sure I understand this > correctly since I haven't used these before: target hooks are essentially for > emitting target specific code for some operations right? Does this mean that > the `EmitFixedPointConversion` function should be moved to a virtual method > under `TargetCodeGenInfo` that can be overridden and this is what get's > called instead during conversion? If this is more than just a hobby — like if there's a backend that wants to do serious work with matching fixed-point operations to hardware support — we should just add real LLVM IR support for fixed-point types instead of adding a bunch of frontend customization hooks. I don't know what better opportunity we're expecting might come along to justify that, and I don't think it's some incredibly onerous task to add a new leaf to the LLVM type hierarchy. Honestly, LLVM programmers across the board have become far too accustomed to pushing things opaquely through an uncooperative IR with an obscure muddle of intrinsics. In the meantime, we can continue working on this code. Even if there's eventually real IR support for fixed-point types, this code will still be useful; it'll just become the core of some legalization pass in the generic backend. Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan added a comment. In https://reviews.llvm.org/D50616#1202034, @ebevhan wrote: > I think I've mentioned this before, but I would like to discuss the > possibility of adding a target hook(s) for some of these operations > (conversions, arithmetic when it comes). Precisely matching the emitted > saturation operation is virtually impossible for a target. Sorry I forgot to address this also. Just to make sure I understand this correctly since I haven't used these before: target hooks are essentially for emitting target specific code for some operations right? Does this mean that the `EmitFixedPointConversion` function should be moved to a virtual method under `TargetCodeGenInfo` that can be overridden and this is what get's called instead during conversion? Repository: rC Clang https://reviews.llvm.org/D50616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:331
+ SourceLocation Loc);
+
/// Emit a conversion from the specified complex type to the specified
ebevhan wrote:
> What's the plan for the other conversions (int<->fix, float<->fix)? Functions
> for those as well?
>
> What about `EmitScalarConversion`? If it cannot handle conversions of
> fixed-point values it should probably be made to assert, since it will likely
> mess up.
Ideally, my plan was to have separate functions for each cast since it seems
the logic for each of them is unique, other than saturation which may be
abstracted to its own function and be used by the others.
I wasn't sure if I should also place the logic for these casts in
`EmitScalarConversion` since `EmitScalarConversion` seemed pretty large enough
and thought it was easier if we instead had separate, self contained functions
for each fixed point conversion. But I'm open for hearing ideas on different
ways on implementing them.
Will add the assertion.
Comment at: lib/CodeGen/CGExprScalar.cpp:1052
+} else if (IsSigned && !DstFPSema.isSigned()) {
+ Value *Zero =
+ ConstantInt::get(CGF.getLLVMContext(), APInt(ResultWidth, 0));
ebevhan wrote:
> `ConstantInt::getNullValue`?
I did not know about this method. Thanks!
Comment at: lib/Sema/Sema.cpp:537
+ case Type::STK_FixedPoint:
+llvm_unreachable("Unknown cast from FixedPoint to boolean");
}
ebevhan wrote:
> Will we want to support this?
I imagine we would want `_Bool` conversions since logical negation works on
fixed point types and more people would probably assume by default for it to
also implicitly be converted to a boolean as opposed to not.
I also don't think implementing in a later patch this will be too hard.
Repository:
rC Clang
https://reviews.llvm.org/D50616
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan updated this revision to Diff 161087.
leonardchan marked 6 inline comments as done.
leonardchan added a comment.
- Added separate case for conversions to a non-saturated type
Repository:
rC Clang
https://reviews.llvm.org/D50616
Files:
include/clang/AST/OperationKinds.def
include/clang/AST/Type.h
include/clang/Basic/DiagnosticCommonKinds.td
lib/AST/Expr.cpp
lib/AST/ExprConstant.cpp
lib/AST/Type.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprComplex.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/Edit/RewriteObjCFoundationAPI.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExpr.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
test/Frontend/fixed_point_conversions.c
test/Frontend/fixed_point_unknown_conversions.c
Index: test/Frontend/fixed_point_unknown_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_unknown_conversions.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -verify -ffixed-point %s
+
+void func() {
+ _Bool b;
+ char c;
+ int i;
+ float f;
+ double d;
+ double _Complex dc;
+ int _Complex ic;
+ struct S {
+int i;
+ } s;
+ enum E {
+A
+ } e;
+ int *ptr;
+ typedef int int_t;
+ int_t i2;
+
+ _Accum accum;
+ _Fract fract = accum; // ok
+ _Accum *accum_ptr;
+
+ accum = b; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = f; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ accum = d; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ accum = dc; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ accum = ic; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ accum = s; // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
+ accum = e; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ accum = ptr; // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
+ accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
+ accum = i2; // expected-error{{conversion between fixed point and 'int_t' (aka 'int') is not yet supported}}
+
+ b = accum; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ c = accum; // expected-error{{conversion between fixed point and 'char' is not yet supported}}
+ i = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ f = accum; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ d = accum; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ dc = accum; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ ic = accum; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ s = accum; // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
+ e = accum; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ ptr = accum; // expected-error{{assigning to 'int *' from incompatible type '_Accum'}}
+ ptr = accum_ptr; // expected-warning{{incompatible pointer types assigning to 'int *' from '_Accum *'}}
+ i2 = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+}
Index: test/Frontend/fixed_point_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_conversions.c
@@ -0,0 +1,283 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+
+void TestFixedPointCastSameType() {
+ _Accum a = 2.5k;
+ _Accum a2 = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+
+ a2 = (_Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+}
+
+void TestFixedPointCastDown() {
+ long _Accum la = 2.5lk;
+ _Accum a = la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[ACCUM_AS_I64:%[0-9]+]] = ashr i64 [[LACCUM]], 16
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = trunc i64 [[ACCUM_AS_I64]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a, align 4
+
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
ebevhan added a comment.
I think I've mentioned this before, but I would like to discuss the possibility
of adding a target hook(s) for some of these operations (conversions,
arithmetic when it comes). Precisely matching the emitted saturation operation
is virtually impossible for a target.
Comment at: lib/CodeGen/CGExprScalar.cpp:331
+ SourceLocation Loc);
+
/// Emit a conversion from the specified complex type to the specified
What's the plan for the other conversions (int<->fix, float<->fix)? Functions
for those as well?
What about `EmitScalarConversion`? If it cannot handle conversions of
fixed-point values it should probably be made to assert, since it will likely
mess up.
Comment at: lib/CodeGen/CGExprScalar.cpp:1017
+// Need to extend first before scaling up
+ResultWidth = SrcWidth + DstScale - SrcScale;
+llvm::Type *UpscaledTy = Builder.getIntNTy(ResultWidth);
It is definitely simpler to always do the 'upscale+resize/downscale,
[saturate], resize' in the constant evaluation case, but when emitting IR it is
not as efficient. There's no need to keep the upshifted bits if you aren't
interested in saturation, so in the non-saturating case it is better to keep
everything in the native type sizes with '[downscale], resize, [upscale]'. This
is why there are a bunch of 'sext i32 to i33' in the test cases.
Perhaps something like
```
if !dst.saturating
downscale if needed
resize
upscale if needed
return
old code here, minus the 'DstFPSema.isSaturated()' which is implied
```
It gives a bit of duplication (or at least similar code) but I think it's an
important disambiguation to make.
Comment at: lib/CodeGen/CGExprScalar.cpp:1052
+} else if (IsSigned && !DstFPSema.isSigned()) {
+ Value *Zero =
+ ConstantInt::get(CGF.getLLVMContext(), APInt(ResultWidth, 0));
`ConstantInt::getNullValue`?
Comment at: lib/Sema/Sema.cpp:537
+ case Type::STK_FixedPoint:
+llvm_unreachable("Unknown cast from FixedPoint to boolean");
}
Will we want to support this?
Repository:
rC Clang
https://reviews.llvm.org/D50616
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:1034
+ if (DstFPSema.isSaturated() &&
+ (CGF.getContext().getCorrespondingSaturatedType(SrcTy) != DstTy)) {
+auto Mask = APInt::getBitsSetFrom(
rjmccall wrote:
> leonardchan wrote:
> > rjmccall wrote:
> > > Why is this condition based on the formal types exactly matching rather
> > > than something about the FP semantics being different? Formal types can
> > > correspond to the same format, right?
> > >
> > > We need to check for saturation if we're either (1) decreasing the
> > > magnitude of the highest usable bit or (2) going signed->unsigned, (2)
> > > we're going signed->unsigned, or (3) we're going unsigned->signed without
> > > increasing the number of integral bits. And I'd expect the checks we
> > > have to do in each case to be different.
> > For simplicity, I more or less copied the logic from
> > `APFixedPoint::convert()` which performs a saturation check that covers all
> > of these cases if the destination semantics were saturated.
> >
> > Added another condition that checks if we need to perform saturation
> > checks. I think your (1) and (3) might be the same thing since I think we
> > only really need to check if the magnitude decreases or if going from
> > signed -> unsigned.
> >
> > I think though that the IR emission would be the same since both cases will
> > require checking for a change in the magnitude (via the mask). The only
> > difference is that if going from signed->unsigned, the min saturation is
> > zero if the value is negative.
> Wow, sorry for the edit failure in that review comment. You're right, it
> should've been just (1) and the first (2).
>
> Are there no fixed-point formats for which the range doesn't go up to
> (almost) 1? I guess there probably aren't.
No problem. The smallest range that a fixed point type can cover is the
`_Fract` type which covers [-1, 1).
Comment at: lib/CodeGen/CGExprScalar.cpp:1044
+
+Value *IsNegative = nullptr;
+if (Mask != 0) {
rjmccall wrote:
> I'm sorry, but this code is really impenetrable. The variable names are
> non-descriptive, and there are a lot of uncommented dependencies between
> values, like how `IsNegative` propagates out, and like how it's checking
> without explanation that there's not a magnitude change using whether the
> mask ends up being all-zero. Please just assign the two components of
> `ShouldCheckSaturation` to reasonably-named local variables and then use
> those to guide the code-generation here.
>
> Also, the code being generated here is pretty weird. I'm not sure the mask
> is helping; it might both produce better code and be easier to understand if
> you just broke it down into cases, like this:
>
> ```
> if a magnitude check is required {
> auto Max = maximum value of dest type;
> auto TooHigh = IsSigned ? Builder.CreateICmpSGT(Result, Max) :
> Builder.CreateICmpUGT(Result, Max);
> Result = Builder.CreateSelect(TooHigh, Max, Result);
> }
>
> if signed -> unsigned {
> auto Zero = zero value of dest type;
> Result = Builder.CreateSelect(Builder.CreateICmpSLT(Result, Zero), Zero,
> Result);
> } else if (IsSigned) {
> auto Min = minimum value of dest type;
> Result = Builder.CreateSelect(Builder.CreateICmpSLT(Result, Min), Min,
> Result);
> }
> ```
My bad. I guess it seemed intuitive at first but can see how it's difficult to
read.
This is the full logic and reasoning for the mask:
https://reviews.llvm.org/D48661?id=153426#inline-427647
But to summarize, it's essentially for checking if the bits above the MSB
changed which would indicate saturation needs to occur.
- `Mask` is for getting the bits above the integral bits in the resulting type
(for signed types, this also captures the sign bit)
- `Masked` is the bits above the highest integral bit in the resulting type
- `Masked == Mask` indicates that all the bits above the highest integral bit
were ones (the value is negative) and none of them changed (no change in
magnitude)
- `Mask == 0` indicates that all the bits above the highest integral bit were
zeros (the value is non-negative) and none of them changed (no change in
magnitude)
- `Masked == Mask || Mask == 0` therefore indicates no change in magnitude
- `!(Masked == Mask || Mask == 0)` indicates a change in magnitude and we
should saturate (but to save an extra instruction, this was emitted as `Masked
!= Mask && Mask != 0`
- If we saturate, `Mask` also happens to be the min value we saturate to for
signed types and `~Mask` is also the max value we saturate to.
The reason for the `IsNegative` laid out like that is to prevent from emitting
an extra instruction for checking a negative value.
Repository:
rC Clang
https://reviews.llvm.org/D50616
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan updated this revision to Diff 160964.
leonardchan marked 7 inline comments as done.
leonardchan added a comment.
- Reworked logic for saturation
Repository:
rC Clang
https://reviews.llvm.org/D50616
Files:
include/clang/AST/OperationKinds.def
include/clang/AST/Type.h
include/clang/Basic/DiagnosticCommonKinds.td
lib/AST/Expr.cpp
lib/AST/ExprConstant.cpp
lib/AST/Type.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprComplex.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/Edit/RewriteObjCFoundationAPI.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExpr.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
test/Frontend/fixed_point_conversions.c
test/Frontend/fixed_point_unknown_conversions.c
Index: test/Frontend/fixed_point_unknown_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_unknown_conversions.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -verify -ffixed-point %s
+
+void func() {
+ _Bool b;
+ char c;
+ int i;
+ float f;
+ double d;
+ double _Complex dc;
+ int _Complex ic;
+ struct S {
+int i;
+ } s;
+ enum E {
+A
+ } e;
+ int *ptr;
+ typedef int int_t;
+ int_t i2;
+
+ _Accum accum;
+ _Fract fract = accum; // ok
+ _Accum *accum_ptr;
+
+ accum = b; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = f; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ accum = d; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ accum = dc; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ accum = ic; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ accum = s; // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
+ accum = e; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ accum = ptr; // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
+ accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
+ accum = i2; // expected-error{{conversion between fixed point and 'int_t' (aka 'int') is not yet supported}}
+
+ b = accum; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ c = accum; // expected-error{{conversion between fixed point and 'char' is not yet supported}}
+ i = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ f = accum; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ d = accum; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ dc = accum; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ ic = accum; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ s = accum; // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
+ e = accum; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ ptr = accum; // expected-error{{assigning to 'int *' from incompatible type '_Accum'}}
+ ptr = accum_ptr; // expected-warning{{incompatible pointer types assigning to 'int *' from '_Accum *'}}
+ i2 = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+}
Index: test/Frontend/fixed_point_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_conversions.c
@@ -0,0 +1,294 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+
+void TestFixedPointCastSameType() {
+ _Accum a = 2.5k;
+ _Accum a2 = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+
+ a2 = (_Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+}
+
+void TestFixedPointCastDown() {
+ long _Accum la = 2.5lk;
+ _Accum a = la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[ACCUM_AS_I64:%[0-9]+]] = ashr i64 [[LACCUM]], 16
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = trunc i64 [[ACCUM_AS_I64]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a, align 4
+
+ a = (_Accum)la;
+ // DEFAU
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:1042
+std::min(DstScale + DstFPSema.getIntegralBits(), ResultWidth));
+Value *Zero = ConstantInt::get(CGF.getLLVMContext(), APInt(ResultWidth,
0));
+
You can just pass 0 here and it'll be zero-extended to the necessary width.
Comment at: lib/CodeGen/CGExprScalar.cpp:1044
+
+Value *IsNegative = nullptr;
+if (Mask != 0) {
I'm sorry, but this code is really impenetrable. The variable names are
non-descriptive, and there are a lot of uncommented dependencies between
values, like how `IsNegative` propagates out, and like how it's checking
without explanation that there's not a magnitude change using whether the mask
ends up being all-zero. Please just assign the two components of
`ShouldCheckSaturation` to reasonably-named local variables and then use those
to guide the code-generation here.
Also, the code being generated here is pretty weird. I'm not sure the mask is
helping; it might both produce better code and be easier to understand if you
just broke it down into cases, like this:
```
if a magnitude check is required {
auto Max = maximum value of dest type;
auto TooHigh = IsSigned ? Builder.CreateICmpSGT(Result, Max) :
Builder.CreateICmpUGT(Result, Max);
Result = Builder.CreateSelect(TooHigh, Max, Result);
}
if signed -> unsigned {
auto Zero = zero value of dest type;
Result = Builder.CreateSelect(Builder.CreateICmpSLT(Result, Zero), Zero,
Result);
} else if (IsSigned) {
auto Min = minimum value of dest type;
Result = Builder.CreateSelect(Builder.CreateICmpSLT(Result, Min), Min,
Result);
}
```
Comment at: lib/CodeGen/CGExprScalar.cpp:1034
+ if (DstFPSema.isSaturated() &&
+ (CGF.getContext().getCorrespondingSaturatedType(SrcTy) != DstTy)) {
+auto Mask = APInt::getBitsSetFrom(
leonardchan wrote:
> rjmccall wrote:
> > Why is this condition based on the formal types exactly matching rather
> > than something about the FP semantics being different? Formal types can
> > correspond to the same format, right?
> >
> > We need to check for saturation if we're either (1) decreasing the
> > magnitude of the highest usable bit or (2) going signed->unsigned, (2)
> > we're going signed->unsigned, or (3) we're going unsigned->signed without
> > increasing the number of integral bits. And I'd expect the checks we have
> > to do in each case to be different.
> For simplicity, I more or less copied the logic from
> `APFixedPoint::convert()` which performs a saturation check that covers all
> of these cases if the destination semantics were saturated.
>
> Added another condition that checks if we need to perform saturation checks.
> I think your (1) and (3) might be the same thing since I think we only really
> need to check if the magnitude decreases or if going from signed -> unsigned.
>
> I think though that the IR emission would be the same since both cases will
> require checking for a change in the magnitude (via the mask). The only
> difference is that if going from signed->unsigned, the min saturation is zero
> if the value is negative.
Wow, sorry for the edit failure in that review comment. You're right, it
should've been just (1) and the first (2).
Are there no fixed-point formats for which the range doesn't go up to (almost)
1? I guess there probably aren't.
Comment at: lib/Sema/SemaExpr.cpp:5889
+case Type::STK_MemberPointer:
+ llvm_unreachable("Unimplemented conversion from FixedPoint to type");
+}
leonardchan wrote:
> rjmccall wrote:
> > Is there something I'm missing that actually diagnoses the unimplemented
> > cases here? There's a lot of code that seems to assume that any two
> > arithmetic types can be converted to each other, and we do prefer not to
> > crash the compiler, especially on valid code.
> The plan was to implement these in other patches. I wasn't sure if
> `llvm_unreachable()` was ok to use as a placeholder for features that will
> eventually be implemented.
>
> Added diagnostics here for now to be eventually removed once these casts are
> implemented in later patches.
You can still use `llvm_unreachable` for the cases here that aren't arithmetic
types, namely all the pointer types.
Repository:
rC Clang
https://reviews.llvm.org/D50616
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan updated this revision to Diff 160884.
leonardchan marked 3 inline comments as done.
leonardchan added a comment.
- Added check for if we should check for saturation when converting to a
saturated fixed point type.
- Replaced `llvm_unreachable()`s with temporary diagnostic to be eventually
replaced when the conversions get implemented.
Repository:
rC Clang
https://reviews.llvm.org/D50616
Files:
include/clang/AST/OperationKinds.def
include/clang/AST/Type.h
include/clang/Basic/DiagnosticCommonKinds.td
lib/AST/Expr.cpp
lib/AST/ExprConstant.cpp
lib/AST/Type.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprComplex.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/Edit/RewriteObjCFoundationAPI.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExpr.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
test/Frontend/fixed_point_conversions.c
test/Frontend/fixed_point_unknown_conversions.c
Index: test/Frontend/fixed_point_unknown_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_unknown_conversions.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 -verify -ffixed-point %s
+
+void func() {
+ _Bool b;
+ char c;
+ int i;
+ float f;
+ double d;
+ double _Complex dc;
+ int _Complex ic;
+ struct S {
+int i;
+ } s;
+ enum E {
+A
+ } e;
+ int *ptr;
+ typedef int int_t;
+ int_t i2;
+
+ _Accum accum;
+ _Fract fract = accum; // ok
+ _Accum *accum_ptr;
+
+ accum = b; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = i; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ accum = f; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ accum = d; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ accum = dc; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ accum = ic; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ accum = s; // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
+ accum = e; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ accum = ptr; // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
+ accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
+ accum = i2; // expected-error{{conversion between fixed point and 'int_t' (aka 'int') is not yet supported}}
+
+ b = accum; // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
+ c = accum; // expected-error{{conversion between fixed point and 'char' is not yet supported}}
+ i = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+ f = accum; // expected-error{{conversion between fixed point and 'float' is not yet supported}}
+ d = accum; // expected-error{{conversion between fixed point and 'double' is not yet supported}}
+ dc = accum; // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
+ ic = accum; // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
+ s = accum; // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
+ e = accum; // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
+ ptr = accum; // expected-error{{assigning to 'int *' from incompatible type '_Accum'}}
+ ptr = accum_ptr; // expected-warning{{incompatible pointer types assigning to 'int *' from '_Accum *'}}
+ i2 = accum; // expected-error{{conversion between fixed point and 'int' is not yet supported}}
+}
Index: test/Frontend/fixed_point_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_conversions.c
@@ -0,0 +1,326 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+
+void TestFixedPointCastSameType() {
+ _Accum a = 2.5k;
+ _Accum a2 = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+
+ a2 = (_Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+}
+
+void TestFixedPointCastDown() {
+ long _Accum la = 2.5lk;
+ _Accum a = la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[ACCUM_AS_I64:%[0-9]
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:1016
+ if (DstScale > SrcScale) {
+// Need to allocate space before shifting left
+ResultWidth = SrcWidth + DstScale - SrcScale;
rjmccall wrote:
> In IR, this isn't really "allocating" space.
My bad. Poor comment wording.
Comment at: lib/CodeGen/CGExprScalar.cpp:1034
+ if (DstFPSema.isSaturated() &&
+ (CGF.getContext().getCorrespondingSaturatedType(SrcTy) != DstTy)) {
+auto Mask = APInt::getBitsSetFrom(
rjmccall wrote:
> Why is this condition based on the formal types exactly matching rather than
> something about the FP semantics being different? Formal types can
> correspond to the same format, right?
>
> We need to check for saturation if we're either (1) decreasing the magnitude
> of the highest usable bit or (2) going signed->unsigned, (2) we're going
> signed->unsigned, or (3) we're going unsigned->signed without increasing the
> number of integral bits. And I'd expect the checks we have to do in each
> case to be different.
For simplicity, I more or less copied the logic from `APFixedPoint::convert()`
which performs a saturation check that covers all of these cases if the
destination semantics were saturated.
Added another condition that checks if we need to perform saturation checks. I
think your (1) and (3) might be the same thing since I think we only really
need to check if the magnitude decreases or if going from signed -> unsigned.
I think though that the IR emission would be the same since both cases will
require checking for a change in the magnitude (via the mask). The only
difference is that if going from signed->unsigned, the min saturation is zero
if the value is negative.
Comment at: lib/Sema/SemaExpr.cpp:5889
+case Type::STK_MemberPointer:
+ llvm_unreachable("Unimplemented conversion from FixedPoint to type");
+}
rjmccall wrote:
> Is there something I'm missing that actually diagnoses the unimplemented
> cases here? There's a lot of code that seems to assume that any two
> arithmetic types can be converted to each other, and we do prefer not to
> crash the compiler, especially on valid code.
The plan was to implement these in other patches. I wasn't sure if
`llvm_unreachable()` was ok to use as a placeholder for features that will
eventually be implemented.
Added diagnostics here for now to be eventually removed once these casts are
implemented in later patches.
Repository:
rC Clang
https://reviews.llvm.org/D50616
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprScalar.cpp:1016
+ if (DstScale > SrcScale) {
+// Need to allocate space before shifting left
+ResultWidth = SrcWidth + DstScale - SrcScale;
In IR, this isn't really "allocating" space.
Comment at: lib/CodeGen/CGExprScalar.cpp:1034
+ if (DstFPSema.isSaturated() &&
+ (CGF.getContext().getCorrespondingSaturatedType(SrcTy) != DstTy)) {
+auto Mask = APInt::getBitsSetFrom(
Why is this condition based on the formal types exactly matching rather than
something about the FP semantics being different? Formal types can correspond
to the same format, right?
We need to check for saturation if we're either (1) decreasing the magnitude of
the highest usable bit or (2) going signed->unsigned, (2) we're going
signed->unsigned, or (3) we're going unsigned->signed without increasing the
number of integral bits. And I'd expect the checks we have to do in each case
to be different.
Comment at: lib/Sema/SemaExpr.cpp:5889
+case Type::STK_MemberPointer:
+ llvm_unreachable("Unimplemented conversion from FixedPoint to type");
+}
Is there something I'm missing that actually diagnoses the unimplemented cases
here? There's a lot of code that seems to assume that any two arithmetic types
can be converted to each other, and we do prefer not to crash the compiler,
especially on valid code.
Repository:
rC Clang
https://reviews.llvm.org/D50616
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50616: [Fixed Point Arithmetic] FixedPointCast
leonardchan created this revision.
leonardchan added reviewers: ebevhan, phosek, mcgrathr, jakehehrlich.
leonardchan added a project: clang.
This patch is a part of https://reviews.llvm.org/D48456 in an attempt to split
them up. This contains the code for casting between fixed point types and other
fixed point types.
The method for converting between fixed point types is based off the convert()
method in APFixedPoint.
Repository:
rC Clang
https://reviews.llvm.org/D50616
Files:
include/clang/AST/OperationKinds.def
include/clang/AST/Type.h
lib/AST/Expr.cpp
lib/AST/ExprConstant.cpp
lib/AST/Type.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprComplex.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/Edit/RewriteObjCFoundationAPI.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExpr.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
test/Frontend/fixed_point_conversions.c
Index: test/Frontend/fixed_point_conversions.c
===
--- /dev/null
+++ test/Frontend/fixed_point_conversions.c
@@ -0,0 +1,326 @@
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang_cc1 -ffixed-point -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+
+void TestFixedPointCastSameType() {
+ _Accum a = 2.5k;
+ _Accum a2 = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+
+ a2 = (_Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a2, align 4
+}
+
+void TestFixedPointCastDown() {
+ long _Accum la = 2.5lk;
+ _Accum a = la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[ACCUM_AS_I64:%[0-9]+]] = ashr i64 [[LACCUM]], 16
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = trunc i64 [[ACCUM_AS_I64]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a, align 4
+
+ a = (_Accum)la;
+ // DEFAULT: [[LACCUM:%[0-9]+]] = load i64, i64* %la, align 8
+ // DEFAULT-NEXT: [[ACCUM_AS_I64:%[0-9]+]] = ashr i64 [[LACCUM]], 16
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = trunc i64 [[ACCUM_AS_I64]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM]], i32* %a, align 4
+
+ short _Accum sa = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: [[SACCUM_AS_I32:%[0-9]+]] = ashr i32 [[ACCUM]], 8
+ // DEFAULT-NEXT: [[SACCUM:%[0-9]+]] = trunc i32 [[SACCUM_AS_I32]] to i16
+ // DEFAULT-NEXT: store i16 [[SACCUM]], i16* %sa, align 2
+
+ sa = (short _Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: [[SACCUM_AS_I32:%[0-9]+]] = ashr i32 [[ACCUM]], 8
+ // DEFAULT-NEXT: [[SACCUM:%[0-9]+]] = trunc i32 [[SACCUM_AS_I32]] to i16
+ // DEFAULT-NEXT: store i16 [[SACCUM]], i16* %sa, align 2
+}
+
+void TestFixedPointCastUp() {
+ short _Accum sa = 2.5hk;
+ _Accum a = sa;
+ // DEFAULT: [[SACCUM:%[0-9]+]] = load i16, i16* %sa, align 2
+ // DEFAULT-NEXT: [[SACCUM_BUFF:%[0-9]+]] = sext i16 [[SACCUM]] to i24
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = shl i24 [[SACCUM_BUFF]], 8
+ // DEFAULT-NEXT: [[ACCUM_EXT:%[0-9]+]] = sext i24 [[ACCUM]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM_EXT]], i32* %a, align 4
+
+ long _Accum la = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: [[ACCUM_BUFF:%[0-9]+]] = sext i32 [[ACCUM]] to i48
+ // DEFAULT-NEXT: [[LACCUM:%[0-9]+]] = shl i48 [[ACCUM_BUFF]], 16
+ // DEFAULT-NEXT: [[LACCUM_EXT:%[0-9]+]] = sext i48 [[LACCUM]] to i64
+ // DEFAULT-NEXT: store i64 [[LACCUM_EXT]], i64* %la, align 8
+
+ a = (_Accum)sa;
+ // DEFAULT: [[SACCUM:%[0-9]+]] = load i16, i16* %sa, align 2
+ // DEFAULT-NEXT: [[SACCUM_BUFF:%[0-9]+]] = sext i16 [[SACCUM]] to i24
+ // DEFAULT-NEXT: [[ACCUM:%[0-9]+]] = shl i24 [[SACCUM_BUFF]], 8
+ // DEFAULT-NEXT: [[ACCUM_EXT:%[0-9]+]] = sext i24 [[ACCUM]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM_EXT]], i32* %a, align 4
+
+ la = (long _Accum)a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: [[ACCUM_BUFF:%[0-9]+]] = sext i32 [[ACCUM]] to i48
+ // DEFAULT-NEXT: [[LACCUM:%[0-9]+]] = shl i48 [[ACCUM_BUFF]], 16
+ // DEFAULT-NEXT: [[LACCUM_EXT:%[0-9]+]] = sext i48 [[LACCUM]] to i64
+ // DEFAULT-NEXT: store i64 [[LACCUM_EXT]], i64* %la, align 8
+}
+
+void TestFixedPointCastSignedness() {
+ _Accum a = 2.5k;
+ unsigned _Accum ua = a;
+ // DEFAULT: [[ACCUM:%[0-9]+]] = load i32, i32* %a, align 4
+ // DEFAULT-NEXT: [[ACCUM_EXT:%[0-9]+]] = sext i32 [[ACCUM]] to i33
+ // DEFAULT-NEXT: [[UACCUM:%[0-9]+]] = shl i33 [[ACCUM_EXT]], 1
+ // DEFAULT-NEXT: [[ACCUM_TRUNC:%[0-9]+]] = trunc i33 [[UACCUM]] to i32
+ // DEFAULT-NEXT: store i32 [[ACCUM_TRUNC]], i32* %ua, align 4
+ // SAME: TestFixedPointCastSignedness
+ // SAME: [[ACCUM:%[0-9]+]] = load i32, i32* %
