[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333814: This diff includes changes for supporting the
following types. (authored by leonardchan, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D46084?vs=149305=149602#toc
Repository:
rL LLVM
https://reviews.llvm.org/D46084
Files:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/AST/BuiltinTypes.def
cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Basic/Specifiers.h
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/include/clang/Basic/TokenKinds.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Sema/DeclSpec.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/MicrosoftMangle.cpp
cfe/trunk/lib/AST/NSAPI.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/AST/TypeLoc.cpp
cfe/trunk/lib/Analysis/PrintfFormatString.cpp
cfe/trunk/lib/Basic/TargetInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Index/USRGeneration.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Sema/DeclSpec.cpp
cfe/trunk/lib/Sema/SemaTemplateVariadic.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/lib/Serialization/ASTCommon.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/test/Frontend/fixed_point.c
cfe/trunk/test/Frontend/fixed_point_bit_widths.c
cfe/trunk/test/Frontend/fixed_point_errors.c
cfe/trunk/test/Frontend/fixed_point_errors.cpp
cfe/trunk/test/Frontend/fixed_point_not_enabled.c
cfe/trunk/tools/libclang/CXType.cpp
Index: cfe/trunk/tools/libclang/CXType.cpp
===
--- cfe/trunk/tools/libclang/CXType.cpp
+++ cfe/trunk/tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: cfe/trunk/include/clang-c/Index.h
===
--- cfe/trunk/include/clang-c/Index.h
+++ cfe/trunk/include/clang-c/Index.h
@@ -3182,8 +3182,14 @@
CXType_Float128 = 30,
CXType_Half = 31,
CXType_Float16 = 32,
+ CXType_ShortAccum = 33,
+ CXType_Accum = 34,
+ CXType_LongAccum = 35,
+ CXType_UShortAccum = 36,
+ CXType_UAccum = 37,
+ CXType_ULongAccum = 38,
CXType_FirstBuiltin = CXType_Void,
- CXType_LastBuiltin = CXType_Float16,
+ CXType_LastBuiltin = CXType_ULongAccum,
CXType_Complex = 100,
CXType_Pointer = 101,
Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -887,6 +887,11 @@
Group, Flags<[CC1Option]>,
HelpText<"Disable support for int128_t type">;
+def ffixed_point : Flag<["-"], "ffixed-point">, Group,
+ Flags<[CC1Option]>, HelpText<"Enable fixed point types">;
+def fno_fixed_point : Flag<["-"], "fno-fixed-point">, Group,
+ HelpText<"Disable fixed point types">;
+
// Begin sanitizer flags. These should all be core options exposed in all driver
// modes.
let Flags = [CC1Option, CoreOption] in {
Index: cfe/trunk/include/clang/AST/ASTContext.h
===
--- cfe/trunk/include/clang/AST/ASTContext.h
+++ cfe/trunk/include/clang/AST/ASTContext.h
@@ -1007,6 +1007,9 @@
CanQualType UnsignedCharTy, UnsignedShortTy, UnsignedIntTy, UnsignedLongTy;
CanQualType UnsignedLongLongTy, UnsignedInt128Ty;
CanQualType FloatTy, DoubleTy, LongDoubleTy, Float128Ty;
+ CanQualType ShortAccumTy, AccumTy,
+ LongAccumTy; // ISO/IEC JTC1 SC22 WG14 N1169 Extension
+ CanQualType UnsignedShortAccumTy, UnsignedAccumTy, UnsignedLongAccumTy;
CanQualType HalfTy; // [OpenCL 6.1.1.1], ARM NEON
CanQualType Float16Ty; // C11 extension ISO/IEC TS 18661-3
CanQualType FloatComplexTy, DoubleComplexTy, LongDoubleComplexTy;
Index: cfe/trunk/include/clang/AST/BuiltinTypes.def
===
---
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added a comment. Hi all, I'll be attempting to commit this patch around 6pm PT today unless anyone has any more comments on this specific patch. Any other suggestions regarding potential design changes can be discussed in future patches since this is only the first of many. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
ebevhan added inline comments. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; rsmith wrote: > leonardchan wrote: > > leonardchan wrote: > > > ebevhan wrote: > > > > leonardchan wrote: > > > > > rsmith wrote: > > > > > > jfb wrote: > > > > > > > This seems weird because Targets which don't have these values > > > > > > > for the non-Accum versions will have .e.g. `sizeof(short) != > > > > > > > sizeof(short _Accum)`. Is there a point in ever having `_Accum` > > > > > > > differ in size, width, and alignment from the underlying type? If > > > > > > > not, can you set these values after the sub-target has specified > > > > > > > its preferences? > > > > > > I'm uncomfortable about opting all targets into this behavior with > > > > > > these default values; this will result in an ABI break if later a > > > > > > target updates these to the correct values. A per-target > > > > > > `AccumSupported` flag would help substantially. But this is OK for > > > > > > the short term while you're still working on the feature. > > > > > > > > > > > > We'll also need the target to inform us of the number of integer > > > > > > and fractional bits for each such type. > > > > > The integral and fractional bits will be set in the target and is > > > > > available in a later patch. > > > > > We'll also need the target to inform us of the number of integer and > > > > > fractional bits for each such type. > > > > > > > > I believe the only one that is needed is for the number of fractional > > > > bits; the number of integer bits is implied by the difference between > > > > the type width and fractional bits. I think I mention this in one of > > > > the other patches. > > > > > > > > > > > You're right. I was stuck in the mindset that we would be providing an > > > integral and fractional value. > > > The integral and fractional bits will be set in the target and is > > > available in a later patch. > > > > I mean just the fractional bits since the integral will just be the bit > > width minus fractional. > You're assuming that all bits will be either integral or fractional bits (and > in particular that there are no padding bits). I don't know if that's > actually going to be true for all target ABIs, but I suppose it's OK to make > this assumption until it's proven wrong by some actual target. It is actually the case for us (downstream) that the unsigned types should have one bit of padding, however, this is a special case. The spec says "Each unsigned fract type has either the same number of fractional bits as, or one more fractional bit than, its corresponding signed fract type." and also under 'recommended practice', "Each signed accum type has the same number of fractional bits as either its corresponding signed fract type or its corresponding unsigned fract type." So I think the approach would be that the integral bits are implied from the fractional bits and the width, except in the unsigned case where the MSB is a padding bit. If there is a target that really does want the unsigned types to have an extra bit of precision it could be added as a flag, but I don't really see the benefit of it. The consistency benefit of having the unsigned types have the same scaling factor as the signed ones outweighs the downsides. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
rsmith added inline comments. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; leonardchan wrote: > leonardchan wrote: > > ebevhan wrote: > > > leonardchan wrote: > > > > rsmith wrote: > > > > > jfb wrote: > > > > > > This seems weird because Targets which don't have these values for > > > > > > the non-Accum versions will have .e.g. `sizeof(short) != > > > > > > sizeof(short _Accum)`. Is there a point in ever having `_Accum` > > > > > > differ in size, width, and alignment from the underlying type? If > > > > > > not, can you set these values after the sub-target has specified > > > > > > its preferences? > > > > > I'm uncomfortable about opting all targets into this behavior with > > > > > these default values; this will result in an ABI break if later a > > > > > target updates these to the correct values. A per-target > > > > > `AccumSupported` flag would help substantially. But this is OK for > > > > > the short term while you're still working on the feature. > > > > > > > > > > We'll also need the target to inform us of the number of integer and > > > > > fractional bits for each such type. > > > > The integral and fractional bits will be set in the target and is > > > > available in a later patch. > > > > We'll also need the target to inform us of the number of integer and > > > > fractional bits for each such type. > > > > > > I believe the only one that is needed is for the number of fractional > > > bits; the number of integer bits is implied by the difference between the > > > type width and fractional bits. I think I mention this in one of the > > > other patches. > > > > > > > > You're right. I was stuck in the mindset that we would be providing an > > integral and fractional value. > > The integral and fractional bits will be set in the target and is available > > in a later patch. > > I mean just the fractional bits since the integral will just be the bit width > minus fractional. You're assuming that all bits will be either integral or fractional bits (and in particular that there are no padding bits). I don't know if that's actually going to be true for all target ABIs, but I suppose it's OK to make this assumption until it's proven wrong by some actual target. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added inline comments. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; leonardchan wrote: > ebevhan wrote: > > leonardchan wrote: > > > rsmith wrote: > > > > jfb wrote: > > > > > This seems weird because Targets which don't have these values for > > > > > the non-Accum versions will have .e.g. `sizeof(short) != sizeof(short > > > > > _Accum)`. Is there a point in ever having `_Accum` differ in size, > > > > > width, and alignment from the underlying type? If not, can you set > > > > > these values after the sub-target has specified its preferences? > > > > I'm uncomfortable about opting all targets into this behavior with > > > > these default values; this will result in an ABI break if later a > > > > target updates these to the correct values. A per-target > > > > `AccumSupported` flag would help substantially. But this is OK for the > > > > short term while you're still working on the feature. > > > > > > > > We'll also need the target to inform us of the number of integer and > > > > fractional bits for each such type. > > > The integral and fractional bits will be set in the target and is > > > available in a later patch. > > > We'll also need the target to inform us of the number of integer and > > > fractional bits for each such type. > > > > I believe the only one that is needed is for the number of fractional bits; > > the number of integer bits is implied by the difference between the type > > width and fractional bits. I think I mention this in one of the other > > patches. > > > > > You're right. I was stuck in the mindset that we would be providing an > integral and fractional value. > The integral and fractional bits will be set in the target and is available > in a later patch. I mean just the fractional bits since the integral will just be the bit width minus fractional. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added inline comments. Comment at: include/clang/Basic/TokenKinds.def:393 +// ISO/IEC JTC1 SC22 WG14 N1169 Extension +KEYWORD(_Accum , KEYALL) + rsmith wrote: > leonardchan wrote: > > jfb wrote: > > > ebevhan wrote: > > > > I believe that having KEYALL will enable the keyword even if > > > > -fno-fixed-point is given. Is this desired? It will mean that `_Accum` > > > > will not be a valid identifier in standard C regardless of the flag. > > > That seems fine: identifiers starting with underscore and a capital > > > letter already aren't valid identifiers in C and C++ because they're > > > reserved for the implementation. > > I think my test for `-fno-fixed-point` already catches this, but I did not > > notice until now the `KEYNOCXX` keyword until now. Using this instead > > allows for not having to check if the language is c++ since `_Accum` is no > > longer treated as a typename. The corresponding test checking fixed points > > in c++ has been updated to reflect this. > Just to make sure we're on the same page: it's OK to disable this feature for > C++ while you're working on it, but it needs to support C++ by the time > you're done. That's the goal. Just disabled in c++ for now with the end goal of getting c++ support. Right now I'm waiting for these types to be added in the next Itanium ABI, which I believe you responded to on Github. Afterwards I'll need to request Microsoft mangling. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; rsmith wrote: > jfb wrote: > > This seems weird because Targets which don't have these values for the > > non-Accum versions will have .e.g. `sizeof(short) != sizeof(short _Accum)`. > > Is there a point in ever having `_Accum` differ in size, width, and > > alignment from the underlying type? If not, can you set these values after > > the sub-target has specified its preferences? > I'm uncomfortable about opting all targets into this behavior with these > default values; this will result in an ABI break if later a target updates > these to the correct values. A per-target `AccumSupported` flag would help > substantially. But this is OK for the short term while you're still working > on the feature. > > We'll also need the target to inform us of the number of integer and > fractional bits for each such type. The integral and fractional bits will be set in the target and is available in a later patch. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; ebevhan wrote: > leonardchan wrote: > > rsmith wrote: > > > jfb wrote: > > > > This seems weird because Targets which don't have these values for the > > > > non-Accum versions will have .e.g. `sizeof(short) != sizeof(short > > > > _Accum)`. Is there a point in ever having `_Accum` differ in size, > > > > width, and alignment from the underlying type? If not, can you set > > > > these values after the sub-target has specified its preferences? > > > I'm uncomfortable about opting all targets into this behavior with these > > > default values; this will result in an ABI break if later a target > > > updates these to the correct values. A per-target `AccumSupported` flag > > > would help substantially. But this is OK for the short term while you're > > > still working on the feature. > > > > > > We'll also need the target to inform us of the number of integer and > > > fractional bits for each such type. > > The integral and fractional bits will be set in the target and is available > > in a later patch. > > We'll also need the target to inform us of the number of integer and > > fractional bits for each such type. > > I believe the only one that is needed is for the number of fractional bits; > the number of integer bits is implied by the difference between the type > width and fractional bits. I think I mention this in one of the other patches. > > You're right. I was stuck in the mindset that we would be providing an integral and fractional value. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 149305.
leonardchan marked 6 inline comments as done.
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/Specifiers.h
include/clang/Basic/TargetInfo.h
include/clang/Basic/TokenKinds.def
include/clang/Driver/Options.td
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/Basic/TargetInfo.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_bit_widths.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_errors.cpp
test/Frontend/fixed_point_not_enabled.c
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned _Accum u_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+ // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -x c++ -ffixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+_Accum accum; // expected-error{{unknown type name '_Accum'}}
Index: test/Frontend/fixed_point_errors.c
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -verify -ffixed-point %s
+
+/* We do not yet support long long. No recommended bit widths are given for this
+ * size. */
+
+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}}
+
+/* Although _Complex types work with floating point numbers, the extension
+ * provides no info for complex fixed point types. */
+
+_Complex signed short _Accum cmplx_s_short_accum; // expected-error{{'_Complex _Accum' is invalid}}
+_Complex signed _Accum cmplx_s_accum; // expected-error{{'_Complex _Accum' is invalid}}
+_Complex signed long _Accum cmplx_s_long_accum; // expected-error{{'_Complex _Accum' is invalid}}
+_Complex unsigned short _Accum cmplx_u_short_accum; //
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
ebevhan added inline comments. Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; rsmith wrote: > jfb wrote: > > This seems weird because Targets which don't have these values for the > > non-Accum versions will have .e.g. `sizeof(short) != sizeof(short _Accum)`. > > Is there a point in ever having `_Accum` differ in size, width, and > > alignment from the underlying type? If not, can you set these values after > > the sub-target has specified its preferences? > I'm uncomfortable about opting all targets into this behavior with these > default values; this will result in an ABI break if later a target updates > these to the correct values. A per-target `AccumSupported` flag would help > substantially. But this is OK for the short term while you're still working > on the feature. > > We'll also need the target to inform us of the number of integer and > fractional bits for each such type. > We'll also need the target to inform us of the number of integer and > fractional bits for each such type. I believe the only one that is needed is for the number of fractional bits; the number of integer bits is implied by the difference between the type width and fractional bits. I think I mention this in one of the other patches. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
rsmith accepted this revision.
rsmith added a comment.
Just minor comments. Feel free to commit after handling them.
Comment at: include/clang/Basic/LangOptions.def:301
+LANGOPT(FixedPointEnabled, 1, 0, "Fixed point types are enabled")
+
ebevhan wrote:
> Just a minor nit... The 'Enabled' is superfluous.
There's still a superfluous "are enabled" in the description. Please consult
the file comment:
```
// The Description field should be a noun phrase, for instance "frobbing all
// widgets" or "C's implicit blintz feature".
```
So just "fixed point types" would be appropriate. (And yes, many of the options
in this file get this wrong.)
Comment at: include/clang/Basic/TokenKinds.def:393
+// ISO/IEC JTC1 SC22 WG14 N1169 Extension
+KEYWORD(_Accum , KEYALL)
+
leonardchan wrote:
> jfb wrote:
> > ebevhan wrote:
> > > I believe that having KEYALL will enable the keyword even if
> > > -fno-fixed-point is given. Is this desired? It will mean that `_Accum`
> > > will not be a valid identifier in standard C regardless of the flag.
> > That seems fine: identifiers starting with underscore and a capital letter
> > already aren't valid identifiers in C and C++ because they're reserved for
> > the implementation.
> I think my test for `-fno-fixed-point` already catches this, but I did not
> notice until now the `KEYNOCXX` keyword until now. Using this instead allows
> for not having to check if the language is c++ since `_Accum` is no longer
> treated as a typename. The corresponding test checking fixed points in c++
> has been updated to reflect this.
Just to make sure we're on the same page: it's OK to disable this feature for
C++ while you're working on it, but it needs to support C++ by the time you're
done.
Comment at: include/clang/Driver/Options.td:893
+def fno_fixed_point : Flag<["-"], "fno-fixed-point">, Group,
+ Flags<[CC1Option]>, HelpText<"Disable fixed point
types">;
+
Only the flag that enables a non-default mode should be a `CC1Option`.
Comment at: include/clang/Serialization/ASTBitCodes.h:960
+
+ /// \brief OpenCL image types with auto numeration
+
Spurious comment, probably from a merge conflict?
Comment at: lib/AST/ExprConstant.cpp:7361-7362
+case BuiltinType::ULongAccum:
+ // GCC does not cover FIXED_POINT_TYPE in its switch stmt and defaults to
+ // no_type_class
+ return GCCTypeClass::None;
GCC's implementation is irrelevant; please only talk about its observable
behavior if you think this deserves a comment.
Comment at: lib/AST/ItaniumMangle.cpp:2552
+ case BuiltinType::ULongAccum:
+llvm_unreachable("Fixed point types are disabled for c++");
case BuiltinType::Half:
leonardchan wrote:
> rsmith wrote:
> > Please check what GCC uses to mangle these, and follow suit; if GCC doesn't
> > have a mangling, you can use a vendor mangling (`u6_Accum`) or produce an
> > error for now, but please open an issue at
> > https://github.com/itanium-cxx-abi/cxx-abi/ to pick a real mangling.
> It seems that GCC uses the characters for each fixed point type's
> corresponding integral type
> (https://github.com/gcc-mirror/gcc/blob/master/gcc/cp/mangle.c). Will follow
> up on this if we end up enabling fixed point types for C++.
OK for now, but as noted, this will need to be addressed eventually.
Comment at: lib/Basic/TargetInfo.cpp:45
+ AccumWidth = AccumAlign = 32;
+ LongAccumWidth = LongAccumAlign = 64;
SuitableAlign = 64;
jfb wrote:
> This seems weird because Targets which don't have these values for the
> non-Accum versions will have .e.g. `sizeof(short) != sizeof(short _Accum)`.
> Is there a point in ever having `_Accum` differ in size, width, and alignment
> from the underlying type? If not, can you set these values after the
> sub-target has specified its preferences?
I'm uncomfortable about opting all targets into this behavior with these
default values; this will result in an ABI break if later a target updates
these to the correct values. A per-target `AccumSupported` flag would help
substantially. But this is OK for the short term while you're still working on
the feature.
We'll also need the target to inform us of the number of integer and fractional
bits for each such type.
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2684
// Types added here must also be added to EmitFundamentalRTTIDescriptors.
switch (Ty->getKind()) {
leonardchan wrote:
> rsmith wrote:
> > Note this comment :)
> Returned false for these types now. Not sure if these types should also be
> added to EmitFundamentalRTTIDescriptors since the OCL types do not appear
> under there.
OK, we'll need to remember to revisit this when adding C++
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added inline comments. Comment at: include/clang/Basic/TokenKinds.def:393 +// ISO/IEC JTC1 SC22 WG14 N1169 Extension +KEYWORD(_Accum , KEYALL) + jfb wrote: > ebevhan wrote: > > I believe that having KEYALL will enable the keyword even if > > -fno-fixed-point is given. Is this desired? It will mean that `_Accum` will > > not be a valid identifier in standard C regardless of the flag. > That seems fine: identifiers starting with underscore and a capital letter > already aren't valid identifiers in C and C++ because they're reserved for > the implementation. I think my test for `-fno-fixed-point` already catches this, but I did not notice until now the `KEYNOCXX` keyword until now. Using this instead allows for not having to check if the language is c++ since `_Accum` is no longer treated as a typename. The corresponding test checking fixed points in c++ has been updated to reflect this. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 149162.
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/Specifiers.h
include/clang/Basic/TargetInfo.h
include/clang/Basic/TokenKinds.def
include/clang/Driver/Options.td
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/Basic/TargetInfo.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_bit_widths.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_errors.cpp
test/Frontend/fixed_point_not_enabled.c
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -x c -verify %s
+// RUN: %clang_cc1 -x c -fno-fixed-point -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned _Accum u_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+ // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -x c++ -ffixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+_Accum accum; // expected-error{{unknown type name '_Accum'}}
Index: test/Frontend/fixed_point_errors.c
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -verify -ffixed-point %s
+
+/* We do not yet support long long. No recommended bit widths are given for this
+ * size. */
+
+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}}
+
+/* Although _Complex types work with floating point numbers, the extension
+ * provides no info for complex fixed point types. */
+
+_Complex signed short _Accum cmplx_s_short_accum; // expected-error{{'_Complex _Accum' is invalid}}
+_Complex signed _Accum cmplx_s_accum; // expected-error{{'_Complex _Accum' is invalid}}
+_Complex signed long _Accum cmplx_s_long_accum; // expected-error{{'_Complex _Accum' is invalid}}
+_Complex unsigned short _Accum cmplx_u_short_accum; //
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
jfb added inline comments. Comment at: include/clang/Basic/TokenKinds.def:393 +// ISO/IEC JTC1 SC22 WG14 N1169 Extension +KEYWORD(_Accum , KEYALL) + ebevhan wrote: > I believe that having KEYALL will enable the keyword even if -fno-fixed-point > is given. Is this desired? It will mean that `_Accum` will not be a valid > identifier in standard C regardless of the flag. That seems fine: identifiers starting with underscore and a capital letter already aren't valid identifiers in C and C++ because they're reserved for the implementation. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
ebevhan added a comment. Sorry for the late notice; I missed something. Comment at: include/clang/Basic/TokenKinds.def:393 +// ISO/IEC JTC1 SC22 WG14 N1169 Extension +KEYWORD(_Accum , KEYALL) + I believe that having KEYALL will enable the keyword even if -fno-fixed-point is given. Is this desired? It will mean that `_Accum` will not be a valid identifier in standard C regardless of the flag. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added a comment. Hi all, I think I've addressed all comments on this patch and will be committing tomorrow morning unless anyone has any more comments they'd like to bring up. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan marked 2 inline comments as done. leonardchan added inline comments. Comment at: include/clang/Driver/Options.td:882 +def enable_fixed_point : Flag<["-", "--"], "enable-fixed-point">, Group, + Flags<[CC1Option]>, HelpText<"Enable fixed point types">; ebevhan wrote: > Shouldn't this be an `-f` flag, like `-ffixed-point`? I'm not for or against > either, just wondering what anyone else thinks. Makes sense since this flag is target independent. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148637.
leonardchan added a comment.
Changed flag names
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/Specifiers.h
include/clang/Basic/TargetInfo.h
include/clang/Basic/TokenKinds.def
include/clang/Driver/Options.td
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/Basic/TargetInfo.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_bit_widths.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_errors.cpp
test/Frontend/fixed_point_not_enabled.c
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -x c -verify %s
+// RUN: %clang_cc1 -x c -fno-fixed-point -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned _Accum u_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+ // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -ffixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned _Accum u_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{fixed point types are only allowed in C}}
+// expected-error@-1{{C++ requires a type specifier for all declarations}}
+long _Accum long_accum; // expected-error{{fixed point types are only allowed in C}}
Index:
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
ebevhan added a comment. With the exception of the two inline comments, this looks good to me now! Comment at: include/clang/Basic/LangOptions.def:301 +LANGOPT(FixedPointEnabled, 1, 0, "Fixed point types are enabled") + Just a minor nit... The 'Enabled' is superfluous. Comment at: include/clang/Driver/Options.td:882 +def enable_fixed_point : Flag<["-", "--"], "enable-fixed-point">, Group, + Flags<[CC1Option]>, HelpText<"Enable fixed point types">; Shouldn't this be an `-f` flag, like `-ffixed-point`? I'm not for or against either, just wondering what anyone else thinks. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added inline comments. Comment at: include/clang/Basic/TargetInfo.h:382 +// enough bits to fit the minumum. +if (getIntWidth() < MinSignedAccumDataBits) + return getLongWidth(); ebevhan wrote: > I'm not sure I agree with this interpretation. It's simply not correct to > consider 'short' the 'underlying type' of 'short _Accum', 'int' the > 'underlying type' of '_Accum', etc. They are wholly independent and should > have separate settings altogether. > > Asserting/ensuring that a target has set an invalid width for its types > should be done separately. This currently feels a bit like the TargetInfo is > guessing. > > (For the record, the reason I'm requesting this change is because this > implementation does not work for us downstream.) You're right. They should be different types. I was stuck in the mindset that `short _Accum` should be tied to `short`, `_Accum` be tied to `int`, etc. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148506.
leonardchan added a comment.
Re-added individual getters/members for _Accum types
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/Specifiers.h
include/clang/Basic/TargetInfo.h
include/clang/Basic/TokenKinds.def
include/clang/Driver/Options.td
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/Basic/TargetInfo.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_bit_widths.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_errors.cpp
test/Frontend/fixed_point_not_enabled.c
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned _Accum u_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+ // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -enable-fixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned _Accum u_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{fixed point types are only allowed in C}}
+// expected-error@-1{{C++ requires a type specifier for all declarations}}
+long _Accum long_accum; // expected-error{{fixed point types are only
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
ebevhan added inline comments. Comment at: include/clang/Basic/TargetInfo.h:382 +// enough bits to fit the minumum. +if (getIntWidth() < MinSignedAccumDataBits) + return getLongWidth(); I'm not sure I agree with this interpretation. It's simply not correct to consider 'short' the 'underlying type' of 'short _Accum', 'int' the 'underlying type' of '_Accum', etc. They are wholly independent and should have separate settings altogether. Asserting/ensuring that a target has set an invalid width for its types should be done separately. This currently feels a bit like the TargetInfo is guessing. (For the record, the reason I'm requesting this change is because this implementation does not work for us downstream.) Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added a comment. In https://reviews.llvm.org/D46084#374, @jfb wrote: > Can you also add a test for `_Bool _Accum`. > > Also, `-enable-fixed-point -x c++` failing. . Done. Also the failing c++ case is under `test/Frontend/fixed_point_errors.cpp` Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148481.
leonardchan marked 2 inline comments as done.
leonardchan added a comment.
- Added test case for `_Bool _Accum`
- Getters for the `_Accum` bit widths return values for their corresponding
integral types (ie. `sizeof(short _Accum) == sizeof(short)`). The only case
where this may not happen is if the target architecture uses 16 bits for an
int. N1169 requires that a `signed/unsigned _Accum` hold at least 15 fractional
bits and 4 integral bits. To be able to fit these bits, the size is upgraded to
that of a long which is guaranteed to be large enough to hold them.
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/Specifiers.h
include/clang/Basic/TargetInfo.h
include/clang/Basic/TokenKinds.def
include/clang/Driver/Options.td
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_bit_widths.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_errors.cpp
test/Frontend/fixed_point_not_enabled.c
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned _Accum u_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+ // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -enable-fixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned _Accum u_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; //
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
jfb added a comment. Can you also add a test for `_Bool _Accum`. Also, `-enable-fixed-point -x c++` failing. Comment at: lib/AST/ExprConstant.cpp:7361 +case BuiltinType::ULongAccum: + // GCC does not cover FIXED_POINT_TYPE in it's switch stmt and defaults to + // no_type_class "its" Comment at: lib/Basic/TargetInfo.cpp:45 + AccumWidth = AccumAlign = 32; + LongAccumWidth = LongAccumAlign = 64; SuitableAlign = 64; This seems weird because Targets which don't have these values for the non-Accum versions will have .e.g. `sizeof(short) != sizeof(short _Accum)`. Is there a point in ever having `_Accum` differ in size, width, and alignment from the underlying type? If not, can you set these values after the sub-target has specified its preferences? Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148452.
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/Specifiers.h
include/clang/Basic/TargetInfo.h
include/clang/Basic/TokenKinds.def
include/clang/Driver/Options.td
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/Basic/TargetInfo.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_errors.cpp
test/Frontend/fixed_point_not_enabled.c
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned _Accum u_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+ // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -enable-fixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned _Accum u_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{fixed point types are only allowed in C}}
+// expected-error@-1{{C++ requires a type specifier for all declarations}}
+long _Accum long_accum; // expected-error{{fixed point types are only allowed in C}}
Index: test/Frontend/fixed_point_errors.c
===
---
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan marked 6 inline comments as done.
leonardchan added inline comments.
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2684
// Types added here must also be added to EmitFundamentalRTTIDescriptors.
switch (Ty->getKind()) {
rsmith wrote:
> Note this comment :)
Returned false for these types now. Not sure if these types should also be
added to EmitFundamentalRTTIDescriptors since the OCL types do not appear under
there.
Repository:
rC Clang
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148445.
leonardchan added a comment.
- Reverted changes involving name mangling since we will only support c++ for
now. Will concurrently raise an issue on
https://github.com/itanium-cxx-abi/cxx-abi/ to get characters for name mangling.
- Added a flag that needs to be provided to enable usage of fixed point types.
Not including this flag and using fixed point types throws an error. Currently,
this patch allows for these types to be used in all versions of C, but this can
be narrowed down to specific versions of C.
- An error is thrown when using fixed point types in C++.
- Fixed point types are ignored during USRGeneration since the type only gets
mangled in C++.
- Fixed point types their own width and alignment accessors/variables in
TargetInfo.
- Updated debug info to use `DW_ATE_signed_fixed` and `DW_ATE_unsigned_fixed`.
- Added tests mixing _Accum with other type specifiers
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/Specifiers.h
include/clang/Basic/TargetInfo.h
include/clang/Basic/TokenKinds.def
include/clang/Driver/Options.td
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/Basic/TargetInfo.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_errors.cpp
test/Frontend/fixed_point_not_enabled.c
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned _Accum u_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
+ // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum; // expected-error{{compile with '-enable-fixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -enable-fixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;//
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
sammccall added inline comments.
Comment at: lib/Index/USRGeneration.cpp:691
+case BuiltinType::ULongAccum:
+ llvm_unreachable("No USR name mangling for fixed point types.");
case BuiltinType::Float16:
leonardchan wrote:
> rsmith wrote:
> > leonardchan wrote:
> > > phosek wrote:
> > > > We need some solution for fixed point types.
> > > Added character ~ to indicate fixed point type followed by string
> > > detailing the type. I have not added a test to it because logically, I do
> > > not think we will ever reach that point. This logic is implemented in the
> > > VisitType method, which mostly gets called by visitors to c++ nodes like
> > > VisitTemplateParameterList, but we have disabled the use of fixed point
> > > types in c++. VisitType does get called in VisitFunctionDecl but the
> > > function exits early since we are not reading c++ (line
> > > lib/Index/USRGeneration.cpp:238).
> > @rjmccall Is this an acceptable USR encoding? (Is our USR encoding scheme
> > documented anywhere?)
> I chatted with @sammccall about this who said it was ok to add these types if
> no one opposed this. I posted this on cfe-dev also and it seemed that no one
> spoke up about it, so I thought this was ok.
>
> I also couldn't find any standard or documentation about reserving characters
> for USR. It doesn't seem that USR is also parsed in any way, so I don't think
> I'm breaking anything (running ninja check-all passes).
>
> And unless we also do enable this for C++, this code actually may not be run
> since this method will not be visited if limited to C.
If the conclusion is to only allow these types for C, then you could punt on
this by setting IgnoreResult so USR generation fails. As you say USRs only
include type information in C++.
I think nothing parses them except humans, and using a prefix character is
probably better than eating 24 single characters. But one prefix character +
one trailing character might be less surprising.
Repository:
rC Clang
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
rjmccall added inline comments. Comment at: include/clang/Basic/DiagnosticCommonKinds.td:172 +def err_fixed_point_only_allowed_in_c : Error< + "Fixed point types are only allowed in C">; leonardchan wrote: > leonardchan wrote: > > leonardchan wrote: > > > rsmith wrote: > > > > Diagnostics should not be capitalized. Also, we generally allow > > > > conforming C extensions to be used in other languages unless there is a > > > > really good reason not to. > > > We decided not to allow fixed point types in other languages because > > > there is no specification provided in N1169 for addressing some features > > > in other languages. Using C++ as an example, N1169 does not provide > > > recommended characters when name mangling so we do not allow this in C++. > > Actually, scratch that. We will be enabling it since GCC does. Will update > > this and other relevant C++ related code appropriately. > Actually, the main thing that was preventing us from allowing this in C++ was > no standardized characters for name mangling. GCC seems to use the same > characters as some integral types (`short _Accum` uses `s`, `_Accum` uses > `i`, ...) but this would mean that a function that takes a `short _Accum` as > a sole argument would also be mangled the same as a similarly named function > that takes a `short`. > > Would copying GCC take priority over not having characters specific for these > types? This standard also proposes 24 different types, of which only 6 are > included in this patch. That makes it sound like GCC ignores `_Accum` and just mangles the unmodified type, which is clearly a bug that should not be imitated. You should raise this issue to the Itanium C++ ABI group and pick something unambiguous as a placeholder. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added a comment. After further discussion, we think the best approach for now would be only supporting fixed point types in C, then go back and support C++ once there is a standardized way for mangling the fixed point types under itanium. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added inline comments. Comment at: include/clang/Basic/DiagnosticCommonKinds.td:172 +def err_fixed_point_only_allowed_in_c : Error< + "Fixed point types are only allowed in C">; leonardchan wrote: > leonardchan wrote: > > rsmith wrote: > > > Diagnostics should not be capitalized. Also, we generally allow > > > conforming C extensions to be used in other languages unless there is a > > > really good reason not to. > > We decided not to allow fixed point types in other languages because there > > is no specification provided in N1169 for addressing some features in other > > languages. Using C++ as an example, N1169 does not provide recommended > > characters when name mangling so we do not allow this in C++. > Actually, scratch that. We will be enabling it since GCC does. Will update > this and other relevant C++ related code appropriately. Actually, the main thing that was preventing us from allowing this in C++ was no standardized characters for name mangling. GCC seems to use the same characters as some integral types (`short _Accum` uses `s`, `_Accum` uses `i`, ...) but this would mean that a function that takes a `short _Accum` as a sole argument would also be mangled the same as a similarly named function that takes a `short`. Would copying GCC take priority over not having characters specific for these types? This standard also proposes 24 different types, of which only 6 are included in this patch. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
jfb added a comment. > Actually, scratch that. We will be enabling it since GCC does. Will update > this and other relevant C++ related code appropriately. Could you also add tests which mix _Accum with volatile, _Atomic, _Complex, constexpr, inline? Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added inline comments. Comment at: include/clang/Basic/DiagnosticCommonKinds.td:172 +def err_fixed_point_only_allowed_in_c : Error< + "Fixed point types are only allowed in C">; leonardchan wrote: > rsmith wrote: > > Diagnostics should not be capitalized. Also, we generally allow conforming > > C extensions to be used in other languages unless there is a really good > > reason not to. > We decided not to allow fixed point types in other languages because there is > no specification provided in N1169 for addressing some features in other > languages. Using C++ as an example, N1169 does not provide recommended > characters when name mangling so we do not allow this in C++. Actually, scratch that. We will be enabling it since GCC does. Will update this and other relevant C++ related code appropriately. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan added a subscriber: sammccall.
leonardchan added inline comments.
Comment at: include/clang/Basic/DiagnosticCommonKinds.td:172
+def err_fixed_point_only_allowed_in_c : Error<
+ "Fixed point types are only allowed in C">;
rsmith wrote:
> Diagnostics should not be capitalized. Also, we generally allow conforming C
> extensions to be used in other languages unless there is a really good reason
> not to.
We decided not to allow fixed point types in other languages because there is
no specification provided in N1169 for addressing some features in other
languages. Using C++ as an example, N1169 does not provide recommended
characters when name mangling so we do not allow this in C++.
Comment at: lib/AST/ItaniumMangle.cpp:2552
+ case BuiltinType::ULongAccum:
+llvm_unreachable("Fixed point types are disabled for c++");
case BuiltinType::Half:
rsmith wrote:
> Please check what GCC uses to mangle these, and follow suit; if GCC doesn't
> have a mangling, you can use a vendor mangling (`u6_Accum`) or produce an
> error for now, but please open an issue at
> https://github.com/itanium-cxx-abi/cxx-abi/ to pick a real mangling.
It seems that GCC uses the characters for each fixed point type's corresponding
integral type (https://github.com/gcc-mirror/gcc/blob/master/gcc/cp/mangle.c).
Will follow up on this if we end up enabling fixed point types for C++.
Comment at: lib/CodeGen/CGDebugInfo.cpp:678-681
+ case BuiltinType::UShortAccum:
+ case BuiltinType::UAccum:
+ case BuiltinType::ULongAccum:
Encoding = llvm::dwarf::DW_ATE_unsigned;
rsmith wrote:
> @echristo @dblaikie Is this appropriate?
My bad, this should be changed to `DW_ATE_signed_fixed` and
`DW_ATE_unsigned_fixed`
Comment at: lib/Index/USRGeneration.cpp:691
+case BuiltinType::ULongAccum:
+ llvm_unreachable("No USR name mangling for fixed point types.");
case BuiltinType::Float16:
rsmith wrote:
> leonardchan wrote:
> > phosek wrote:
> > > We need some solution for fixed point types.
> > Added character ~ to indicate fixed point type followed by string detailing
> > the type. I have not added a test to it because logically, I do not think
> > we will ever reach that point. This logic is implemented in the VisitType
> > method, which mostly gets called by visitors to c++ nodes like
> > VisitTemplateParameterList, but we have disabled the use of fixed point
> > types in c++. VisitType does get called in VisitFunctionDecl but the
> > function exits early since we are not reading c++ (line
> > lib/Index/USRGeneration.cpp:238).
> @rjmccall Is this an acceptable USR encoding? (Is our USR encoding scheme
> documented anywhere?)
I chatted with @sammccall about this who said it was ok to add these types if
no one opposed this. I posted this on cfe-dev also and it seemed that no one
spoke up about it, so I thought this was ok.
I also couldn't find any standard or documentation about reserving characters
for USR. It doesn't seem that USR is also parsed in any way, so I don't think
I'm breaking anything (running ninja check-all passes).
And unless we also do enable this for C++, this code actually may not be run
since this method will not be visited if limited to C.
Repository:
rC Clang
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
ebevhan added inline comments.
Comment at: lib/AST/ASTContext.cpp:1775
+case BuiltinType::UShortAccum:
Width = Target->getShortWidth();
Align = Target->getShortAlign();
Please give the types their own width and alignment accessors/variables in
TargetInfo and use those instead of reusing the existing ones.
Comment at: lib/Sema/SemaType.cpp:1395
+ case DeclSpec::TST_accum: {
+if (S.getLangOpts().CPlusPlus) {
+ S.Diag(DS.getTypeSpecTypeLoc(), diag::err_fixed_point_only_allowed_in_c);
This (and the rest of the fixed-point support) should be behind its own option.
The error should reflect this as well.
Repository:
rC Clang
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
rjmccall added a comment. There are at least three good reasons to make sure this can enabled/disabled by a flag: - We have to anticipate that introducing new keywords will cause some compatibility problems. New language standards that introduce new keywords can be disabled using `-std=`. We shouldn't let this bypass that just because it's an out-of-band addition to the base language. - It seems likely to me that this feature will be target-restricted. - It seems plausible to me that development of this feature might continue past the Clang 7 branch date, and we shouldn't release a compiler with significantly-incomplete features on by default. That said, when it's ready, I think it should probably be on by default on the targets that support it. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
jfb added a comment. This is on by default for any version of C? AFAICK `_Accum` isn't on the C17 draft that I have, I'd expect to have to specify a command-line flag pertaining to TR 18037 to get this. At a minimum I'd be OK having it with the GNU variant of C, but not the `__ANSI_C__` one. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
rjmccall added a reviewer: akyrtzi. rjmccall added a comment. CC: Argyrios for the USR question. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
rsmith added subscribers: echristo, dblaikie, rjmccall.
rsmith added inline comments.
Comment at: include/clang/Basic/DiagnosticCommonKinds.td:172
+def err_fixed_point_only_allowed_in_c : Error<
+ "Fixed point types are only allowed in C">;
Diagnostics should not be capitalized. Also, we generally allow conforming C
extensions to be used in other languages unless there is a really good reason
not to.
Comment at: lib/AST/ASTContext.cpp:6261
+case BuiltinType::ULongAccum:
+ llvm_unreachable("No ObjC encoding for fixed point types");
+
`llvm_unreachable` should only be used for cases where you believe the
situation to be impossible. I would add these cases to the `return ' ';` case
above (for `Float16` / `Float128` / `Half`) for now.
Comment at: lib/AST/ExprConstant.cpp:7323-7325
+case BuiltinType::UShortAccum:
+case BuiltinType::UAccum:
+case BuiltinType::ULongAccum:
Have you checked what value GCC uses for these? Using `integer_type_class` for
a non-integer value seems very strange to me, but then they probably are
*passed* as integers, so maybe it's correct?
Comment at: lib/AST/ItaniumMangle.cpp:2552
+ case BuiltinType::ULongAccum:
+llvm_unreachable("Fixed point types are disabled for c++");
case BuiltinType::Half:
Please check what GCC uses to mangle these, and follow suit; if GCC doesn't
have a mangling, you can use a vendor mangling (`u6_Accum`) or produce an error
for now, but please open an issue at
https://github.com/itanium-cxx-abi/cxx-abi/ to pick a real mangling.
Comment at: lib/CodeGen/CGDebugInfo.cpp:678-681
+ case BuiltinType::UShortAccum:
+ case BuiltinType::UAccum:
+ case BuiltinType::ULongAccum:
Encoding = llvm::dwarf::DW_ATE_unsigned;
@echristo @dblaikie Is this appropriate?
Comment at: lib/CodeGen/CodeGenTypes.cpp:448
+case BuiltinType::ULongAccum:
ResultType = llvm::IntegerType::get(getLLVMContext(),
static_cast(Context.getTypeSize(T)));
jakehehrlich wrote:
> Add TODO for accum types using a different drawf type.
I think this TODO was added to the wrong file. Should it be in CGDebugInfo?
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2684
// Types added here must also be added to EmitFundamentalRTTIDescriptors.
switch (Ty->getKind()) {
Note this comment :)
Comment at: lib/Index/USRGeneration.cpp:691
+case BuiltinType::ULongAccum:
+ llvm_unreachable("No USR name mangling for fixed point types.");
case BuiltinType::Float16:
leonardchan wrote:
> phosek wrote:
> > We need some solution for fixed point types.
> Added character ~ to indicate fixed point type followed by string detailing
> the type. I have not added a test to it because logically, I do not think we
> will ever reach that point. This logic is implemented in the VisitType
> method, which mostly gets called by visitors to c++ nodes like
> VisitTemplateParameterList, but we have disabled the use of fixed point types
> in c++. VisitType does get called in VisitFunctionDecl but the function exits
> early since we are not reading c++ (line lib/Index/USRGeneration.cpp:238).
@rjmccall Is this an acceptable USR encoding? (Is our USR encoding scheme
documented anywhere?)
Comment at: lib/Sema/SemaType.cpp:1390-1394
+if (S.getLangOpts().CPlusPlus) {
+ S.Diag(DS.getTypeSpecTypeLoc(), diag::err_fixed_point_only_allowed_in_c);
+ Result = Context.ShortAccumTy;
+ break;
+}
As noted earlier, we should not have such a restriction.
Comment at: lib/Sema/SemaType.cpp:1424
+case DeclSpec::TSW_longlong:
+ // TODO: Replace with diag
+ llvm_unreachable("Unable to specify long long as _Accum width");
No need for a diagnostic as this is already caught elsewhere.
Repository:
rC Clang
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148148.
leonardchan added a comment.
pulled changes from source tree
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/Specifiers.h
include/clang/Basic/TokenKinds.def
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/accum.c
test/Frontend/accum_errors.c
test/Frontend/accum_errors.cpp
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/accum_errors.cpp
===
--- /dev/null
+++ test/Frontend/accum_errors.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -x c++ %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed long _Accum s_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned _Accum u_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{Fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{Fixed point types are only allowed in C}}
+long _Accum long_accum; // expected-error{{Fixed point types are only allowed in C}}
Index: test/Frontend/accum_errors.c
===
--- /dev/null
+++ test/Frontend/accum_errors.c
@@ -0,0 +1,4 @@
+// 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}}
Index: test/Frontend/accum.c
===
--- /dev/null
+++ test/Frontend/accum.c
@@ -0,0 +1,26 @@
+// RUN: %clang -cc1 -x c -ast-dump %s | FileCheck %s --strict-whitespace
+
+/* Various contexts where type _Accum can appear. */
+
+// Primary fixed point types
+signed short _Accum s_short_accum;
+signed _Accum s_accum;
+signed long _Accum s_long_accum;
+unsigned short _Accum u_short_accum;
+unsigned _Accum u_accum;
+unsigned long _Accum u_long_accum;
+
+// Aliased fixed point types
+short _Accum short_accum;
+_Accum accum;
+long _Accum long_accum;
+
+// CHECK: |-VarDecl {{.*}} s_short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_accum '_Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_long_accum 'long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_short_accum 'unsigned short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_accum 'unsigned _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_long_accum 'unsigned long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} accum '_Accum'
+// CHECK-NEXT: `-VarDecl {{.*}} long_accum 'long _Accum'
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6818,6 +6818,24 @@
case PREDEF_TYPE_LONGDOUBLE_ID:
T = Context.LongDoubleTy;
break;
+case PREDEF_TYPE_SHORT_ACCUM_ID:
+ T = Context.ShortAccumTy;
+ break;
+case PREDEF_TYPE_ACCUM_ID:
+ T = Context.AccumTy;
+ break;
+case PREDEF_TYPE_LONG_ACCUM_ID:
+ T =
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148121.
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/Specifiers.h
include/clang/Basic/TokenKinds.def
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/accum.c
test/Frontend/accum_errors.c
test/Frontend/accum_errors.cpp
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -542,6 +548,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/accum_errors.cpp
===
--- /dev/null
+++ test/Frontend/accum_errors.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -x c++ %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed long _Accum s_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned _Accum u_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{Fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{Fixed point types are only allowed in C}}
+long _Accum long_accum; // expected-error{{Fixed point types are only allowed in C}}
Index: test/Frontend/accum_errors.c
===
--- /dev/null
+++ test/Frontend/accum_errors.c
@@ -0,0 +1,4 @@
+// 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}}
Index: test/Frontend/accum.c
===
--- /dev/null
+++ test/Frontend/accum.c
@@ -0,0 +1,26 @@
+// RUN: %clang -cc1 -x c -ast-dump %s | FileCheck %s --strict-whitespace
+
+/* Various contexts where type _Accum can appear. */
+
+// Primary fixed point types
+signed short _Accum s_short_accum;
+signed _Accum s_accum;
+signed long _Accum s_long_accum;
+unsigned short _Accum u_short_accum;
+unsigned _Accum u_accum;
+unsigned long _Accum u_long_accum;
+
+// Aliased fixed point types
+short _Accum short_accum;
+_Accum accum;
+long _Accum long_accum;
+
+// CHECK: |-VarDecl {{.*}} s_short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_accum '_Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_long_accum 'long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_short_accum 'unsigned short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_accum 'unsigned _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_long_accum 'unsigned long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} accum '_Accum'
+// CHECK-NEXT: `-VarDecl {{.*}} long_accum 'long _Accum'
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6816,6 +6816,24 @@
case PREDEF_TYPE_LONGDOUBLE_ID:
T = Context.LongDoubleTy;
break;
+case PREDEF_TYPE_SHORT_ACCUM_ID:
+ T = Context.ShortAccumTy;
+ break;
+case PREDEF_TYPE_ACCUM_ID:
+ T = Context.AccumTy;
+ break;
+case PREDEF_TYPE_LONG_ACCUM_ID:
+ T = Context.LongAccumTy;
+ break;
+case PREDEF_TYPE_USHORT_ACCUM_ID:
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148117.
leonardchan marked an inline comment as done.
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/Specifiers.h
include/clang/Basic/TokenKinds.def
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/accum.c
test/Frontend/accum_errors.c
test/Frontend/accum_errors.cpp
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -542,6 +548,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/accum_errors.cpp
===
--- /dev/null
+++ test/Frontend/accum_errors.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -x c++ %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed long _Accum s_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned _Accum u_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{Fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{Fixed point types are only allowed in C}}
+long _Accum long_accum; // expected-error{{Fixed point types are only allowed in C}}
Index: test/Frontend/accum_errors.c
===
--- /dev/null
+++ test/Frontend/accum_errors.c
@@ -0,0 +1,4 @@
+// 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}}
Index: test/Frontend/accum.c
===
--- /dev/null
+++ test/Frontend/accum.c
@@ -0,0 +1,26 @@
+// RUN: %clang -cc1 -x c -ast-dump %s | FileCheck %s --strict-whitespace
+
+/* Various contexts where type _Accum can appear. */
+
+// Primary fixed point types
+signed short _Accum s_short_accum;
+signed _Accum s_accum;
+signed long _Accum s_long_accum;
+unsigned short _Accum u_short_accum;
+unsigned _Accum u_accum;
+unsigned long _Accum u_long_accum;
+
+// Aliased fixed point types
+short _Accum short_accum;
+_Accum accum;
+long _Accum long_accum;
+
+// CHECK: |-VarDecl {{.*}} s_short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_accum '_Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_long_accum 'long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_short_accum 'unsigned short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_accum 'unsigned _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_long_accum 'unsigned long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} accum '_Accum'
+// CHECK-NEXT: `-VarDecl {{.*}} long_accum 'long _Accum'
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6816,6 +6816,24 @@
case PREDEF_TYPE_LONGDOUBLE_ID:
T = Context.LongDoubleTy;
break;
+case PREDEF_TYPE_SHORT_ACCUM_ID:
+ T = Context.ShortAccumTy;
+ break;
+case PREDEF_TYPE_ACCUM_ID:
+ T = Context.AccumTy;
+ break;
+case PREDEF_TYPE_LONG_ACCUM_ID:
+ T = Context.LongAccumTy;
+
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan marked 2 inline comments as done.
leonardchan added inline comments.
Comment at: lib/Index/USRGeneration.cpp:731
+
+ if (c == '~') {
+switch (BT->getKind()) {
jakehehrlich wrote:
> You can make the 'c' a Twine instead. That will let you inline these in their
> respective locations as ` c = "~UA" ` for instance.
So Twine also isn't assignable. If I still want to keep the pattern of
assigning to a temporary variable, I could instead just make `c` a string.
Repository:
rC Clang
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148025.
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/Specifiers.h
include/clang/Basic/TokenKinds.def
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/accum.c
test/Frontend/accum_errors.c
test/Frontend/accum_errors.cpp
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -542,6 +548,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/accum_errors.cpp
===
--- /dev/null
+++ test/Frontend/accum_errors.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -x c++ %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed long _Accum s_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned _Accum u_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{Fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{Fixed point types are only allowed in C}}
+long _Accum long_accum; // expected-error{{Fixed point types are only allowed in C}}
Index: test/Frontend/accum_errors.c
===
--- /dev/null
+++ test/Frontend/accum_errors.c
@@ -0,0 +1,4 @@
+// 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}}
Index: test/Frontend/accum.c
===
--- /dev/null
+++ test/Frontend/accum.c
@@ -0,0 +1,26 @@
+// RUN: %clang -cc1 -x c -ast-dump %s | FileCheck %s --strict-whitespace
+
+/* Various contexts where type _Accum can appear. */
+
+// Primary fixed point types
+signed short _Accum s_short_accum;
+signed _Accum s_accum;
+signed long _Accum s_long_accum;
+unsigned short _Accum u_short_accum;
+unsigned _Accum u_accum;
+unsigned long _Accum u_long_accum;
+
+// Aliased fixed point types
+short _Accum short_accum;
+_Accum accum;
+long _Accum long_accum;
+
+// CHECK: |-VarDecl {{.*}} s_short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_accum '_Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_long_accum 'long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_short_accum 'unsigned short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_accum 'unsigned _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_long_accum 'unsigned long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} accum '_Accum'
+// CHECK-NEXT: `-VarDecl {{.*}} long_accum 'long _Accum'
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6816,6 +6816,24 @@
case PREDEF_TYPE_LONGDOUBLE_ID:
T = Context.LongDoubleTy;
break;
+case PREDEF_TYPE_SHORT_ACCUM_ID:
+ T = Context.ShortAccumTy;
+ break;
+case PREDEF_TYPE_ACCUM_ID:
+ T = Context.AccumTy;
+ break;
+case PREDEF_TYPE_LONG_ACCUM_ID:
+ T = Context.LongAccumTy;
+ break;
+case PREDEF_TYPE_USHORT_ACCUM_ID:
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
jakehehrlich added a comment.
LGTM with a couple additions.
Comment at: lib/CodeGen/CodeGenTypes.cpp:448
+case BuiltinType::ULongAccum:
ResultType = llvm::IntegerType::get(getLLVMContext(),
static_cast(Context.getTypeSize(T)));
Add TODO for accum types using a different drawf type.
Comment at: lib/Index/USRGeneration.cpp:731
+
+ if (c == '~') {
+switch (BT->getKind()) {
You can make the 'c' a Twine instead. That will let you inline these in their
respective locations as ` c = "~UA" ` for instance.
Repository:
rC Clang
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 147560.
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/Specifiers.h
include/clang/Basic/TokenKinds.def
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/accum.c
test/Frontend/accum_errors.c
test/Frontend/accum_errors.cpp
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -542,6 +548,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/accum_errors.cpp
===
--- /dev/null
+++ test/Frontend/accum_errors.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -x c++ %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed long _Accum s_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned _Accum u_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{Fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{Fixed point types are only allowed in C}}
+long _Accum long_accum; // expected-error{{Fixed point types are only allowed in C}}
Index: test/Frontend/accum_errors.c
===
--- /dev/null
+++ test/Frontend/accum_errors.c
@@ -0,0 +1,4 @@
+// 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}}
Index: test/Frontend/accum.c
===
--- /dev/null
+++ test/Frontend/accum.c
@@ -0,0 +1,26 @@
+// RUN: %clang -cc1 -x c -ast-dump %s | FileCheck %s --strict-whitespace
+
+/* Various contexts where type _Accum can appear. */
+
+// Primary fixed point types
+signed short _Accum s_short_accum;
+signed _Accum s_accum;
+signed long _Accum s_long_accum;
+unsigned short _Accum u_short_accum;
+unsigned _Accum u_accum;
+unsigned long _Accum u_long_accum;
+
+// Aliased fixed point types
+short _Accum short_accum;
+_Accum accum;
+long _Accum long_accum;
+
+// CHECK: |-VarDecl {{.*}} s_short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_accum '_Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_long_accum 'long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_short_accum 'unsigned short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_accum 'unsigned _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_long_accum 'unsigned long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} accum '_Accum'
+// CHECK-NEXT: `-VarDecl {{.*}} long_accum 'long _Accum'
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6816,6 +6816,24 @@
case PREDEF_TYPE_LONGDOUBLE_ID:
T = Context.LongDoubleTy;
break;
+case PREDEF_TYPE_SHORT_ACCUM_ID:
+ T = Context.ShortAccumTy;
+ break;
+case PREDEF_TYPE_ACCUM_ID:
+ T = Context.AccumTy;
+ break;
+case PREDEF_TYPE_LONG_ACCUM_ID:
+ T = Context.LongAccumTy;
+ break;
+case PREDEF_TYPE_USHORT_ACCUM_ID:
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 147549.
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/Specifiers.h
include/clang/Basic/TokenKinds.def
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/accum.c
test/Frontend/accum_errors.c
test/Frontend/accum_errors.cpp
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -542,6 +548,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/accum_errors.cpp
===
--- /dev/null
+++ test/Frontend/accum_errors.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -x c++ %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed long _Accum s_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned _Accum u_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{Fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{Fixed point types are only allowed in C}}
+long _Accum long_accum; // expected-error{{Fixed point types are only allowed in C}}
Index: test/Frontend/accum_errors.c
===
--- /dev/null
+++ test/Frontend/accum_errors.c
@@ -0,0 +1,4 @@
+// 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}}
Index: test/Frontend/accum.c
===
--- /dev/null
+++ test/Frontend/accum.c
@@ -0,0 +1,26 @@
+// RUN: %clang -cc1 -x c -ast-dump %s | FileCheck %s --strict-whitespace
+
+/* Various contexts where type _Accum can appear. */
+
+// Primary fixed point types
+signed short _Accum s_short_accum;
+signed _Accum s_accum;
+signed long _Accum s_long_accum;
+unsigned short _Accum u_short_accum;
+unsigned _Accum u_accum;
+unsigned long _Accum u_long_accum;
+
+// Aliased fixed point types
+short _Accum short_accum;
+_Accum accum;
+long _Accum long_accum;
+
+// CHECK: |-VarDecl {{.*}} s_short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_accum '_Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_long_accum 'long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_short_accum 'unsigned short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_accum 'unsigned _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_long_accum 'unsigned long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} accum '_Accum'
+// CHECK-NEXT: `-VarDecl {{.*}} long_accum 'long _Accum'
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6816,6 +6816,24 @@
case PREDEF_TYPE_LONGDOUBLE_ID:
T = Context.LongDoubleTy;
break;
+case PREDEF_TYPE_SHORT_ACCUM_ID:
+ T = Context.ShortAccumTy;
+ break;
+case PREDEF_TYPE_ACCUM_ID:
+ T = Context.AccumTy;
+ break;
+case PREDEF_TYPE_LONG_ACCUM_ID:
+ T = Context.LongAccumTy;
+ break;
+case PREDEF_TYPE_USHORT_ACCUM_ID:
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 147406.
leonardchan marked an inline comment as done.
leonardchan added a comment.
Undid git-clang-formatting on ASTBitcodes.h
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/Specifiers.h
include/clang/Basic/TokenKinds.def
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/accum.c
test/Frontend/accum_errors.c
test/Frontend/accum_errors.cpp
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -542,6 +548,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/accum_errors.cpp
===
--- /dev/null
+++ test/Frontend/accum_errors.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -x c++ %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed long _Accum s_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned _Accum u_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{Fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{Fixed point types are only allowed in C}}
+long _Accum long_accum; // expected-error{{Fixed point types are only allowed in C}}
Index: test/Frontend/accum_errors.c
===
--- /dev/null
+++ test/Frontend/accum_errors.c
@@ -0,0 +1,4 @@
+// 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}}
Index: test/Frontend/accum.c
===
--- /dev/null
+++ test/Frontend/accum.c
@@ -0,0 +1,26 @@
+// RUN: %clang -cc1 -x c -ast-dump %s | FileCheck %s --strict-whitespace
+
+/* Various contexts where type _Accum can appear. */
+
+// Primary fixed point types
+signed short _Accum s_short_accum;
+signed _Accum s_accum;
+signed long _Accum s_long_accum;
+unsigned short _Accum u_short_accum;
+unsigned _Accum u_accum;
+unsigned long _Accum u_long_accum;
+
+// Aliased fixed point types
+short _Accum short_accum;
+_Accum accum;
+long _Accum long_accum;
+
+// CHECK: |-VarDecl {{.*}} s_short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_accum '_Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_long_accum 'long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_short_accum 'unsigned short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_accum 'unsigned _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_long_accum 'unsigned long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} accum '_Accum'
+// CHECK-NEXT: `-VarDecl {{.*}} long_accum 'long _Accum'
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6816,6 +6816,24 @@
case PREDEF_TYPE_LONGDOUBLE_ID:
T = Context.LongDoubleTy;
break;
+case PREDEF_TYPE_SHORT_ACCUM_ID:
+ T = Context.ShortAccumTy;
+ break;
+case PREDEF_TYPE_ACCUM_ID:
+ T = Context.AccumTy;
+ break;
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan marked 2 inline comments as done.
leonardchan added inline comments.
Comment at: lib/Index/USRGeneration.cpp:691
+case BuiltinType::ULongAccum:
+ llvm_unreachable("No USR name mangling for fixed point types.");
case BuiltinType::Float16:
phosek wrote:
> We need some solution for fixed point types.
Added character ~ to indicate fixed point type followed by string detailing the
type. I have not added a test to it because logically, I do not think we will
ever reach that point. This logic is implemented in the VisitType method, which
mostly gets called by visitors to c++ nodes like VisitTemplateParameterList,
but we have disabled the use of fixed point types in c++. VisitType does get
called in VisitFunctionDecl but the function exits early since we are not
reading c++ (line lib/Index/USRGeneration.cpp:238).
Repository:
rC Clang
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 147400.
leonardchan added a comment.
Added break. We still assign `Result` since it cannot be null at the end of the
switch stmt, though the value doesn't matter.
Added character `~` to indicate fixed point type followed by string detailing
the type. I have not added a test to it because logically, I do not think we
will ever reach that point. This logic is implemented in the `VisitType`
method, which mostly gets called by visitors to c++ nodes like
`VisitTemplateParameterList`, but we have disabled the use of fixed point types
in c++. `VisitType` does get called in `VisitFunctionDecl` but the function
exits early since we are not reading c++ (line lib/Index/USRGeneration.cpp:238).
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/Specifiers.h
include/clang/Basic/TokenKinds.def
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/accum.c
test/Frontend/accum_errors.c
test/Frontend/accum_errors.cpp
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -542,6 +548,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/accum_errors.cpp
===
--- /dev/null
+++ test/Frontend/accum_errors.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -x c++ %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed long _Accum s_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned _Accum u_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{Fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{Fixed point types are only allowed in C}}
+long _Accum long_accum; // expected-error{{Fixed point types are only allowed in C}}
Index: test/Frontend/accum_errors.c
===
--- /dev/null
+++ test/Frontend/accum_errors.c
@@ -0,0 +1,4 @@
+// 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}}
Index: test/Frontend/accum.c
===
--- /dev/null
+++ test/Frontend/accum.c
@@ -0,0 +1,26 @@
+// RUN: %clang -cc1 -x c -ast-dump %s | FileCheck %s --strict-whitespace
+
+/* Various contexts where type _Accum can appear. */
+
+// Primary fixed point types
+signed short _Accum s_short_accum;
+signed _Accum s_accum;
+signed long _Accum s_long_accum;
+unsigned short _Accum u_short_accum;
+unsigned _Accum u_accum;
+unsigned long _Accum u_long_accum;
+
+// Aliased fixed point types
+short _Accum short_accum;
+_Accum accum;
+long _Accum long_accum;
+
+// CHECK: |-VarDecl {{.*}} s_short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_accum '_Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_long_accum 'long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_short_accum 'unsigned short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_accum 'unsigned _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_long_accum 'unsigned long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}}
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 147386.
leonardchan added a comment.
Ran git-clang-tidy on all affected files
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/Specifiers.h
include/clang/Basic/TokenKinds.def
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/accum.c
test/Frontend/accum_errors.c
test/Frontend/accum_errors.cpp
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -542,6 +548,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/accum_errors.cpp
===
--- /dev/null
+++ test/Frontend/accum_errors.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -x c++ %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{Fixed point types are only allowed in C}}
+signed long _Accum s_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned _Accum u_accum; // expected-error{{Fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{Fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{Fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{Fixed point types are only allowed in C}}
+long _Accum long_accum; // expected-error{{Fixed point types are only allowed in C}}
Index: test/Frontend/accum_errors.c
===
--- /dev/null
+++ test/Frontend/accum_errors.c
@@ -0,0 +1,4 @@
+// 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}}
Index: test/Frontend/accum.c
===
--- /dev/null
+++ test/Frontend/accum.c
@@ -0,0 +1,26 @@
+// RUN: %clang -cc1 -x c -ast-dump %s | FileCheck %s --strict-whitespace
+
+/* Various contexts where type _Accum can appear. */
+
+// Primary fixed point types
+signed short _Accum s_short_accum;
+signed _Accum s_accum;
+signed long _Accum s_long_accum;
+unsigned short _Accum u_short_accum;
+unsigned _Accum u_accum;
+unsigned long _Accum u_long_accum;
+
+// Aliased fixed point types
+short _Accum short_accum;
+_Accum accum;
+long _Accum long_accum;
+
+// CHECK: |-VarDecl {{.*}} s_short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_accum '_Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} s_long_accum 'long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_short_accum 'unsigned short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_accum 'unsigned _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} u_long_accum 'unsigned long _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} short_accum 'short _Accum'
+// CHECK-NEXT: |-VarDecl {{.*}} accum '_Accum'
+// CHECK-NEXT: `-VarDecl {{.*}} long_accum 'long _Accum'
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6816,6 +6816,24 @@
case PREDEF_TYPE_LONGDOUBLE_ID:
T = Context.LongDoubleTy;
break;
+case PREDEF_TYPE_SHORT_ACCUM_ID:
+ T = Context.ShortAccumTy;
+ break;
+case PREDEF_TYPE_ACCUM_ID:
+ T = Context.AccumTy;
+ break;
+case PREDEF_TYPE_LONG_ACCUM_ID:
+ T =
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
phosek added inline comments.
Comment at: lib/Index/USRGeneration.cpp:691
+case BuiltinType::ULongAccum:
+ llvm_unreachable("No USR name mangling for fixed point types.");
case BuiltinType::Float16:
We need some solution for fixed point types.
Comment at: lib/Sema/SemaType.cpp:1391
+if (S.getLangOpts().CPlusPlus) {
+ S.Diag(DS.getTypeSpecTypeLoc(), diag::err_fixed_point_only_allowed_in_c);
+}
Shouldn't there be a break after this line?
https://reviews.llvm.org/D46084
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
