On Fri, 2011-05-06 at 13:34 +0100, Julian Brown wrote:
> Hi,
>
> This is the first of two patches to add unaligned-access support to the
> ARM backend. This is done somewhat differently to Jie Zhang's earlier
> patch:
>
> http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01890.html
>
> In that with Jie's patch, *any* pointer dereference would be allowed to
> access unaligned data. This has the undesirable side-effect of
> disallowing instructions which don't support unaligned accesses (LDRD,
> LDM etc.) when unaligned accesses are enabled.
>
> Instead, this patch enables only packed-structure accesses to use
> ldr/str/ldrh/strh, by taking a hint from the MIPS ldl/ldr
> implementation. I figured the unaligned-access ARM case is kind of
> similar to those, except that normal loads/stores are used, and the
> shifting/merging happens in hardware.
>
> The standard names extv/extzv/insv can take a memory
> operand for the source/destination of the extract/insert operation, so
> we just expand to unspec'ed versions of the load and store operations
> when unaligned-access support is enabled: the benefit of doing that
> rather than, say, expanding using the regular movsi pattern is that we
> bypass any smartness in the compiler which might replace operations
> which work for unaligned accesses (ldr/str/ldrh/strh) with operations
> which don't work (ldrd/strd/ldm/stm/vldr/...). The downside is we might
> potentially miss out on optimization opportunities (since these things
> no longer look like plain memory accesses).
>
> Doing things this way allows us to leave the settings for
> STRICT_ALIGNMENT/SLOW_BYTE_ACCESS alone, avoiding the disruption that
> changing them might cause.
>
> 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.
>
> The logic for choosing when to enable the unaligned-access support (and
> the name of the option to override the default behaviour) is lifted from
> Jie's patch.
>
> Tested with cross to ARM Linux, and (on a branch) in both little &
> big-endian mode cross to ARM EABI, with no regressions. OK to apply?
>
> Thanks,
>
> Julian
>
> ChangeLog
>
> gcc/
> * config/arm/arm.c (arm_override_options): Add unaligned_access
> support.
> * 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.
> (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.
> * expmed.c (store_bit_field_1): Don't tweak bitfield numbering for
> memory locations if BITS_BIG_ENDIAN != BYTES_BIG_ENDIAN.
> (extract_bit_field_1): Likewise.
+ if (unaligned_access == 2)
+ {
+ if (arm_arch6 && (arm_arch_notm || arm_arch7))
+ unaligned_access = 1;
+ else
+ unaligned_access = 0;
+ }
+
The compiler should fault -munaligned-access on cores that don't support
it.
+(define_insn "unaligned_loadsi"
+ [(set (match_operand:SI 0 "s_register_operand" "=r")
+ (unspec:SI [(match_operand:SI 1 "memory_operand" "m")]
+ UNSPEC_UNALIGNED_LOAD))]
+ "unaligned_access"
+ "ldr%?\t%0, %1\t@ unaligned"
+ [(set_attr "predicable" "yes")
+ (set_attr "type" "load1")])
I think the final condition should also include TARGET_32BIT, as these
patterns aren't expected to work with Thumb-1.
Secondly, they should be structured to get the length information right
when a 16-bit encoding can be used in Thumb-2 (you'll keep Carrot happy
that way :-) : just add an alternative that's only enabled for Thumb
mode and which matches the requirements for a 16-bit instruction (beware
however of the 16-bit write-back case).
Finally, I don't see anything to put out the correct build attribute
information for unaligned access enabled (Tag_CPU_unaligned_access).
Have I just missed it?
R.
