RE: [ping][patch] Fixed-point patch 4/10

["Fu, Chao-Ying" <fu at mips dot com>]

Joseph S. Myers wrote:
> 
> On Wed, 1 Aug 2007, Fu, Chao-Ying wrote:
> 
> > 	* c-common.h (enum rid): Add new enumeration values of RID_SAT,
> > 	RID_FRACT, and RID_ACCUM.
> 
> Why do you have RID_SAT in a different part of the enumeration from 
> RID_FRACT and RIC_ACCUM, instead of all together?

  In "declspecs_add_type", the detection of type specifier is as follows.
It checks i <= RID_LAST_MODIFIER.
------ (c-decl.c)
  /* Handle type specifier keywords.  */
  if (TREE_CODE (type) == IDENTIFIER_NODE && C_IS_RESERVED_WORD (type))
    {
      enum rid i = C_RID_CODE (type);
      if (specs->type)
        {
          error ("two or more data types in declaration specifiers");
          return specs;
        }
      if ((int) i <= (int) RID_LAST_MODIFIER)
        {
          /* "long", "short", "signed", "unsigned", "_Complex" or "_Sat".  */
          bool dupe = false;
          switch (i)
------

  Then, "RID_LAST_MODIFIER" is the same as "RID_ONEWAY", so I need to
insert "RID_SAT" to a location before "RID_ONEWAY" as follows.
------- (c-common.h)
  /* C extensions */
  RID_COMPLEX, RID_THREAD, RID_SAT,

  /* C++ */
  RID_FRIEND, RID_VIRTUAL, RID_EXPLICIT, RID_EXPORT, RID_MUTABLE,

  /* ObjC */
  RID_IN, RID_OUT, RID_INOUT, RID_BYCOPY, RID_BYREF, RID_ONEWAY,
...
  RID_MAX,

  RID_FIRST_MODIFIER = RID_STATIC,
  RID_LAST_MODIFIER = RID_ONEWAY,
-------

> 
> > 	* c-tree.h (enum c_typespec_keyword): Add cts_fract and 
> cts_accum.
> 
> You correctly add the new keywords to the comment, but it was 
> already out 
> of date; please add the other missing ones _Decimal32, _Decimal64, 
> _Decimal128 to the comment at the same time.

  Yes.  I will add the missing ones.

> 
> > 	* c-decl.c (build_null_declspecs): Initialize saturating_p.
> > 	(declspecs_add_type): Avoid using complex with _Fract or _Accum.
> > 	Handle RID_SAT.
> > 	Avoid using void, bool, char, int, float, double, _Decimal32,
> > 	_Decimal64, _Decimal128 with _Sat.
> > 	Handle RID_FRACT and RID_ACCUM.
> > 	Make sure _Sat is used with _Fract or _Accum.
> 
> _Sat _Complex doesn't appear in any valid types, so I think 
> it would be 
> worth detecting it whenever the second of those keywords is 
> added rather 
> than at a later stage.

  Yes.  I will add code to detect _Sat _Complex and _Complex _Sat.

> 
> > 	(c_common_fixed_point_type_for_size): New.
> 
> Errors shouldn't end with a trailing ".",
  
  Yes.

> 
> +      error ("cannot find a common fixed-point mode to operate.");
> 
> and in any case this error seems uninformative to the user - 
> it's written 
> in compiler-internals terms, not in terms of what's wrong 
> with the user's 
> code.  If it's an internal error and should never occur for valid or 
> invalid user code, make it an internal_error or a gcc_assert; 
> otherwise, 
> make it explain what's wrong with the user's code (or make 
> the caller give 
> such a more meaningful error).

  This error comes from valid user code that tries to operate on
mixed data types of integers and fractional types.
Ex: 
long long a; /* 63 integral bits + a sign bit. */
long long _Fract b; /* 63 fractional bits + a sign bit.  */
b = a * b; // For this operation (a * b), GCC tries to find an accum mode
that can support at least 63 integral bits and 63 fractional bits.
But, if the target doesn't have this kind of accum mode, GCC fails.

  Maybe I will use this error message.
---
      error ("should not use integer types and fixed-point types that have too\
\nmany integral and fractional bits together");
---
Is it ok?

> 
> > 	* c-typeck.c (c_common_type): Check FIXED_POINT_TYPE.  Build
> > 	a common fixed-point type based on fbit, ibit, sign, 
> and saturation.
> 
> Again, you have errors that are not meaningful to the user.
> 
> +               error ("machine mode is wrong");
> 
> +               error ("machine mode is wrong");
> 
> Make them meaningful if they can arise from erroneous user 
> code, make them 
> internal errors or assertions if they can't.

  I will change to gcc_assert (0), because these cases should not happen.

> 
> > 	* target-def.h (TARGET_FIXED_POINT_SUPPORTED_P): New.
> > 	(TARGET_INITIALIZER): Add TARGET_FIXED_POINT_SUPPORTED_P.
> > 	* target.h (gcc_target): Add fixed_point_supported_p.
> > 	* targhooks.c (default_scalar_mode_supported_p): Handle 
> MODE_FRACT,
> > 	MODE_UFRACT, MODE_ACCUM, and MODE_UACCUM.
> > 	(default_fixed_point_supported_p): Define.
> > 	* targhooks.h (default_fixed_point_supported_p): Declare.
> > 	* doc/tm.texi (TARGET_FIXED_POINT_SUPPORTED_P): Add.
> > 	* doc/install.texi (Configuration): Add --enable-fixed-point.
> > 	* configure.ac (--enable-fixed-point): New to enable fixed-point
> > 	arithmetic extension to C.
> > 	* configure, config.in: Regenerate.
> 
> I can't review these parts though I don't see anything wrong 
> with them, 
> except: I think your AC_ARG_ENABLE call is more verbose than 
> necessary for 
> what it currently does; the variable enable_fixed_point will be 
> automatically set so you don't need
> 
> +  if test x$enableval = xyes ; then
> +    enable_fixed_point=yes
> +  else
> +    enable_fixed_point=no
> +  fi
> 
> (and appropriate quoting in the subsequent fixedpoint=... 
> will avoid the 
> need to convert strange arguments to the configure option into "no").
> 
> However, the configure test actually needs a whitelist of supported 
> targets, like --enable-decimal-float has.  

  Ok.  I will add mips*-*-* as supported targets for now.
---
# Enable C extension for fixed-point arithmetic.
AC_ARG_ENABLE(fixed-point,
[  --enable-fixed-point    enable fixed-point arithmetic extension to C],
[
],
[
  case $target in
    mips*-*-*)
      enable_fixed_point=yes
      ;;
    *)
      AC_MSG_WARN(fixed-point is not supported for this target, ignored)
      enable_fixed_point=no
      ;;
  esac
])
AC_SUBST(enable_fixed_point)
---

> This should contain only 
> targets for which there is publically available documentation of the 
> necessary ABI extensions for layout and argument passing and function 
> return involving the new types.  (If there is no ABI authority for a 
> target to define the ABI in a compiler-independent way, you 
> can always 
> write your own GCC ABI and put it in a file in projects/ on 
> the website, 
> but it should still be documented before allowing the extension to be 
> enabled for a target.)

  I think the ABI for the fixed-point types is similar to the ABI for
integer types for MIPS.  I will talk to Nigel about the ABI file later.

> 
> gcc/doc/extend.texi needs updating to describe the support for the 
> fixed-point extension, if this isn't in another of the 
> patches in this 
> series.

  The change of extend.texi is in the patch 7/10 (changes.diff).
---
Index: gcc4x/gcc/gcc/doc/extend.texi
===================================================================
--- gcc4x.orig/gcc/gcc/doc/extend.texi  2007-08-13 10:26:05.000000000 -0700
+++ gcc4x/gcc/gcc/doc/extend.texi       2007-08-13 16:33:30.000000000 -0700
@@ -36,6 +36,7 @@ extensions, accepted by GCC in C89 mode
 * Floating Types::      Additional Floating Types.
 * Decimal Float::       Decimal Floating Types.
 * Hex Floats::          Hexadecimal floating-point constants.
+* Fixed-Point::         Fixed-Point Types.
 * Zero Length::         Zero-length arrays.
 * Variable Length::     Arrays whose length is computed at run time.
 * Empty Structures::    Structures with no members.
@@ -923,6 +924,134 @@ would not be able to resolve the ambigui
 could mean @code{1.0f} or @code{1.9375} since @samp{f} is also the
 extension for floating-point constants of type @code{float}.

+@node Fixed-Point
+@section Fixed-Point Types
+@cindex fixed-point types
+@cindex @code{_Fract} data type
+@cindex @code{_Accum} data type
+@cindex @code{_Sat} data type
+@cindex @code{hr} fixed-suffix
+@cindex @code{r} fixed-suffix
+@cindex @code{lr} fixed-suffix
+@cindex @code{llr} fixed-suffix
+@cindex @code{uhr} fixed-suffix
+@cindex @code{ur} fixed-suffix
+@cindex @code{ulr} fixed-suffix
+@cindex @code{ullr} fixed-suffix
+@cindex @code{hk} fixed-suffix
+@cindex @code{k} fixed-suffix
+@cindex @code{lk} fixed-suffix
+@cindex @code{llk} fixed-suffix
+@cindex @code{uhk} fixed-suffix
+@cindex @code{uk} fixed-suffix
+@cindex @code{ulk} fixed-suffix
+@cindex @code{ullk} fixed-suffix
+@cindex @code{HR} fixed-suffix
+@cindex @code{R} fixed-suffix
+@cindex @code{LR} fixed-suffix
+@cindex @code{LLR} fixed-suffix
+@cindex @code{UHR} fixed-suffix
+@cindex @code{UR} fixed-suffix
+@cindex @code{ULR} fixed-suffix
+@cindex @code{ULLR} fixed-suffix
+@cindex @code{HK} fixed-suffix
+@cindex @code{K} fixed-suffix
+@cindex @code{LK} fixed-suffix
+@cindex @code{LLK} fixed-suffix
+@cindex @code{UHK} fixed-suffix
+@cindex @code{UK} fixed-suffix
+@cindex @code{ULK} fixed-suffix
+@cindex @code{ULLK} fixed-suffix
+
+As an extension, the GNU C compiler supports fixed-point types as
+defined in the N1169 draft of ISO/IEC DTR 18037.  Support for fixed-point
+types in GCC will evolve as the draft technical report changes.
+Calling conventions for any target might also change.  Not all targets
+support fixed-point types.
+
+The fixed-point types are
+@code{short _Fract},
+@code{_Fract},
+@code{long _Fract},
+@code{long long _Fract},
+@code{unsigned short _Fract},
+@code{unsigned _Fract},
+@code{unsigned long _Fract},
+@code{unsigned long long _Fract},
+@code{_Sat short _Fract},
+@code{_Sat _Fract},
+@code{_Sat long _Fract},
+@code{_Sat long long _Fract},
+@code{_Sat unsigned short _Fract},
+@code{_Sat unsigned _Fract},
+@code{_Sat unsigned long _Fract},
+@code{_Sat unsigned long long _Fract},
+@code{short _Accum},
+@code{_Accum},
+@code{long _Accum},
+@code{long long _Accum},
+@code{unsigned short _Accum},
+@code{unsigned _Accum},
+@code{unsigned long _Accum},
+@code{unsigned long long _Accum},
+@code{_Sat short _Accum},
+@code{_Sat _Accum},
+@code{_Sat long _Accum},
+@code{_Sat long long _Accum},
+@code{_Sat unsigned short _Accum},
+@code{_Sat unsigned _Accum},
+@code{_Sat unsigned long _Accum},
+@code{_Sat unsigned long long _Accum}.
+Fixed-point data values contain fractional and optional integral parts.
+The format of fixed-point data varies and depends on the target machine.
+
+Support for fixed-point types includes prefix and postfix increment
+and decrement operators (@code{++}, @code{--}); unary arithmetic operators
+(@code{+}, @code{-}, @code{!}); binary arithmetic operators (@code{+},
+@code{-}, @code{*}, @code{/}); binary shift operators (@code{<<}, @code{>>});
+relational operators (@code{<}, @code{<=}, @code{>=}, @code{>});
+equality operators (@code{==}, @code{!=}); assignment operators
+(@code{+=}, @code{-=}, @code{*=}, @code{/=}, @code{<<=}, @code{>>=});
+and conversions to and from integer, floating-point, or fixed-point types.
+
+Use a suffix @samp{hr} or @samp{HR} in a literal constant of type
+@code{short _Fract} and @code{_Sat short _Fract},
+@samp{r} or @samp{R} for @code{_Fract} and @code{_Sat _Fract},
+@samp{lr} or @samp{LR} for @code{long _Fract} and @code{_Sat long _Fract},
+@samp{llr} or @samp{LLR} for @code{long long _Fract} and
+@code{_Sat long long _Fract},
+@samp{uhr} or @samp{UHR} for @code{unsigned short _Fract} and
+@code{_Sat unsigned short _Fract},
+@samp{ur} or @samp{UR} for @code{unsigned _Fract} and
+@code{_Sat unsigned _Fract},
+@samp{ulr} or @samp{ULR} for @code{unsigned long _Fract} and
+@code{_Sat unsigned long _Fract},
+@samp{ullr} or @samp{ULLR} for @code{unsigned long long _Fract}
+@code{_Sat unsigned long _Fract},
+@samp{ullr} or @samp{ULLR} for @code{unsigned long long _Fract}
+and @code{_Sat unsigned long long _Fract},
+@samp{hk} or @samp{HK} for @code{short _Accum} and @code{_Sat short _Accum},
+@samp{k} or @samp{K} for @code{_Accum} and @code{_Sat _Accum},
+@samp{lk} or @samp{LK} for @code{long _Accum} and @code{_Sat long _Accum},
+@samp{llk} or @samp{LLK} for @code{long long _Accum} and
+@code{_Sat long long _Accum},
+@samp{uhk} or @samp{UHK} for @code{unsigned short _Accum} and
+@code{_Sat unsigned short _Accum},
+@samp{uk} or @samp{UK} for @code{unsigned _Accum} and
+@code{_Sat unsigned _Accum},
+@samp{ulk} or @samp{ULK} for @code{unsigned long _Accum} and
+@code{_Sat unsigned long _Accum},
+and @samp{ullk} or @samp{ULLK} for @code{unsigned long long _Accum}
+and @code{_Sat unsigned long long _Accum}.
+
+GCC support of fixed-point types as specified by the draft technical report
+is incomplete:
+
+@itemize @bullet
+@item
+Pragmas to control overflow and rounding behaviors are not implemented.
+@end itemize
+
+Fixed-point types are supported by the DWARF2 debug information format.
+
 @node Zero Length
 @section Arrays of Length Zero
 @cindex arrays of length zero
---

  The new patch is attached.  Thanks!

Regards,
Chao-ying

Attachment: c-parser.diff
Description: c-parser.diff