| Message ID | alpine.LNX.2.00.1105311335430.810@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
> hmm, yes. Again practically for most targets size_t will be > following its SIZE_TYPE advice, but surely not for all. OTOH while > the above clearly doesn't look "accidential", it certainly looks > wrong. If not for sizetype then at least for size_type_node. The > comment hints that the patch at most will no longer "get better > code", but if Pmode gets better code when used for sizetype(!) then > we should do so unconditionally and could get rid of the size_t > reverse-engineering in initialize_sizetypes completely (m32c might > disagree here). On m32c, Pmode is a 24-bit type, and the chip just doesn't have enough math opcodes to to 24-bit pointer math with any degree of efficiency. So, you either do 32-bit math (performance is horrible, since it's all emulated) or 16-bit math on just the offset (sizeof size_t < Pmode).
On Tue, May 31, 2011 at 8:24 PM, DJ Delorie <dj@redhat.com> wrote: > >> hmm, yes. Again practically for most targets size_t will be >> following its SIZE_TYPE advice, but surely not for all. OTOH while >> the above clearly doesn't look "accidential", it certainly looks >> wrong. If not for sizetype then at least for size_type_node. The >> comment hints that the patch at most will no longer "get better >> code", but if Pmode gets better code when used for sizetype(!) then >> we should do so unconditionally and could get rid of the size_t >> reverse-engineering in initialize_sizetypes completely (m32c might >> disagree here). > > On m32c, Pmode is a 24-bit type, and the chip just doesn't have enough > math opcodes to to 24-bit pointer math with any degree of efficiency. > So, you either do 32-bit math (performance is horrible, since it's all > emulated) or 16-bit math on just the offset (sizeof size_t < Pmode). Which means that Ada must be seriously broken on m32c (well, I guess nobody tried it there ;)). Richard.
> Which means that Ada must be seriously broken on m32c (well, I guess > nobody tried it there ;)). I usually only build C and C++.
On Tue, 31 May 2011, Richard Guenther wrote: > > This initializes sizetypes correctly from the start, using target > definitions available. All Frontends initialize sizetypes from > size_type_node for which there is a target macro SIZE_TYPE which > tells what type to use for this (C runtime ABI) type. > > Now, there are two frontends who do not honor SIZE_TYPE but > have an idea on its own. That's Java (probably by accident) and > Ada (of course). Java does > > /* This is not a java type, however tree-dfa requires a definition for > size_type_node. */ > size_type_node = make_unsigned_type (POINTER_SIZE); > set_sizetype (size_type_node); > > so the FE itself doesn't care and POINTER_SIZE for almost all targets > yields the same result as following the SIZE_TYPE advice. Ada has > its own idea and thinks it can choose size_t freely, > > /* In Ada, we use the unsigned type corresponding to the width of Pmode > as > SIZETYPE. In most cases when ptr_mode and Pmode differ, C will use > the > width of ptr_mode for SIZETYPE, but we get better code using the > width > of Pmode. Note that, although we manipulate negative offsets for > some > internal constructs and rely on compile time overflow detection in > size > computations, using unsigned types for SIZETYPEs is fine since they > are > treated specially by the middle-end, in particular sign-extended. */ > size_type_node = gnat_type_for_mode (Pmode, 1); > set_sizetype (size_type_node); > TYPE_NAME (sizetype) = get_identifier ("size_type"); > > hmm, yes. Again practically for most targets size_t will be following > its SIZE_TYPE advice, but surely not for all. OTOH while the above > clearly doesn't look "accidential", it certainly looks wrong. If > not for sizetype then at least for size_type_node. The comment hints > that the patch at most will no longer "get better code", but if > Pmode gets better code when used for sizetype(!) then we should do > so unconditionally and could get rid of the size_t reverse-engineering > in initialize_sizetypes completely (m32c might disagree here). > > Not yet bootstrapped or tested (but I don't expect any issues other > than eventual typos on the targets I have access to). > > Now, any objections? (Patch to be adjusted to really remove > all set_sizetype calls) And this one, ontop of the previously posted patch to defer things to the middle-end, passed bootstrap and regtest for all languages on x86_64-unknown-linux-gnu. Richard. 2011-05-31 Richard Guenther <rguenther@suse.de> * stor-layout.c (initialize_sizetypes): Initialize all sizetypes based on target definitions. (set_sizetype): Remove. Index: gcc/stor-layout.c =================================================================== *** gcc/stor-layout.c.orig 2011-06-01 15:41:56.000000000 +0200 --- gcc/stor-layout.c 2011-06-01 16:14:03.000000000 +0200 *************** make_accum_type (int precision, int unsi *** 2189,2216 **** return type; } ! /* Initialize sizetype and bitsizetype to a reasonable and temporary ! value to enable integer types to be created. */ void initialize_sizetypes (void) { ! tree t = make_node (INTEGER_TYPE); ! int precision = GET_MODE_BITSIZE (SImode); ! SET_TYPE_MODE (t, SImode); ! TYPE_ALIGN (t) = GET_MODE_ALIGNMENT (SImode); ! TYPE_IS_SIZETYPE (t) = 1; ! TYPE_UNSIGNED (t) = 1; ! TYPE_SIZE (t) = build_int_cst (t, precision); ! TYPE_SIZE_UNIT (t) = build_int_cst (t, GET_MODE_SIZE (SImode)); ! TYPE_PRECISION (t) = precision; ! set_min_and_max_values_for_integral_type (t, precision, /*is_unsigned=*/true); ! sizetype = t; ! bitsizetype = build_distinct_type_copy (t); } /* Make sizetype a version of TYPE, and initialize *sizetype accordingly. --- 2189,2258 ---- return type; } ! /* Initialize sizetypes so layout_type can use them. */ void initialize_sizetypes (void) { ! int precision, bprecision; ! /* Get sizetypes precision from the SIZE_TYPE target macro. */ ! if (strcmp (SIZE_TYPE, "unsigned int") == 0) ! precision = INT_TYPE_SIZE; ! else if (strcmp (SIZE_TYPE, "long unsigned int") == 0) ! precision = LONG_TYPE_SIZE; ! else if (strcmp (SIZE_TYPE, "long long unsigned int") == 0) ! precision = LONG_LONG_TYPE_SIZE; ! else ! gcc_unreachable (); ! ! bprecision ! = MIN (precision + BITS_PER_UNIT_LOG + 1, MAX_FIXED_MODE_SIZE); ! bprecision ! = GET_MODE_PRECISION (smallest_mode_for_size (bprecision, MODE_INT)); ! if (bprecision > HOST_BITS_PER_WIDE_INT * 2) ! bprecision = HOST_BITS_PER_WIDE_INT * 2; ! ! /* Create stubs for sizetype and bitsizetype so we can create constants. */ ! sizetype = make_node (INTEGER_TYPE); ! /* ??? We can't set a name for sizetype because it appears in C diagnostics ! and pp_c_type_specifier doesn't deal with IDENTIFIER_NODE TYPE_NAMEs. */ ! TYPE_PRECISION (sizetype) = precision; ! TYPE_UNSIGNED (sizetype) = 1; ! TYPE_IS_SIZETYPE (sizetype) = 1; ! bitsizetype = make_node (INTEGER_TYPE); ! TYPE_NAME (bitsizetype) = get_identifier ("bitsizetype"); ! TYPE_PRECISION (bitsizetype) = bprecision; ! TYPE_UNSIGNED (bitsizetype) = 1; ! TYPE_IS_SIZETYPE (bitsizetype) = 1; ! ! /* Now layout both types manually. */ ! SET_TYPE_MODE (sizetype, smallest_mode_for_size (precision, MODE_INT)); ! TYPE_ALIGN (sizetype) = GET_MODE_ALIGNMENT (TYPE_MODE (sizetype)); ! TYPE_SIZE (sizetype) = bitsize_int (precision); ! TYPE_SIZE_UNIT (sizetype) = size_int (GET_MODE_SIZE (TYPE_MODE (sizetype))); ! set_min_and_max_values_for_integral_type (sizetype, precision, ! /*is_unsigned=*/true); ! /* sizetype is unsigned but we need to fix TYPE_MAX_VALUE so that it is ! sign-extended in a way consistent with force_fit_type. */ ! TYPE_MAX_VALUE (sizetype) ! = double_int_to_tree (sizetype, ! tree_to_double_int (TYPE_MAX_VALUE (sizetype))); ! SET_TYPE_MODE (bitsizetype, smallest_mode_for_size (bprecision, MODE_INT)); ! TYPE_ALIGN (bitsizetype) = GET_MODE_ALIGNMENT (TYPE_MODE (bitsizetype)); ! TYPE_SIZE (bitsizetype) = bitsize_int (bprecision); ! TYPE_SIZE_UNIT (bitsizetype) ! = size_int (GET_MODE_SIZE (TYPE_MODE (bitsizetype))); ! set_min_and_max_values_for_integral_type (bitsizetype, bprecision, /*is_unsigned=*/true); + /* ??? TYPE_MAX_VALUE is not properly sign-extended. */ ! /* Create the signed variants of *sizetype. */ ! ssizetype = make_signed_type (TYPE_PRECISION (sizetype)); ! TYPE_IS_SIZETYPE (ssizetype) = 1; ! sbitsizetype = make_signed_type (TYPE_PRECISION (bitsizetype)); ! TYPE_IS_SIZETYPE (sbitsizetype) = 1; } /* Make sizetype a version of TYPE, and initialize *sizetype accordingly. *************** initialize_sizetypes (void) *** 2222,2282 **** void set_sizetype (tree type) { ! tree t, max; ! int oprecision = TYPE_PRECISION (type); ! /* The *bitsizetype types use a precision that avoids overflows when ! calculating signed sizes / offsets in bits. However, when ! cross-compiling from a 32 bit to a 64 bit host, we are limited to 64 bit ! precision. */ ! int precision ! = MIN (oprecision + BITS_PER_UNIT_LOG + 1, MAX_FIXED_MODE_SIZE); ! precision ! = GET_MODE_PRECISION (smallest_mode_for_size (precision, MODE_INT)); ! if (precision > HOST_BITS_PER_WIDE_INT * 2) ! precision = HOST_BITS_PER_WIDE_INT * 2; ! ! /* sizetype must be an unsigned type. */ ! gcc_assert (TYPE_UNSIGNED (type)); ! ! t = build_distinct_type_copy (type); ! /* We want to use sizetype's cache, as we will be replacing that type. */ ! TYPE_CACHED_VALUES (t) = TYPE_CACHED_VALUES (sizetype); ! TYPE_CACHED_VALUES_P (t) = TYPE_CACHED_VALUES_P (sizetype); ! TYPE_UID (t) = TYPE_UID (sizetype); ! TYPE_IS_SIZETYPE (t) = 1; ! ! /* Replace our original stub sizetype. */ ! memcpy (sizetype, t, tree_size (sizetype)); ! TYPE_MAIN_VARIANT (sizetype) = sizetype; ! TYPE_CANONICAL (sizetype) = sizetype; ! ! /* sizetype is unsigned but we need to fix TYPE_MAX_VALUE so that it is ! sign-extended in a way consistent with force_fit_type. */ ! max = TYPE_MAX_VALUE (sizetype); ! TYPE_MAX_VALUE (sizetype) ! = double_int_to_tree (sizetype, tree_to_double_int (max)); ! ! t = make_node (INTEGER_TYPE); ! TYPE_NAME (t) = get_identifier ("bit_size_type"); ! /* We want to use bitsizetype's cache, as we will be replacing that type. */ ! TYPE_CACHED_VALUES (t) = TYPE_CACHED_VALUES (bitsizetype); ! TYPE_CACHED_VALUES_P (t) = TYPE_CACHED_VALUES_P (bitsizetype); ! TYPE_PRECISION (t) = precision; ! TYPE_UID (t) = TYPE_UID (bitsizetype); ! TYPE_IS_SIZETYPE (t) = 1; ! ! /* Replace our original stub bitsizetype. */ ! memcpy (bitsizetype, t, tree_size (bitsizetype)); ! TYPE_MAIN_VARIANT (bitsizetype) = bitsizetype; ! TYPE_CANONICAL (bitsizetype) = bitsizetype; ! ! fixup_unsigned_type (bitsizetype); ! ! /* Create the signed variants of *sizetype. */ ! ssizetype = make_signed_type (oprecision); ! TYPE_IS_SIZETYPE (ssizetype) = 1; ! sbitsizetype = make_signed_type (precision); ! TYPE_IS_SIZETYPE (sbitsizetype) = 1; } /* TYPE is an integral type, i.e., an INTEGRAL_TYPE, ENUMERAL_TYPE --- 2264,2270 ---- void set_sizetype (tree type) { ! gcc_assert (TYPE_PRECISION (sizetype) == TYPE_PRECISION (type)); } /* TYPE is an integral type, i.e., an INTEGRAL_TYPE, ENUMERAL_TYPE
> This initializes sizetypes correctly from the start, using target > definitions available. All Frontends initialize sizetypes from > size_type_node for which there is a target macro SIZE_TYPE which > tells what type to use for this (C runtime ABI) type. And this is a prerequisite if you want to do LTO in the language; otherwise, LTO doesn't work at all, for example for Ada on the 4.5 branch. > so the FE itself doesn't care and POINTER_SIZE for almost all targets > yields the same result as following the SIZE_TYPE advice. Ada has > its own idea and thinks it can choose size_t freely, Yes, like for boolean_type_node, you can set size_type_node to whatever you want as long as you don't do LTO. At least it must be unsigned now. > hmm, yes. Again practically for most targets size_t will be following > its SIZE_TYPE advice, but surely not for all. OTOH while the above > clearly doesn't look "accidential", it certainly looks wrong. If > not for sizetype then at least for size_type_node. The comment hints > that the patch at most will no longer "get better code", but if > Pmode gets better code when used for sizetype(!) then we should do > so unconditionally and could get rid of the size_t reverse-engineering > in initialize_sizetypes completely (m32c might disagree here). The thing is, I don't think you can have different types for size_type_node and the *sizetype series. So, while for the C family of language, you are forced to use SIZE_TYPE for both because of size_t, you still need to have the same type for the other languages. > Now, any objections? (Patch to be adjusted to really remove > all set_sizetype calls) Fine with me at least. When I was changing the signedness of sizetype in Ada, I hesitated to remove the Pmode vs ptr_mode kludge and eventually erred on the side of conservatism. But it clearly needs to go and now seems a good time.
Index: gcc/stor-layout.c =================================================================== --- gcc/stor-layout.c (revision 174469) +++ gcc/stor-layout.c (working copy) @@ -2189,95 +2189,83 @@ make_accum_type (int precision, int unsi return type; } -/* Initialize sizetype and bitsizetype to a reasonable and temporary - value to enable integer types to be created. */ +/* Initialize sizetypes so layout_type can use them. */ void initialize_sizetypes (void) { - tree t = make_node (INTEGER_TYPE); - int precision = GET_MODE_BITSIZE (SImode); + tree t; + int precision; - SET_TYPE_MODE (t, SImode); - TYPE_ALIGN (t) = GET_MODE_ALIGNMENT (SImode); - TYPE_IS_SIZETYPE (t) = 1; + if (strcmp (SIZE_TYPE, "unsigned int") == 0) + precision = INT_TYPE_SIZE; + else if (strcmp (SIZE_TYPE, "long unsigned int") == 0) + precision = LONG_TYPE_SIZE; + else if (strcmp (SIZE_TYPE, "long long unsigned int") == 0) + precision = LONG_LONG_TYPE_SIZE; + else + gcc_unreachable (); + + t = make_node (INTEGER_TYPE); + TYPE_NAME (t) = get_identifier ("sizetype"); + TYPE_PRECISION (t) = precision; TYPE_UNSIGNED (t) = 1; + TYPE_IS_SIZETYPE (t) = 1; + /* Layout sizetype manually. */ + SET_TYPE_MODE (t, smallest_mode_for_size (TYPE_PRECISION (t), MODE_INT)); + TYPE_ALIGN (t) = GET_MODE_ALIGNMENT (TYPE_MODE (t)); TYPE_SIZE (t) = build_int_cst (t, precision); - TYPE_SIZE_UNIT (t) = build_int_cst (t, GET_MODE_SIZE (SImode)); - TYPE_PRECISION (t) = precision; - + TYPE_SIZE_UNIT (t) = build_int_cst (t, GET_MODE_SIZE (TYPE_MODE (t))); set_min_and_max_values_for_integral_type (t, precision, /*is_unsigned=*/true); - + /* sizetype is unsigned but we need to fix TYPE_MAX_VALUE so that it is + sign-extended in a way consistent with force_fit_type. */ + TYPE_MAX_VALUE (t) + = double_int_to_tree (t, tree_to_double_int (TYPE_MAX_VALUE (t))); sizetype = t; - bitsizetype = build_distinct_type_copy (t); -} -/* Make sizetype a version of TYPE, and initialize *sizetype accordingly. - We do this by overwriting the stub sizetype and bitsizetype nodes created - by initialize_sizetypes. This makes sure that (a) anything stubby about - them no longer exists and (b) any INTEGER_CSTs created with such a type, - remain valid. */ -void -set_sizetype (tree type) -{ - tree t, max; - int oprecision = TYPE_PRECISION (type); - /* The *bitsizetype types use a precision that avoids overflows when - calculating signed sizes / offsets in bits. However, when - cross-compiling from a 32 bit to a 64 bit host, we are limited to 64 bit - precision. */ - int precision - = MIN (oprecision + BITS_PER_UNIT_LOG + 1, MAX_FIXED_MODE_SIZE); + precision + = MIN (precision + BITS_PER_UNIT_LOG + 1, MAX_FIXED_MODE_SIZE); precision = GET_MODE_PRECISION (smallest_mode_for_size (precision, MODE_INT)); if (precision > HOST_BITS_PER_WIDE_INT * 2) precision = HOST_BITS_PER_WIDE_INT * 2; - /* sizetype must be an unsigned type. */ - gcc_assert (TYPE_UNSIGNED (type)); - - t = build_distinct_type_copy (type); - /* We want to use sizetype's cache, as we will be replacing that type. */ - TYPE_CACHED_VALUES (t) = TYPE_CACHED_VALUES (sizetype); - TYPE_CACHED_VALUES_P (t) = TYPE_CACHED_VALUES_P (sizetype); - TYPE_UID (t) = TYPE_UID (sizetype); - TYPE_IS_SIZETYPE (t) = 1; - - /* Replace our original stub sizetype. */ - memcpy (sizetype, t, tree_size (sizetype)); - TYPE_MAIN_VARIANT (sizetype) = sizetype; - TYPE_CANONICAL (sizetype) = sizetype; - - /* sizetype is unsigned but we need to fix TYPE_MAX_VALUE so that it is - sign-extended in a way consistent with force_fit_type. */ - max = TYPE_MAX_VALUE (sizetype); - TYPE_MAX_VALUE (sizetype) - = double_int_to_tree (sizetype, tree_to_double_int (max)); - t = make_node (INTEGER_TYPE); - TYPE_NAME (t) = get_identifier ("bit_size_type"); - /* We want to use bitsizetype's cache, as we will be replacing that type. */ - TYPE_CACHED_VALUES (t) = TYPE_CACHED_VALUES (bitsizetype); - TYPE_CACHED_VALUES_P (t) = TYPE_CACHED_VALUES_P (bitsizetype); + TYPE_NAME (t) = get_identifier ("bitsizetype"); TYPE_PRECISION (t) = precision; - TYPE_UID (t) = TYPE_UID (bitsizetype); + TYPE_UNSIGNED (t) = 1; TYPE_IS_SIZETYPE (t) = 1; + /* Layout bitsizetype manually. */ + SET_TYPE_MODE (t, smallest_mode_for_size (TYPE_PRECISION (t), MODE_INT)); + TYPE_ALIGN (t) = GET_MODE_ALIGNMENT (TYPE_MODE (t)); + TYPE_SIZE (t) = build_int_cst (t, precision); + TYPE_SIZE_UNIT (t) = build_int_cst (t, GET_MODE_SIZE (TYPE_MODE (t))); + set_min_and_max_values_for_integral_type (t, precision, + /*is_unsigned=*/true); + /* ??? TYPE_MAX_VALUE is not properly sign-extended. */ + bitsizetype = t; - /* Replace our original stub bitsizetype. */ - memcpy (bitsizetype, t, tree_size (bitsizetype)); - TYPE_MAIN_VARIANT (bitsizetype) = bitsizetype; - TYPE_CANONICAL (bitsizetype) = bitsizetype; - - fixup_unsigned_type (bitsizetype); /* Create the signed variants of *sizetype. */ - ssizetype = make_signed_type (oprecision); + ssizetype = make_signed_type (TYPE_PRECISION (sizetype)); TYPE_IS_SIZETYPE (ssizetype) = 1; - sbitsizetype = make_signed_type (precision); + sbitsizetype = make_signed_type (TYPE_PRECISION (bitsizetype)); TYPE_IS_SIZETYPE (sbitsizetype) = 1; } + +/* Make sizetype a version of TYPE, and initialize *sizetype accordingly. + We do this by overwriting the stub sizetype and bitsizetype nodes created + by initialize_sizetypes. This makes sure that (a) anything stubby about + them no longer exists and (b) any INTEGER_CSTs created with such a type, + remain valid. */ + +void +set_sizetype (tree type) +{ + gcc_assert (TYPE_PRECISION (sizetype) == TYPE_PRECISION (type)); +} /* TYPE is an integral type, i.e., an INTEGRAL_TYPE, ENUMERAL_TYPE or BOOLEAN_TYPE. Set TYPE_MIN_VALUE and TYPE_MAX_VALUE
This initializes sizetypes correctly from the start, using target definitions available. All Frontends initialize sizetypes from size_type_node for which there is a target macro SIZE_TYPE which tells what type to use for this (C runtime ABI) type. Now, there are two frontends who do not honor SIZE_TYPE but have an idea on its own. That's Java (probably by accident) and Ada (of course). Java does /* This is not a java type, however tree-dfa requires a definition for size_type_node. */ size_type_node = make_unsigned_type (POINTER_SIZE); set_sizetype (size_type_node); so the FE itself doesn't care and POINTER_SIZE for almost all targets yields the same result as following the SIZE_TYPE advice. Ada has its own idea and thinks it can choose size_t freely, /* In Ada, we use the unsigned type corresponding to the width of Pmode as SIZETYPE. In most cases when ptr_mode and Pmode differ, C will use the width of ptr_mode for SIZETYPE, but we get better code using the width of Pmode. Note that, although we manipulate negative offsets for some internal constructs and rely on compile time overflow detection in size computations, using unsigned types for SIZETYPEs is fine since they are treated specially by the middle-end, in particular sign-extended. */ size_type_node = gnat_type_for_mode (Pmode, 1); set_sizetype (size_type_node); TYPE_NAME (sizetype) = get_identifier ("size_type"); hmm, yes. Again practically for most targets size_t will be following its SIZE_TYPE advice, but surely not for all. OTOH while the above clearly doesn't look "accidential", it certainly looks wrong. If not for sizetype then at least for size_type_node. The comment hints that the patch at most will no longer "get better code", but if Pmode gets better code when used for sizetype(!) then we should do so unconditionally and could get rid of the size_t reverse-engineering in initialize_sizetypes completely (m32c might disagree here). Not yet bootstrapped or tested (but I don't expect any issues other than eventual typos on the targets I have access to). Now, any objections? (Patch to be adjusted to really remove all set_sizetype calls) Thanks, Richard. 2011-05-31 Richard Guenther <rguenther@suse.de> * stor-layout.c (initialize_sizetypes): Initialize all sizetypes based on target definitions. (set_sizetype): Remove.