Eric Botcazou <ebotca...@adacore.com> writes:
>> Yeah, I don't think constants are any different here. One fix might be
>> to use simplify_expand_binop instead of extract_bit_field, as per the
>> patch below. The patch also restricts the shifting to forward walks,
>> as discussed in the PR trail.
>
> This looks fine to me, please apply (with a few words added to the comment,
> e.g. "...doesn't have full wordsize and we transfer in memory order...").
>
>> The point of r8007 seems to be that WORDS_BIG_ENDIAN doesn't apply to
>> BLKmode fieldmodes, but there must surely be some endianness involved
>> somewhere, given that we're extracting words from a multiword value?
>
> I think that BLKmode objects are treated as left-justified in memory order,
> i.e. left-justified on big-endian and right-justified on little-endian.
>
>> So if possible, I'd prefer to revert the original patch for this PR
>> and instead adjust the "backwards" test. I'm just not sure whether
>> it should be "WORDS_BIG_ENDIAN" (reverting r8007, which was obviously
>> applied for a reason) or something like:
>>
>> (fieldmode == BLKmode ? BYTES_BIG_ENDIAN : WORDS_BIG_ENDIAN)
>
> This seems to be a little risky for stage 3 though.
What's the main potential problem you see? The backwards condition:
(fieldmode == BLKmode ? BYTES_BIG_ENDIAN : WORDS_BIG_ENDIAN)
should apply the same bit-for-bit mapping between source and target
as the patch applies. The only difference should be in the order that
the reads and writes happen. But does that matter? If we were worried
about something like volatile MEMs here, I'd expect a check for that,
since volatile MEMs can be something like doubleword as well as BLKmode.
Richard
