This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [wide-int] Make the main tree decompose cheaper
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: zadeck at naturalbridge dot com, mikestump at comcast dot net, gcc-patches at gcc dot gnu dot org
- Date: Thu, 24 Oct 2013 20:57:57 +0100
- Subject: Re: [wide-int] Make the main tree decompose cheaper
- Authentication-results: sourceware.org; auth=none
- References: <877gd34rmr dot fsf at talisman dot default> <alpine dot LNX dot 2 dot 00 dot 1310241041330 dot 11149 at zhemvz dot fhfr dot qr>
Richard Biener <rguenther@suse.de> writes:
> On Wed, 23 Oct 2013, Richard Sandiford wrote:
>> This patch stores two array lengths in an INTEGER_CST: the length that
>> should be used when accessing the constant in its TYPE_PRECISION,
>> and the one that should be used for wider precisions. This means that
>> the main tree decompose routine is just a simple storage_ref constructor.
>> It also means that cst_fits_shwi_p can be simplified back to its earlier
>> form (before r203602).
>>
>> The patch also fixes wi::extended_tree <N>::get_len so that it uses
>> the right length when a tree has the same precision as addr_wide_int.
>> I just noticed this by inspection, I don't have a testcase.
>>
>> (It might be better to require "address" trees to be smaller than
>> addr_wide_int though -- I can look into that if you agree. But just
>> changing:
>>
>> gcc_checking_assert (TYPE_PRECISION (TREE_TYPE (t)) <= N);
>>
>> to:
>>
>> gcc_checking_assert (TYPE_PRECISION (TREE_TYPE (t)) < N);
>>
>> did trigger when I tried it in the first version of the wi::address patch.)
>>
>> Tested on powerpc64-linux-gnu. OK for wide-int?
>
> I like how this simplifies code, still storing two lengths is a bit
> odd ... ah well.
>
> Minor nits:
>
> + /* Make sure no one is clobbering the shared constant. We
> + must be careful here because tree-csts and wide-ints are
> + not canonicalized in the same way. */
> + gcc_assert (TREE_TYPE (t) == type);
> + gcc_assert (TREE_INT_CST_NUNITS (t) == 1);
> + gcc_assert (TREE_INT_CST_EXT_NUNITS (t) == 1);
> + gcc_assert (TREE_INT_CST_ELT (t, 0) == hwi);
>
> please merge into a single gcc_assert.
OK.
>> + /* The number of HOST_WIDE_INTs in an INTEGER_CST. */
>> + struct {
>> + /* The number of HOST_WIDE_INTs if the INTEGER_CST is accessed in
>> + its native precision. */
>> + unsigned short unextended;
>> +
>> + /* The number of HOST_WIDE_INTs if the INTEGER_CST is extended to
>> + wider precisions based on its TYPE_SIGN. */
>> + unsigned short extended;
>> + } split_length;
>
> split_length sounds generic while unextended and extended not. I suggest
> to either rename split_length to something clearly integer constant
> specific or renaming unextended and extended to length1 and length2.
Yeah, good point. I changed it to "int_length" instead.
Committed with those changes, thanks.
Richard