[PATCH] D46915: [Fixed Point Arithmetic] Set Fixed Point Precision Bits and Create Fixed Point Literals
leonardchan updated this revision to Diff 150248.
leonardchan marked 20 inline comments as done.
leonardchan added a comment.
- Moved definition of the fractional/integral bits to TargetInfo. Checks are
also performed to make sure any target specific definitions of the F/IBits
follow the restrictions set in clause 6.2.6.3 in N1169.
- Added flag `-fsame-fbits` to force the number of fractional bits in unsigned
types to be the same as those of their their corresponding signed types. The
default values for all fractional bits is of signed fixed point types are one
less than the default values for their unsigned counterparts.
- Implemented custom parsing for fixed point literals and added tests for them.
Both decimal and hexadecimal exponentiation are supported.
- I do not have many examples involving assignment to saturated types because
this patch does not include conversions yet between fixed point types. This
will be in a future patch. The same goes for testing the minimum values for
each type because I have not yet implemented folding on binary expressions
involving fixed point types.
Repository:
rC Clang
https://reviews.llvm.org/D46915
Files:
include/clang/AST/ASTContext.h
include/clang/AST/Expr.h
include/clang/AST/OperationKinds.def
include/clang/AST/RecursiveASTVisitor.h
include/clang/AST/Type.h
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/StmtNodes.td
include/clang/Basic/TargetInfo.h
include/clang/Driver/Options.td
include/clang/Lex/LiteralSupport.h
lib/AST/ASTContext.cpp
lib/AST/ASTDumper.cpp
lib/AST/Expr.cpp
lib/AST/ExprClassification.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/StmtPrinter.cpp
lib/AST/StmtProfile.cpp
lib/AST/Type.cpp
lib/Basic/TargetInfo.cpp
lib/Basic/Targets.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprAgg.cpp
lib/CodeGen/CGExprComplex.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CGExprScalar.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Edit/RewriteObjCFoundationAPI.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Lex/LiteralSupport.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExceptionSpec.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/TreeTransform.h
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_declarations.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_same_fbits.c
test/Frontend/fixed_point_validation.c
tools/libclang/CXCursor.cpp
Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -305,6 +305,10 @@
K = CXCursor_IntegerLiteral;
break;
+ case Stmt::FixedPointLiteralClass:
+llvm_unreachable("No cursor for FixedPointLiteralClass");
+break;
+
case Stmt::FloatingLiteralClass:
K = CXCursor_FloatingLiteral;
break;
Index: test/Frontend/fixed_point_validation.c
===
--- /dev/null
+++ test/Frontend/fixed_point_validation.c
@@ -0,0 +1,19 @@
+// RUN: %clang -ffixed-point -S -emit-llvm -o - %s | lli -force-interpreter=true
+
+// Run simple validation tests
+
+#define assert(b) if (!(b)) { return 1; }
+
+int main(){
+ short _Accum s_accum = 0.0hk;
+ short _Accum s_accum2 = 2.0hk;
+ short _Fract s_fract = 0.999hr;
+ short _Fract s_fract2 = -0.999hr;
+
+ assert(s_accum == 0);
+
+ s_accum = s_accum2;
+
+ assert(s_accum == s_accum2);
+ assert(s_accum == 2);
+}
Index: test/Frontend/fixed_point_same_fbits.c
===
--- /dev/null
+++ test/Frontend/fixed_point_same_fbits.c
@@ -0,0 +1,28 @@
+// RUN: %clang -ffixed-point -S -emit-llvm -o - %s | FileCheck %s -check-prefix=DEFAULT
+// RUN: %clang -ffixed-point -fsame-fbits -S -emit-llvm -o - %s | FileCheck %s -check-prefix=SAME
+
+/* The scale for unsigned fixed point types should be the same as that of signed
+ * fixed point types when -fsame-fbits is enabled. */
+
+void func() {
+ unsigned short _Accum u_short_accum = 0.5uhk;
+ unsigned _Accum u_accum = 0.5uk;
+ unsigned long _Accum u_long_accum = 0.5ulk;
+ unsigned short _Fract u_short_fract = 0.5uhr;
+ unsigned _Fract u_fract = 0.5ur;
+ unsigned long _Fract u_long_fract = 0.5ulr;
+
+// DEFAULT: store i16 128, i16* %u_short_accum, align 2
+// DEFAULT: store i32 32768, i32* %u_accum, align 4
+// DEFAULT: store i64 2147483648, i64* %u_long_accum, align 8
+// DEFAULT: store i16 128, i16* %u_short_fract, align 2
+// DEFAULT: store i32 32768, i32* %u_fract, align 4
+// DEFAULT: store i64 2147483648, i64* %u_long_fract, align 8
+
+// SAME: store i16 64, i16* %u_short_accum, align 2
+// SAME: store i32 16384, i32* %u_accum, align 4
+// SAME: store i64 1073741824, i64* %u_l
[PATCH] D46915: [Fixed Point Arithmetic] Set Fixed Point Precision Bits and Create Fixed Point Literals
ebevhan added a comment.
You should not define the fixed-point precision as compiler macros at build
configure time. The number of fractional bits (the scaling factor) should be
added to TargetInfo as target-configurable variables/accessors, and an accessor
should be added to ASTContext (we call it 'getFixedPointScale') to fetch the
scaling factor of arbitrary fixed-point types.
As I mentioned on the mailing list, we would also like to solve the problem
with different scaling factors on the signed and unsigned _Fract types. I'm not
sure what the best approach is, but the simplest solution could perhaps be
enabled with a target flag.
If the flag is true, the scaling factor of the unsigned _Fract types is the
same as the signed ones and the MSB is padding. If the flag is false, the
scaling factor of the unsigned _Fract types is one greater than the signed
types and all of the bits are used.
-
Overall, the literal parsing code seems very ad-hoc. Fixed-point values are
just integers, it should be possible to process them in exact precision instead
of relying on host system implementation details (using double) and floating
point types.
I could provide patches from our downstream port, but there are a number of
hurdles:
- DSP-C does a bunch of extra 'type promotion' when parsing fixed-point literals
- literals in DSP-C cannot contain the 'exponent notation' so the parsing
routine cannot handle this
- the routine uses utility functions added to APInt in our LLVM branch
If you are interested I can share anyway so you can see how we have done it.
Comment at: include/clang/Lex/LiteralSupport.h:72
+ enum FixedPointType { FPT_UNSPECIFIED, FPT_ACCUM, FPT_FRACT };
+
Is there a reason this is not added as two fields 'isAccum' and 'isFract'?
Comment at: include/clang/Lex/LiteralSupport.h:75
+ // We use separate fields for fixed point sizes b/c the isHalf/isLong
booleans
+ // assume that this literal is an integral type instead of fixed point type.
+ enum FixedPointSize { FPS_UNSPECIFIED, FPS_SHORT, FPS_LONG };
Shouldn't the flags be amended instead?
Comment at: lib/AST/ASTContext.cpp:1788
case BuiltinType::UShortAccum:
+case BuiltinType::ShortFract:
+case BuiltinType::UShortFract:
See my comments on the _Accum patch.
Comment at: lib/AST/ASTDumper.cpp:2186
+ ColorScope Color(*this, ValueColor);
+ OS << " " << Node->getValue().toString(10, isSigned);
+}
This will not print as a fixed-point number.
Comment at: lib/AST/Expr.cpp:766
+ const auto *BT = type->getAs();
+ assert(BT && "Not a fixed point type!");
+ switch (BT->getKind()) {
Is this possible given the assertion above?
Comment at: lib/AST/Expr.cpp:767
+ assert(BT && "Not a fixed point type!");
+ switch (BT->getKind()) {
+default:
There is no reason to have this switch.
Comment at: lib/AST/Expr.cpp:774
+case BuiltinType::UShortFract:
+ assert(V.getBitWidth() == C.getIntWidth(type) &&
+ "Short fixed point type is not the correct size for constant.");
'getIntWidth' is likely the wrong accessor to use here. Fixed-point types are
technically not ints.
Comment at: lib/AST/ExprConstant.cpp:9323
+ if (Value.isSigned() && Value.isMinSignedValue() && E->canOverflow() &&
+ !HandleOverflow(Info, E, -Value.extend(Value.getBitWidth() + 1),
+ E->getType()))
Is signed fixed-point overflow actually considered undefined behavior? I'm not
familiar with what Embedded-C says about this.
Also, this routine will likely print the wrong number for fixed-point values.
Comment at: lib/AST/ExprConstant.cpp:9328
+}
+case UO_Not: {
+ if (!Visit(E->getSubExpr())) return false;
I do not believe ~ is valid on fixed-point types.
Comment at: lib/AST/StmtPrinter.cpp:1532
+ bool isSigned = Node->getType()->isSignedFixedPointType();
+ OS << Node->getValue().toString(10, isSigned);
+
This will not print a fixed-point number.
Comment at: lib/CodeGen/CGExprAgg.cpp:675
+ case CK_IntegralToFixedPoint:
+llvm_unreachable(
+"AggExprEmitter::VisitCastExpr CK_IntegralToFixedPoint"); // TODO
This probably goes in the large default case below, as fixed-point types are
not aggregates.
Comment at: lib/CodeGen/CGExprComplex.cpp:451
+llvm_unreachable(
+"ComplexExprEmitter::EmitCast CK_IntegralToFixedPoint"); // TODO
case CK_Dependent: llvm_unreachable("dependent cast kind in IR gen!");
Same as the aggregate case.
Comment at: lib/CodeGen/CGExprScalar.cpp:1789
+ case BuiltinType::ShortAccum:
+fbits = B
[PATCH] D46915: [Fixed Point Arithmetic] Set Fixed Point Precision Bits and Create Fixed Point Literals
leonardchan updated this revision to Diff 147595.
leonardchan added a comment.
formatting
Repository:
rC Clang
https://reviews.llvm.org/D46915
Files:
CMakeLists.txt
cmake/modules/InitFixedPointBits.cmake
include/clang/AST/Expr.h
include/clang/AST/OperationKinds.def
include/clang/AST/RecursiveASTVisitor.h
include/clang/AST/Type.h
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/FixedPoint.h.in
include/clang/Basic/StmtNodes.td
include/clang/Lex/LiteralSupport.h
lib/AST/ASTContext.cpp
lib/AST/ASTDumper.cpp
lib/AST/Expr.cpp
lib/AST/ExprClassification.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/StmtPrinter.cpp
lib/AST/StmtProfile.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/Lex/LiteralSupport.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExceptionSpec.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/TreeTransform.h
lib/Serialization/ASTReaderStmt.cpp
lib/Serialization/ASTWriterStmt.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/ExprEngineC.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_declarations.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_validation.c
tools/libclang/CXCursor.cpp
Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -305,6 +305,10 @@
K = CXCursor_IntegerLiteral;
break;
+ case Stmt::FixedPointLiteralClass:
+llvm_unreachable("No cursor for FixedPointLiteralClass");
+break;
+
case Stmt::FloatingLiteralClass:
K = CXCursor_FloatingLiteral;
break;
Index: test/Frontend/fixed_point_validation.c
===
--- /dev/null
+++ test/Frontend/fixed_point_validation.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -S -emit-llvm -o - %s | lli
+
+// Run simple validation tests
+
+#define assert(b) if (!(b)) { return 1; }
+
+int main(){
+ short _Accum s_accum;
+ short _Accum s_accum2 = 2.0hk;
+ short _Fract s_fract = 0.999hr;
+ short _Fract s_fract2 = -0.999hr;
+
+ assert(s_accum == 0);
+
+ s_accum = s_accum2;
+
+ assert(s_accum == s_accum2);
+ assert(s_accum == 2);
+}
Index: test/Frontend/fixed_point_errors.c
===
--- test/Frontend/fixed_point_errors.c
+++ test/Frontend/fixed_point_errors.c
@@ -1,14 +1,23 @@
// RUN: %clang_cc1 -x c -fsyntax-only -verify -pedantic %s
-long long _Accum longlong_accum; // expected-error{{'long long _Accum' is invalid}}
-unsigned long long _Accum u_longlong_accum; // expected-error{{'long long _Accum' is invalid}}
-long long _Fract longlong_fract; // expected-error{{'long long _Fract' is invalid}}
-unsigned long long _Fract u_longlong_fract; // expected-error{{'long long _Fract' is invalid}}
+long long _Accum longlong_accum; // expected-error{{'long long _Accum' is invalid}}
+unsigned long long _Accum u_longlong_accum; // expected-error{{'long long _Accum' is invalid}}
+long long _Fract longlong_fract; // expected-error{{'long long _Fract' is invalid}}
+unsigned long long _Fract u_longlong_fract; // expected-error{{'long long _Fract' is invalid}}
_Sat int i; // expected-error{{'int' cannot be saturated. Only _Fract and _Accum can.}}
_Sat _Sat _Fract fract; // expected-warning{{duplicate '_Sat' declaration specifier}}
-_Sat long long _Accum sat_longlong_accum; // expected-error{{'long long _Accum' is invalid}}
+_Sat long long _Accum sat_longlong_accum; // expected-error{{'long long _Accum' is invalid}}
_Sat unsigned long long _Accum sat_u_longlong_accum; // expected-error{{'long long _Accum' is invalid}}
-_Sat long long _Fract sat_longlong_fract; // expected-error{{'long long _Fract' is invalid}}
+_Sat long long _Fract sat_longlong_fract; // expected-error{{'long long _Fract' is invalid}}
_Sat unsigned long long _Fract sat_u_longlong_fract; // expected-error{{'long long _Fract' is invalid}}
+
+short _Fract fract2 = 1.2hr; // expected-error{{a _Fract type cannot have an integral part}}
+
+signed short _Accum s_short_accum = 129.0hk; // expected-error{{the integral part of this literal is too large for this signed _Accum type}}
+unsigned short _Accum u_short_accum = 256.0uhk; // expected-error{{the integral part of this literal is too large for this unsigned _Accum type}}
+signed _Accum s_accum = 32770.0k; // expected-error{{the integral part of this literal is too large for this signed _Accum type}}
+unsigned _Accum u_accum = 65536.0uk; // expected-error{{the integral part of this literal is too large for this unsigned _Accum type}}
+short _Accum short_accum = 129.0hk; // expected-error{{the integr
