Jeff Law <jeffreya...@gmail.com> writes:
> On 12/30/24 8:16 AM, Richard Sandiford wrote:
>
>>
>> The divisor is by definition 1. I think dropping it would make the
>> loop more obviously correct, since the same assumption is implicit in
>> the loop body.
> I'll likely pick this up as Robin is on PTO for the next ~10 days. So
> just to be 100% clear you want the loop iteration to look like:
>
> for (unsigned i = 0; i < GET_MODE_SIZE; i++)
>
> Which seems sensible. Just want to be sure as I haven't been following
> too closely.
Yeah, that's right. (I wondered whether we could just iterate on
buffer.length (), but then I thought that Robin's GET_MODE_SIZE
version was clearer.)
>>>> + i++)
>>>> {
>>>> - unsigned HOST_WIDE_INT value = 0;
>>>> - unsigned int limit = MIN (nelts - i, elts_per_int);
>>>> - for (unsigned int j = 0; j < limit; ++j)
>>>> - {
>>>> - auto elt = INTVAL (CONST_VECTOR_ELT (x, i + j));
>>>> - value |= (elt & mask) << (j * elt_bits);
>>>> - }
>>>> - output_constant_pool_2 (int_mode, gen_int_mode (value,
>>>> int_mode),
>>>> - i != 0 ? MIN (align, int_bits) :
>>>> align);
>>>> + unsigned HOST_WIDE_INT value = buffer[i];
>>>> + output_constant_pool_2 (byte_mode, gen_int_mode (value,
>>>> byte_mode),
>>>> + i == 0 ? 0 : 1);
>>
>> This should be i == 0 ? align : 1
>>
>> OK with those three changes, thanks. I tested on aarch64 and similarly
>> saw no regressions. I don't think we need to test big-endian aarch64
>> specifically.
> ACK on the testing. I could fire up s390x as a final sanity check if
> you think it's worthwhile; it takes a day or so in qemu, but it's easy
> enough to fire and forget...
s390x is unlikely to cover this, since it doesn't have any
VECTOR_BOOL modes.
Given that the aarch64 tests weren't sensitive to the choice of
byte_mode for little-endian, they seem unlikely to be sensitive
to it for big-endian. So I think we're good. :)
Thanks,
Richard
