[ARM] Unaligned accesses for packed structures [1/2]

On Mon, 4 Jul 2011 13:43:31 +0200 (CEST)
"Ulrich Weigand" <uweigand@de.ibm.com> wrote:

> Julian Brown wrote:
> 
> > The most awkward change in the patch is to generic code (expmed.c,
> > {store,extract}_bit_field_1): in big-endian mode, the existing
> > behaviour (when inserting/extracting a bitfield to a memory
> > location) is definitely bogus: "unit" is set to BITS_PER_UNIT for
> > memory locations, and if bitsize (the size of the field to
> > insert/extract) is greater than BITS_PER_UNIT (which isn't unusual
> > at all), xbitpos becomes negative. That can't possibly be
> > intentional; I can only assume that this code path is not exercised
> > for machines which have memory alternatives for bitfield
> > insert/extract, and BITS_BIG_ENDIAN of 0 in BYTES_BIG_ENDIAN mode.

> I agree that the current code cannot possibly be correct.  However,
> just disabling the BITS_BIG_ENDIAN != BYTES_BIG_ENDIAN renumbering
> *completely* seems wrong to me as well.  
> 
> According to the docs, the meaning bit position passed to the
> extv/insv expanders is determined by BITS_BIG_ENDIAN, both in the
> cases of register and memory operands.  Therefore, if BITS_BIG_ENDIAN
> differs from BYTES_BIG_ENDIAN, we should need a correction for memory
> operands as well.  However, this correction needs to be relative to
> the size of the access (i.e. the operand to the extv/insn), not just
> BITS_PER_UNIT.

> Note that with that change, the new code your patch introduces to the
> ARM back-end will also need to change.  You currently handle bitpos
> like this:
> 
>           base_addr = adjust_address (operands[1], HImode,
>                                       bitpos / BITS_PER_UNIT);
> 
> This implicitly assumes that bitpos counts according to
> BYTES_BIG_ENDIAN, not BITS_BIG_ENDIAN -- which exactly cancels out
> the common code behaviour introduced by your patch ...

I've updated the patch to work with current mainline, and implemented
your suggestion along with the change of the interpretation of bitpos in
the insv/extv/extzv expanders in arm.md. It seems to work fine
(testing still in progress), but I'm a bit concerned that the semantics
of bit-positioning for memory operands when BYTES_BIG_ENDIAN
&& !BITS_BIG_ENDIAN are now strange to the point of perversity.

The problem is, if we're using little-endian bit numbering for memory
locations in big-endian-bytes mode, we need to define an origin from
which to count "backwards" from. For the current implementation, this
will now be (I believe) one word beyond the base address of the access
in question, which is IMO slightly bizarre, to say the least. But, I
can't think of any other easy ways forward than either this patch, or
the previous one which disabled bit-endian switching entirely for
memory operands in this case.

So, OK to apply this version, assuming testing comes out OK? (And the
followup patch [2/2], which remains unchanged?)

Thanks,

Julian

ChangeLog

    gcc/
    * config/arm/arm.c (arm_override_options): Add unaligned_access
    support.
    (arm_file_start): Emit attribute for unaligned access as
    appropriate.
    * config/arm/arm.md (UNSPEC_UNALIGNED_LOAD)
    (UNSPEC_UNALIGNED_STORE): Add constants for unspecs.
    (insv, extzv): Add unaligned-access support.
    (extv): Change to expander. Likewise.
    (extzv_t1, extv_regsi): Add helpers.
    (unaligned_loadsi, unaligned_loadhis, unaligned_loadhiu)
    (unaligned_storesi, unaligned_storehi): New.
    (*extv_reg): New (previous extv implementation).
    * config/arm/arm.opt (munaligned_access): Add option.
    * config/arm/constraints.md (Uw): New constraint.
    * expmed.c (store_bit_field_1): Adjust bitfield numbering according
    to size of access, not size of unit, when BITS_BIG_ENDIAN !=
    BYTES_BIG_ENDIAN.
    (extract_bit_field_1): Likewise.

[ARM] Unaligned accesses for packed structures [1/2]

Commit Message

Comments

Patch