Richard Biener <richard.guent...@gmail.com> writes:
> On Sat, Nov 30, 2013 at 10:43 AM, Richard Sandiford
> <rdsandif...@googlemail.com> wrote:
>> So maybe two INTEGER_CST lengths weren't enough. Because bitsizetype
>> can be offset_int-sized, wi::to_offset had a TYPE_PRECISION condition
>> to pick the array length:
>>
>> template <int N>
>> inline unsigned int
>> wi::extended_tree <N>::get_len () const
>> {
>> if (N == MAX_BITSIZE_MODE_ANY_INT
>> || N > TYPE_PRECISION (TREE_TYPE (m_t)))
>> return TREE_INT_CST_EXT_NUNITS (m_t);
>> else
>> return TREE_INT_CST_NUNITS (m_t);
>> }
>>
>> and this TYPE_PRECISION condition was relatively hot in
>> get_ref_base_and_extent when compiling insn-recog.ii.
>>
>> Adding a third length for offset_int does seem to reduce the cost of
>> the offset_int + to_offset addition.
>>
>> Tested on x86_64-linux-gnu. OK to install?
>
> Hmm, that's now getting a bit excessive ...
Well, we access trees in three different ways, and we want all of them
to be cheap, so having three fields in the tree seems pretty natural.
> inline unsigned int
> wi::extended_tree <N>::get_len () const
> {
> - if (N == MAX_BITSIZE_MODE_ANY_INT
> - || N > TYPE_PRECISION (TREE_TYPE (m_t)))
> + if (N == ADDR_MAX_PRECISION)
> + return TREE_INT_CST_OFFSET_NUNITS (m_t);
> + else if (N == MAX_BITSIZE_MODE_ANY_INT
> + || N > TYPE_PRECISION (TREE_TYPE (m_t)))
> return TREE_INT_CST_EXT_NUNITS (m_t);
> else
> return TREE_INT_CST_NUNITS (m_t);
>
> Shouldn't it be N >= TYPE_PRECISION () btw?
No, TREE_INT_CST_NUNITS is for accessing the tree in TYPE_PRECISION
and TREE_INT_CST_EXT_NUNITS is for accessing it in wider precisions.
N is always >= TYPE_PRECISION here, from the assert in the constructor:
gcc_checking_assert (TYPE_PRECISION (TREE_TYPE (t)) <= N);
TREE_INT_CST_OFFSET_NUNITS is just a cached x ? TREE_INT_CST_NUNITS (m_t)
: TREE_INT_CST_EXT_NUNITS (m_t) result for a particular precision.
> Looking at the implementation it seems it would also work with
>
> return MIN (TREE_INT_CST_EXT_NUNITS (m_t), N / HOST_BITS_PER_WIDE_INT);
Yeah, the MIN in the patch was probably bogus sorry. It only works
if we can assume that no bitsizetype will have ADDR_MAX_PRECISION
significant (non-sign) bits -- in particular that there's no such thing as
an all-1s _unsigned_ bitsizetype. That might be true in practice given
the way we use offsets, but it isn't safe to generalise that to all N.
A safer form would be:
if (ext_len > OFFSET_INT_ELTS)
TREE_INT_CST_OFFSET_NUNITS (t) = len;
else
TREE_INT_CST_OFFSET_NUNITS (t) = ext_len;
The reason the general form doesn't work for all N is because of the
compressed representation. E.g. the example I gave a while ago about
a 256-bit all-1s unsigned number being { -1 } as a 256-bit number and
{ -1, -1, -1, -1, 0 } as a 257+-bit number.
But the point of the patch is to avoid any runtime checks here,
so the TYPE_PRECISION case is never actually used now. I just kept
it around for completeness, since we'd been using it successfully so far.
I can put in a gcc_unreachable if you prefer.
Thanks,
Richard
