This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Re: locating unsigned type for non-standard precision
On Wed, Apr 25, 2012 at 7:34 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
> Georg-Johann Lay wrote:
>> Richard Guenther wrote:
>>> On Tue, Apr 24, 2012 at 7:24 PM, Georg-Johann Lay <avr@gjlay.de> wrote:
>>>> Richard Guenther wrote:
>>>>
>>>>>> I've run into another issue supporting a 20-bit integer for which I'd
>>>>>> appreciate a hint. ?With this code:
>>>>>>
>>>>>> ? typedef long int __attribute__((__a20__)) int20_t;
>>>>>> ? int20_t xi;
>>>>>> ? int20_t addit () { xi += 0x54321L; }
>>>>>>
>>>>>> xi ends up in mode PSImode, which is a MODE_PARTIAL_INT with 20 bits of
>>>>>> precision and 32-bit width.
>>>>>>
>>>>>> convert() notices that, because the constant in the add expression is
>>>>>> SImode, there's an SImode add being truncated to a PSImode result, and
>>>>>> pushes the truncation down into the operands.
>>>>>>
>>>>>> The problem is this ends up in convert_to_integer, which detects that the
>>>>>> signed operation might overflow so calls unsigned_type_for() to get the
>>>>>> unsigned variant.
>>>>>>
>>>>>> Unfortunately, this ends up in c_common_type_for_size(), which knows nothing
>>>>>> about PSImode, and returns an unsigned type with 32 bits of precision when
>>>>>> asked for one with 20 bits of precision.
>>>>> That's expected - this function returns a type that is suitable for holding all
>>>>> values, not a type that has necessarily matching precision. ?If the caller
>>>>> wants such type it needs to verify what the function returned. ?Which seems
>>>>> to me to be the correct fix for this (premature) optimization in
>>>>> convert_to_integer.
>>>>>
>>>>> Richard.
>>>>>
>>>>>> ? The expression is rewritten with
>>>>>> the 32-bit constant integer recast to the 32-bit unsigned type (instead
>>>>>> of the 20-bit one it might have used), and infinite recursion results.
>>>> This is already filed as
>>>>
>>>> http://gcc.gnu.org/PR51527
>>>>
>>>> It works with 4.8 trunk but crashes with 4.7.
>>>> Did not yet track what changes made it work with 4.8, though.
>>>> Unfortunately, noone remembers :-(
>>>>
>>>> http://gcc.gnu.org/ml/gcc/2012-03/msg00440.html
>>> I have done changes in this area, trying to remove the type_for_size langhook.
>>>
>>> Richard.
>>
>> One apparent change is tree.c:signed_or_unsigned_type_for
>>
>> that changed from 4.7:
>>
>> tree
>> signed_or_unsigned_type_for (int unsignedp, tree type)
>> {
>> ? tree t = type;
>> ? if (POINTER_TYPE_P (type))
>> ? ? {
>> ? ? ? /* If the pointer points to the normal address space, use the
>> ? ? ? ?size_type_node. ?Otherwise use an appropriate size for the pointer
>> ? ? ? ?based on the named address space it points to. ?*/
>> ? ? ? if (!TYPE_ADDR_SPACE (TREE_TYPE (t)))
>> ? ? ? t = size_type_node;
>> ? ? ? else
>> ? ? ? return lang_hooks.types.type_for_size (TYPE_PRECISION (t), unsignedp);
>> ? ? }
>>
>> ? if (!INTEGRAL_TYPE_P (t) || TYPE_UNSIGNED (t) == unsignedp)
>> ? ? return t;
>>
>> ? return lang_hooks.types.type_for_size (TYPE_PRECISION (t), unsignedp);
>> }
>>
>> to 4.8:
>>
>> tree
>> signed_or_unsigned_type_for (int unsignedp, tree type)
>> {
>> ? if (TREE_CODE (type) == INTEGER_TYPE && TYPE_UNSIGNED (type) == unsignedp)
>> ? ? return type;
>>
>> ? if (!INTEGRAL_TYPE_P (type)
>> ? ? ? && !POINTER_TYPE_P (type))
>> ? ? return NULL_TREE;
>>
>> ? return build_nonstandard_integer_type (TYPE_PRECISION (type), unsignedp);
>> }
>>
>> at 2012-03-12
>>
>> http://gcc.gnu.org/viewcvs?view=revision&revision=185226
>>
>> Is this appropriate to backport?
No, it's not.
>> Or is the preferred solution to override lang_hooks.types.type_for_size in the
>> backend, if applicable?
Neither. It is a "lang"-hook, not a target-hook after all.
I already told you what the right fix is - the callers of
type_for_size have to cater
for the returned type to be of different precision. Btw, I already see it does
/* But now perhaps TYPEX is as wide as INPREC.
In that case, do nothing special here.
(Otherwise would recurse infinitely in convert. */
if (TYPE_PRECISION (typex) != inprec)
Richard.
> Now I have this piece of code that works for 4.7.
>
> I don't like it much, but if there are no plans to otherwise fix PR51527,
> I'd make a patch and propose it as target-specific work-around.
>
> /* Some auxiliary stuff for the AVR-specific built-in 24-bit scalar integer
> ? types __int24 and __uint24 to be used in `avr_init_builtins'. ?*/
>
> static GTY(()) tree avr_int24_type;
> static GTY(()) tree avr_uint24_type;
>
>
> /* Implement `LANG_HOOKS_TYPE_FOR_SIZE'. ?*/
> /* To avoid infinite recursion in `convert_to_integer' as
> ? reported in PR c/51527. ?*/
>
> static tree (*avr_default_type_for_size)(unsigned int, int);
>
> static tree
> avr_type_for_size (unsigned int bits, int unsignedp)
> {
> ?if (bits == 24)
> ? ?return unsignedp ? avr_uint24_type : avr_int24_type;
>
> ?return avr_default_type_for_size (bits, unsignedp);
> }
>
>
> static void
> avr_init_builtin_int24 (void)
> {
> ?avr_int24_type ?= make_signed_type (GET_MODE_BITSIZE (PSImode));
> ?avr_uint24_type = make_unsigned_type (GET_MODE_BITSIZE (PSImode));
>
> ?lang_hooks.types.register_builtin_type (avr_int24_type, "__int24");
> ?lang_hooks.types.register_builtin_type (avr_uint24_type, "__uint24");
>
> ?/* Hook-in to avoid infinite recursion in `convert_to_integer' as
> ? ? reported in PR c/51527. ?*/
>
> ?avr_default_type_for_size = lang_hooks.types.type_for_size;
> ?lang_hooks.types.type_for_size = avr_type_for_size;
> }
>
>
> A proper, target-independent fix would be great because it is the right place
> and because Peter has the same problem.
>
> Johann
>
>>>>>> Is it proper for c_common_type_for_size() to know about partial int modes
>>>>>> and return the best one available? ?Is there a hook that would allow me to
>>>>>> do this customized in my back-end? ?Or is there another way to get
>>>>>> unsigned_type_for() to return the "right" type for an unusual integer
>>>>>> precision?
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>> Peter