This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch]: Add support of new __int128 type for targets having 128-bit integer scalar support
On Fri, 16 Apr 2010, Kai Tietz wrote:
> + if (bits == TYPE_PRECISION (int128_integer_type_node))
> + return (unsignedp ? int128_unsigned_type_node
> + : int128_integer_type_node);
> +
Should be conditional on __int128 actually being supported. That is,
HOST_BITS_PER_WIDE_INT >= 64 and TImode being supported.
> + if (mode == TYPE_MODE (int128_integer_type_node))
> + return unsignedp ? int128_unsigned_type_node : int128_integer_type_node;
Likewise.
> @@ -2983,6 +2994,8 @@ c_common_type_for_mode (enum machine_mod
>
> if (mode == TYPE_MODE (complex_integer_type_node) && !unsignedp)
> return complex_integer_type_node;
> + if (mode == TYPE_MODE (complex_int128_type_node) && !unsignedp)
> + return complex_int128_type_node;
Why is this needed? Won't the following code using GET_MODE_INNER
suffice?
> + if (width == TYPE_PRECISION (int128_integer_type_node))
> + return (unsignedp ? int128_unsigned_type_node
> + : int128_integer_type_node);
Again, conditional on __int128 being supported.
> + if (targetm.scalar_mode_supported_p (TYPE_MODE (int128_integer_type_node)))
This is not the right test for it being supported ... as-is, you'll try to
create a 128-bit type, then if HOST_BITS_PER_WIDE_INT is 32
fixup_signed_type will reduce it to 64-bit and a mode will be chosen
accordingly. I think what you want is not to create these nodes at all
(so leave them as NULL) unless the existing condition for TImode support
passes.
> lang_hooks.decls.pushdecl (build_decl (UNKNOWN_LOCATION,
> TYPE_DECL,
> + get_identifier ("complex __int128"),
> + complex_int128_type_node));
Again, appropriately conditional.
> + if (targetm.scalar_mode_supported_p (TYPE_MODE (int128_integer_type_node)))
> + builtin_define_type_sizeof ("__SIZEOF_INT128__",
> + int128_integer_type_node);
Likewise.
> static const char *
> type_suffix (tree type)
> {
> - static const char *const suffixes[] = { "", "U", "L", "UL", "LL", "ULL" };
> + static const char *const suffixes[] = { "", "U", "L", "UL", "LL", "ULL", "I128", "UI128" };
> int unsigned_suffix;
> int is_long;
>
> - if (type == long_long_integer_type_node
> + if (type == int128_integer_type_node || type == int128_unsigned_type_node)
> + is_long = 3;
> + else if (type == long_long_integer_type_node
Since an int128 suffix is not in fact being defined, don't change this
function at all. That way, it will abort if it can't produce valid output
for a standard macro on the given platform, which is the correct thing for
it to do.
> @@ -8551,6 +8551,13 @@ declspecs_add_type (location_t loc, stru
> switch (i)
> {
> case RID_LONG:
> + if (specs->typespec_word == cts_int128)
> + {
> + error_at (loc,
> + ("both %<long%> and %<__int128%> in "
> + "declaration specifiers"));
> + break;
> + }
> if (specs->long_long_p)
Should go along with the checks for void etc. rather than before that for
long long long.
> @@ -8607,7 +8614,13 @@ declspecs_add_type (location_t loc, stru
> break;
> case RID_SHORT:
> dupe = specs->short_p;
> - if (specs->long_p)
> + if (specs->typespec_word == cts_int128)
> + {
> + error_at (loc,
> + ("both %<short%> and %<__int128%> in "
> + "declaration specifiers"));
> + }
> + else if (specs->long_p)
Likewise, should be grouped with checks for void etc..
> + if (!targetm.scalar_mode_supported_p (TYPE_MODE (int128_integer_type_node)))
> + {
> + error_at (loc, "%<__int128%> is not supported for this target");
> + return specs;
Correct check for being supported.
> @@ -8972,7 +9016,7 @@ declspecs_add_type (location_t loc, stru
> ("both %<long long%> and %<%s%> in "
> "declaration specifiers"),
> str);
> - if (specs->long_p)
> + else if (specs->long_p)
> error_at (loc,
> ("both %<long%> and %<%s%> in "
> "declaration specifiers"),
Gratuitous change.
> @@ -9024,7 +9068,7 @@ declspecs_add_type (location_t loc, stru
> str = "_Fract";
> else
> str = "_Accum";
> - if (specs->complex_p)
> + if (specs->complex_p)
> error_at (loc,
> ("both %<complex%> and %<%s%> in "
> "declaration specifiers"),
Gratuitous change. If you want to fix indentation to use TABs properly,
fix this whole function as a separate commit rather than mixing in changes
to lines that don't otherwise need changing as part of this patch.
> +#ifndef INT128_TYPE_SIZE
> +#define INT128_TYPE_SIZE 128
> +#endif
As this type is defined to have a fixed precision, there is no need for
this macro. It's reasonable for consistency to define such a macro, but
it should be unconditional; don't leave or document the option for a
target to override it.
> +As extension the integer scalar type @code{__int128} is supported for
"As an extension"
> +targets with have an integer mode wide enough to hold 128-bit.
"with have" is not grammatical English.
This fails to explain that though the type has some superficial similarity
to an integer type, it is not generally an extended integer type within
the meaning of C99 (as target ABIs generally define intmax_t to be 64
bits).
> + if (type1 == int128_integer_type_node || type1 == int128_unsigned_type_node)
> + return unsignedp
> + ? int128_unsigned_type_node
> + : int128_integer_type_node;
> #if HOST_BITS_PER_WIDE_INT >= 64
> if (type1 == intTI_type_node || type1 == unsigned_intTI_type_node)
> return unsignedp ? unsigned_intTI_type_node : intTI_type_node;
> @@ -4098,6 +4102,10 @@ gimple_signed_or_unsigned_type (bool uns
> return (unsignedp
> ? long_long_unsigned_type_node
> : long_long_integer_type_node);
> + if (TYPE_OK (int128_integer_type_node))
> + return (unsignedp
> + ? int128_unsigned_type_node
> + : int128_integer_type_node);
Appropriate conditions.
--
Joseph S. Myers
joseph@codesourcery.com