Richard Biener <richard.guent...@gmail.com> writes:
> On Mon, Jul 29, 2019 at 11:11 AM Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>>
>> inchash::hash::add_wide_int operated directly on the raw encoding
>> of the wide_int, including any redundant upper bits. The problem
>> with that is that the upper bits are only defined for some wide-int
>> storage types (including wide_int itself). wi::to_wide(tree) instead
>> returns a value that is extended according to the signedness of the
>> type (so that wi::to_widest can use the same encoding) while rtxes
>> have the awkward special case of BI, which can be zero-extended
>> rather than sign-extended.
>>
>> In the PR, we computed a hash for a "normal" sign-extended wide_int
>> while the existing entries hashed wi::to_wide(tree). This gives
>> different results for unsigned types that have the top bit set.
>>
>> The patch fixes that by hashing the canonical sign-extended form even
>> if the raw encoding happens to be different.
>>
>> Tested on aarch64-linux-gnu (with and without SVE), armeb-eabi
>> and x86_64-linux-gnu. OK to install?
>
> But only the most significant elt needs this treatment, no? So
> we can save some compile-time in the hasing by not doing
> it for all elts.
Yeah, we only need it for the most significant elt. But the vast
majority of constants need 64 bits or fewer of encoding anyway
(even if the precision is greater than 64 bits), so that's usually the
only one we need to process. I'm not convinced handling multiple elts
specially would pay off.
Thanks,
Richard
>
>> Richard
>>
>>
>> 2019-07-29 Richard Sandiford <richard.sandif...@arm.com>
>>
>> gcc/
>> * wide-int.h (generic_wide_int::sext_elt): New function.
>> * inchash.h (hash::add_wide_int): Use it instead of elt.
>>
>> Index: gcc/wide-int.h
>> ===================================================================
>> --- gcc/wide-int.h 2019-07-10 19:41:26.391898059 +0100
>> +++ gcc/wide-int.h 2019-07-29 10:08:12.048610030 +0100
>> @@ -730,6 +730,7 @@ class GTY(()) generic_wide_int : public
>> /* Public accessors for the interior of a wide int. */
>> HOST_WIDE_INT sign_mask () const;
>> HOST_WIDE_INT elt (unsigned int) const;
>> + HOST_WIDE_INT sext_elt (unsigned int) const;
>> unsigned HOST_WIDE_INT ulow () const;
>> unsigned HOST_WIDE_INT uhigh () const;
>> HOST_WIDE_INT slow () const;
>> @@ -909,6 +910,23 @@ generic_wide_int <storage>::elt (unsigne
>> return this->get_val ()[i];
>> }
>>
>> +/* Like elt, but sign-extend beyond the upper bit, instead of returning
>> + the raw encoding. */
>> +template <typename storage>
>> +inline HOST_WIDE_INT
>> +generic_wide_int <storage>::sext_elt (unsigned int i) const
>> +{
>> + HOST_WIDE_INT elt_i = elt (i);
>> + if (!is_sign_extended)
>> + {
>> + unsigned int precision = this->get_precision ();
>> + unsigned int lsb = i * HOST_BITS_PER_WIDE_INT;
>> + if (precision - lsb < HOST_BITS_PER_WIDE_INT)
>> + elt_i = sext_hwi (elt_i, precision - lsb);
>> + }
>> + return elt_i;
>> +}
>> +
>> template <typename storage>
>> template <typename T>
>> inline generic_wide_int <storage> &
>> Index: gcc/inchash.h
>> ===================================================================
>> --- gcc/inchash.h 2019-03-08 18:15:36.704740334 +0000
>> +++ gcc/inchash.h 2019-07-29 10:08:12.048610030 +0100
>> @@ -85,7 +85,7 @@ hashval_t iterative_hash_hashval_t (hash
>> {
>> add_int (x.get_len ());
>> for (unsigned i = 0; i < x.get_len (); i++)
>> - add_hwi (x.elt (i));
>> + add_hwi (x.sext_elt (i));
>> }
>>
>> /* Hash in pointer PTR. */
