Richard Biener <richard.guent...@gmail.com> writes:
> On Sat, Dec 7, 2013 at 11:28 AM, Richard Sandiford
> <rdsandif...@googlemail.com> wrote:
>> Kenneth Zadeck <zad...@naturalbridge.com> writes:
>>>> +    /* One could argue that GET_MODE_PRECISION (TYPE_MODE (type))
>>>> +       should always be the same as TYPE_PRECISION (type).
>>>> +       However, it is not.  Since we are converting from tree to
>>>> +       rtl, we have to expose this ugly truth here.  */
>>>> +    temp = immed_wide_int_const (wide_int::from
>>>> +                                   (exp,
>>>> +                                    GET_MODE_PRECISION (TYPE_MODE (type)),
>>>> +                                    TYPE_SIGN (type)),
>>>> +                                 TYPE_MODE (type));
>>>> +    return temp;
>>>> +      }
>>>>
>>>> I don't really see how one could argue that, given that there are much 
>>>> fewer
>>>> modes than possible type precisions, so please rewrite the comment, e.g.:
>>>>
>>>> "Given that TYPE_PRECISION (type) is not always equal to
>>>> GET_MODE_PRECISION (TYPE_MODE (type)), we need to extend from the former
>>>> to the latter according to the signedness of the type".
>>>>
>>>> What about a fast track where the precisions are indeed equal?
>>>>
>>>
>>> There is not really a faster track here.    you still are starting with
>>> a tree and converting to an rtx.   All that the default one would do
>>> would be to access the types precision and sign and use that.
>>
>> FWIW it would be:
>>
>>         temp = immed_wide_int_const (exp, TYPE_MODE (type));
>>
>> But it's hard to tell whether it would buy much.  It didn't show up as
>> a hot spot when I was doing performance measurements before.
>>
>>>> --- a/gcc/machmode.def
>>>> +++ b/gcc/machmode.def
>>>> @@ -229,6 +229,9 @@ UACCUM_MODE (USA, 4, 16, 16); /* 16.16 */
>>>>   UACCUM_MODE (UDA, 8, 32, 32); /* 32.32 */
>>>>   UACCUM_MODE (UTA, 16, 64, 64); /* 64.64 */
>>>>
>>>> +/* Should be overridden by EXTRA_MODES_FILE if wrong.  */
>>>> +#define MAX_BITS_PER_UNIT 8
>>>> +
>>>>
>>>> What is it for?  It's not documented at all.
>>>>
>>> This requires some discussion as to the direction we want to go. This is
>>> put in so that in gen_modes we can compute MAX_BITSIZE_MODE_ANY_INT and
>>> MAX_BITSIZE_MODE_ANY_MODE.    The problem is that during genmodes we do
>>> have access to BITS_PER_UNIT.    These two computed symbols are then
>>> used as compile time constants in other parts of the compiler to
>>> allocate data structures that are guaranteed to be large enough.
>>>
>>> Richard Sandiford put this in so we would preserve the ability to build
>>> a multi-targetted compiler where the targets had different values for
>>> BITS_PER_UNIT.   So one possibility is that we add some documentation to
>>> this effect.
>>
>> Sorry, I forgot yesterday an important detail behind why this seemed
>> like a good thing.  I think there was a strong feeling (from me and others)
>> that wide-int.h shouldn't depend on tm.h.  If we make wide-int.h depend
>> on tm.h then basically all the compiler does.
>>
>> So as it stands we can't use BITS_PER_UNIT directly.  Having a
>> MAX_BITS_PER_UNIT for "all compiled-in targets" (which obviously
>> as things stand is exactly one) seemed like a reasonable abstraction.
>>
>> Alternatively we could say that BITS_PER_UNIT is really part of the
>> definition of QImode and move it to the modes.def file.
>
> That makes sense to me - thus it will end up in insn-modes.h?

Yeah.

> Note that this file already uses BITS_PER_UNIT ...

Right, but only in the definitions that Kenny added in preparation for
wide-int:

2013-03-28  Kenneth Zadeck  <zad...@naturalbridge.com>

        * genmodes.c (emit_max_int): New function.
        (emit_insn_modes_h): Added call to emit_max_function.
        * doc/rtl.texi (MAX_BITSIZE_MODE_ANY_INT, MAX_BITSIZE_MODE_ANY_MODE):
        Added doc.
        * machmode.def: Fixed comment.

And it's those that we were changing to use MAX_BITS_PER_UNITS instead,
so that there were no direct uses of BITS_PER_UNIT in insn-modes.h.

In the original version wide-int.h had to include tm.h in order for
these BITS_PER_UNIT-based macros to be usable.

Thanks,
Richard

Reply via email to