On Fri, Sep 25, 2026 at 2:37 PM Dominic P <[email protected]> wrote:
>
> On a STRICT_ALIGNMENT target store merging never widens a group below its
> alignment, even where movmisalign<mode> is one unaligned str or strh. Let
> such
> targets use the aligned/unaligned store-count comparison for each size whose
> movmisalign expansion, like every smaller one split_group may shrink it to,
> costs no more for size than the aligned accesses it replaces. arm's unaligned
> strh and str pass. Alpha without BWX (EV5) merges and shrinks; alpha with
> BWX,
> whose byte stores are cheaper, is unchanged.
>
> Pi1MHz firmware (arm1176, -O2, LTO): .text -416 bytes; a merged strh takes 30%
> fewer cycles than the two strb it replaces.
>
> Bootstrapped and regression tested on x86_64-pc-linux-gnu. Regression tested
> on arm-none-eabi (arm.exp, dg.exp, tree-ssa.exp; compile-only Cortex-A15 Arm
> and
> Thumb, Cortex-M0, Armv8-A); execute.exp, dg-torture.exp and dg.exp are
> unchanged
> on qemu arm1176 with the MMU on.
>
> Assisted-by: Claude Opus 5 (Anthropic)
>
> gcc/ChangeLog:
>
> * gimple-ssa-store-merging.cc: Include memmodel.h, emit-rtl.h and
> optabs.h.
> (max_access_size): New function.
> (imm_store_chain_info::try_coalesce_bswap): Use it for the store and
> the load alignment checks.
> (split_group): Use it to limit unaligned stores on STRICT_ALIGNMENT
> targets and loads below their alignment.
> (imm_store_chain_info::output_merged_store): Allow unaligned stores
> on STRICT_ALIGNMENT targets but not unaligned loads. Adjust the
> function comment.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/torture/store-merging-misaligned.c: New test.
> * gcc.target/arm/store-merge-unaligned.c: New test.
> * gcc.target/arm/store-merge-unaligned-2.c: New test.
> * gcc.target/arm/store-merge-unaligned-3.c: New test.
> * gcc.target/arm/store-merge-unaligned-4.c: New test.
>
> Signed-off-by: Dominic P <[email protected]>
> ---
> Changes in v2: no new hook (Andrea Pinski). Store merging asks about its own
> transform (Richard Biener): below the alignment it uses movmisalign<mode> only
> where that costs no more than the aligned accesses it replaces. The arm patch
> is gone; the tests check function bodies, with SLP off where it merged first.
>
> Building RTL at GIMPLE time for costs has precedent (ivopts' computation_cost,
> tree-ssa-address, reassoc and switch conversion); here it asks the target only
> about the one sequence the pass would emit.
>
> OK for trunk?
>
> gcc/gimple-ssa-store-merging.cc | 56 +++++++++--
> .../gcc.dg/torture/store-merging-misaligned.c | 43 ++++++++
> .../gcc.target/arm/store-merge-unaligned-2.c | 22 +++++
> .../gcc.target/arm/store-merge-unaligned-3.c | 20 ++++
> .../gcc.target/arm/store-merge-unaligned-4.c | 20 ++++
> .../gcc.target/arm/store-merge-unaligned.c | 99 +++++++++++++++++++
> 6 files changed, 253 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/torture/store-merging-misaligned.c
> create mode 100644 gcc/testsuite/gcc.target/arm/store-merge-unaligned-2.c
> create mode 100644 gcc/testsuite/gcc.target/arm/store-merge-unaligned-3.c
> create mode 100644 gcc/testsuite/gcc.target/arm/store-merge-unaligned-4.c
> create mode 100644 gcc/testsuite/gcc.target/arm/store-merge-unaligned.c
>
> diff --git a/gcc/gimple-ssa-store-merging.cc b/gcc/gimple-ssa-store-merging.cc
> index 39ef59ab9a6..7f086f1fb35 100644
> --- a/gcc/gimple-ssa-store-merging.cc
> +++ b/gcc/gimple-ssa-store-merging.cc
> @@ -166,7 +166,10 @@
> #include "target.h"
> #include "gimplify-me.h"
> #include "rtl.h"
> +#include "memmodel.h"
> +#include "emit-rtl.h"
> #include "expr.h" /* For get_bit_range. */
> +#include "optabs.h"
> #include "optabs-tree.h"
> #include "dbgcnt.h"
> #include "selftest.h"
> @@ -2952,6 +2955,44 @@ check_no_overlap (const vec<store_immediate_info *>
> &m_store_info,
> return true;
> }
>
> +/* Return the largest power of two of at most SIZE bits that a load (if LOAD)
> + or store at ALIGN bits of alignment may use. A size above ALIGN needs a
> + movmisalign whose size cost, like that of every smaller such size, is no
> + more than that of the aligned accesses it replaces. */
> +
> +static unsigned HOST_WIDE_INT
> +max_access_size (unsigned HOST_WIDE_INT size, unsigned HOST_WIDE_INT align,
> + bool load)
> +{
> + unsigned HOST_WIDE_INT s = MIN (size, align);
> + if (!param_store_merging_allow_unaligned || s == size)
> + return s;
> + rtx addr = gen_raw_REG (Pmode, LAST_VIRTUAL_REGISTER + 1);
> + rtx mem = gen_rtx_MEM (int_mode_for_size (s, 0).require (), addr);
> + rtx reg = gen_raw_REG (GET_MODE (mem), LAST_VIRTUAL_REGISTER + 2);
> + start_sequence ();
> + emit_move_insn (load ? reg : mem, load ? mem : reg);
> + unsigned aligned_cost = seq_cost (end_sequence (), false);
> + for (; s < size; s *= 2)
> + {
> + scalar_int_mode mode = int_mode_for_size (s * 2, 0).require ();
it seems 'size' needs to be power-of-two but this isn't documented?
You should probably be using FOR_EACH_WIDER_MODE
or FOR_EACH_2XWIDER_MODE which is more efficient.
> + insn_code icode = optab_handler (movmisalign_optab, mode);
> + if (icode == CODE_FOR_nothing)
Not sure if we have such target, but targetm.slow_unaligned_access
might be false for 'mode'. We do have targets that do not define
STRICT_ALIGNMENT but have targetm.slow_unaligned_access for
some modes.
As indicated in the previous mail we should eliminate the STRICT_ALIGNMENT
macro in favor of testing targetm.slow_unaligned_access.
> + break;
> + mem = gen_rtx_MEM (mode, addr);
> + set_mem_align (mem, align);
> + reg = gen_raw_REG (mode, LAST_VIRTUAL_REGISTER + 2);
> + class expand_operand ops[2];
> + create_fixed_operand (&ops[0], load ? reg : mem);
> + create_fixed_operand (&ops[1], load ? mem : reg);
> + start_sequence ();
> + expand_insn (icode, 2, ops);
> + if (seq_cost (end_sequence (), false) > aligned_cost * (s * 2 / align))
> + break;
Given all this incurs quite some cost the result should be cached somehow,
possibly allowing a cheap 'is size ok for align' query as well rather than only
finding the maximum.
> + }
> + return s;
> +}
> +
> /* Return true if m_store_info[first] and at least one following store
> form a group which store try_size bitsize value which is byte swapped
> from a memory load or some value, or identity from some value.
> @@ -3006,7 +3047,7 @@ imm_store_chain_info::try_coalesce_bswap
> (merged_store_group *merged_store,
> = (m_store_info[first]->bitpos - align_base) & (align - 1);
> if (align_bitpos)
> align = least_bit_hwi (align_bitpos);
> - if (align < try_size)
> + if (max_access_size (try_size, align, false) < try_size)
this needs the answer for exactly one size, not the "max"?
> return false;
> }
>
> @@ -3145,7 +3186,7 @@ imm_store_chain_info::try_coalesce_bswap
> (merged_store_group *merged_store,
> if (!allow_unaligned && n.base_addr)
> {
> unsigned int align = get_object_alignment (n.src);
> - if (align < try_size)
> + if (max_access_size (try_size, align, true) < try_size)
> return false;
> }
>
> @@ -3933,6 +3974,8 @@ split_group (merged_store_group *group, bool
> allow_unaligned_store,
> align = least_bit_hwi (align_bitpos);
> if (!allow_unaligned_store)
> try_size = MIN (try_size, align);
> + else if (STRICT_ALIGNMENT)
> + try_size = max_access_size (try_size, align, false);
> if (!allow_unaligned_load)
> {
> /* If we can't do or don't want to do unaligned stores
> @@ -3956,7 +3999,7 @@ split_group (merged_store_group *group, bool
> allow_unaligned_store,
> load_align = MIN (load_align, a);
> }
> }
> - try_size = MIN (try_size, load_align);
> + try_size = max_access_size (try_size, load_align, true);
You said you are not adjusting loads?
> }
> store_immediate_info *info
> = find_constituent_stores (group, NULL, &first, try_bitpos, try_size);
> @@ -4229,7 +4272,7 @@ invert_op (split_store *split_store, int idx, tree
> int_type, tree &mask)
> /* Given a merged store group GROUP output the widened version of it.
> The store chain is against the base object BASE.
> Try store sizes of at most MAX_STORE_BITSIZE bits wide and don't output
> - unaligned stores for STRICT_ALIGNMENT targets or if it's too expensive.
> + unaligned stores if it's too expensive.
> Make sure that the number of statements output is less than the number of
> original statements. If a better sequence is possible emit it and
> return true. */
> @@ -4243,9 +4286,8 @@ imm_store_chain_info::output_merged_store
> (merged_store_group *group)
> if (orig_num_stmts < 2)
> return false;
>
> - bool allow_unaligned_store
> - = !STRICT_ALIGNMENT && param_store_merging_allow_unaligned;
> - bool allow_unaligned_load = allow_unaligned_store;
> + bool allow_unaligned_store = param_store_merging_allow_unaligned;
> + bool allow_unaligned_load = !STRICT_ALIGNMENT && allow_unaligned_store;
> bool bzero_first = false;
> store_immediate_info *store;
> unsigned int num_clobber_stmts = 0;
> diff --git a/gcc/testsuite/gcc.dg/torture/store-merging-misaligned.c
> b/gcc/testsuite/gcc.dg/torture/store-merging-misaligned.c
> new file mode 100644
> index 00000000000..6a8e8791fee
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/store-merging-misaligned.c
> @@ -0,0 +1,43 @@
> +/* Byte stores merged below their alignment must store the right bytes. */
> +
> +/* { dg-do run } */
> +
> +typedef __UINT8_TYPE__ u8;
> +typedef __UINT32_TYPE__ u32;
> +
> +static u8 buf[24];
> +
> +static void __attribute__((noipa))
> +st_h (u8 *p, u32 v) { p[0] = v; p[1] = v >> 8; }
> +
> +static void __attribute__((noipa))
> +st_w (u8 *p, u32 v)
> +{ p[0] = v; p[1] = v >> 8; p[2] = v >> 16; p[3] = v >> 24; }
> +
> +static void __attribute__((noipa))
> +st_wbe (u8 *p, u32 v)
> +{ p[0] = v >> 24; p[1] = v >> 16; p[2] = v >> 8; p[3] = v; }
> +
> +int
> +main (void)
> +{
> + for (int off = 0; off < 8; off++)
> + {
> + __builtin_memset (buf, 0xa5, sizeof (buf));
> + st_h (buf + off, 0x1234);
> + st_w (buf + off + 4, 0x89abcdef);
> + st_wbe (buf + off + 12, 0x89abcdef);
> + if (buf[off] != 0x34 || buf[off + 1] != 0x12)
> + __builtin_abort ();
> + if (buf[off + 4] != 0xef || buf[off + 5] != 0xcd
> + || buf[off + 6] != 0xab || buf[off + 7] != 0x89)
> + __builtin_abort ();
> + if (buf[off + 12] != 0x89 || buf[off + 13] != 0xab
> + || buf[off + 14] != 0xcd || buf[off + 15] != 0xef)
> + __builtin_abort ();
> + if (buf[off + 2] != 0xa5 || buf[off + 3] != 0xa5
> + || buf[off + 8] != 0xa5 || buf[off + 11] != 0xa5)
> + __builtin_abort ();
> + }
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/arm/store-merge-unaligned-2.c
> b/gcc/testsuite/gcc.target/arm/store-merge-unaligned-2.c
> new file mode 100644
> index 00000000000..3517c9c3c41
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/store-merge-unaligned-2.c
> @@ -0,0 +1,22 @@
> +/* Without unaligned access there is no movmisalign, so nothing merges. */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm_arch_v6_arm_ok } */
> +/* { dg-options "-O2 -mno-unaligned-access -fno-tree-slp-vectorize" } */
> +/* { dg-add-options arm_arch_v6_arm } */
> +
> +typedef unsigned char u8;
> +typedef unsigned int u32;
> +
> +void h_le (u8 *p, u32 v) { p[0] = v; p[1] = v >> 8; }
> +void w_le (u8 *p, u32 v)
> +{ p[0] = v; p[1] = v >> 8; p[2] = v >> 16; p[3] = v >> 24; }
> +void cph (u8 *__restrict d, const u8 *__restrict q)
> +{
> + u8 *p = __builtin_assume_aligned (d, 2);
> + p[0] = q[0]; p[1] = q[1];
> +}
> +
> +/* { dg-final { scan-assembler-not {\tstrh\t} } } */
> +/* { dg-final { scan-assembler-not {\tstr\t} } } */
> +/* { dg-final { scan-assembler-not {\tldrh\t} } } */
> diff --git a/gcc/testsuite/gcc.target/arm/store-merge-unaligned-3.c
> b/gcc/testsuite/gcc.target/arm/store-merge-unaligned-3.c
> new file mode 100644
> index 00000000000..bd2e2c26ea9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/store-merge-unaligned-3.c
> @@ -0,0 +1,20 @@
> +/* --param=store-merging-allow-unaligned=0 stops the unaligned merges. */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm_arch_v6_arm_ok } */
> +/* { dg-options "-O2 -fno-tree-slp-vectorize
> --param=store-merging-allow-unaligned=0" } */
> +/* { dg-add-options arm_arch_v6_arm } */
> +
> +typedef unsigned char u8;
> +typedef unsigned int u32;
> +
> +void h_le (u8 *p, u32 v) { p[0] = v; p[1] = v >> 8; }
> +void h_be (u8 *p, u32 v) { p[0] = v >> 8; p[1] = v; }
> +void cph (u8 *__restrict d, const u8 *__restrict q)
> +{
> + u8 *p = __builtin_assume_aligned (d, 2);
> + p[0] = q[0]; p[1] = q[1];
> +}
> +
> +/* { dg-final { scan-assembler-not {\tstrh\t} } } */
> +/* { dg-final { scan-assembler-not {\tldrh\t} } } */
> diff --git a/gcc/testsuite/gcc.target/arm/store-merge-unaligned-4.c
> b/gcc/testsuite/gcc.target/arm/store-merge-unaligned-4.c
> new file mode 100644
> index 00000000000..9cdf9f7bf14
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/store-merge-unaligned-4.c
> @@ -0,0 +1,20 @@
> +/* The bswap path: a reversed unaligned copy becomes ldr, rev and str. */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm_arch_v6_arm_ok } */
> +/* { dg-options "-O2 -fno-tree-slp-vectorize" } */
> +/* { dg-add-options arm_arch_v6_arm } */
> +/* { dg-add-options check_function_bodies } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +typedef unsigned char u8;
> +
> +/*
> +** copy_be:
> +** ldr (r[0-9]+), \[r1\] @ unaligned
> +** rev \1, \1
> +** str \1, \[r0\] @ unaligned
> +** bx lr
> +*/
> +void copy_be (u8 *__restrict p, const u8 *__restrict q)
> +{ p[0] = q[3]; p[1] = q[2]; p[2] = q[1]; p[3] = q[0]; }
> diff --git a/gcc/testsuite/gcc.target/arm/store-merge-unaligned.c
> b/gcc/testsuite/gcc.target/arm/store-merge-unaligned.c
> new file mode 100644
> index 00000000000..c3dd7350dde
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/store-merge-unaligned.c
> @@ -0,0 +1,99 @@
> +/* Stores below their alignment merge into unaligned strh or str on Armv6.
> */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target arm_arch_v6_arm_ok } */
> +/* { dg-require-effective-target arm_little_endian } */
> +/* { dg-options "-O2 -fno-tree-slp-vectorize" } */
> +/* { dg-add-options arm_arch_v6_arm } */
> +/* { dg-add-options check_function_bodies } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +typedef unsigned char u8;
> +typedef unsigned short u16;
> +typedef unsigned int u32;
> +
> +/*
> +** h_le:
> +** strh r1, \[r0\] @ unaligned
> +** bx lr
> +*/
> +void h_le (u8 *p, u32 v) { p[0] = v; p[1] = v >> 8; }
> +
> +/*
> +** h_be:
> +** rev16 (r[0-9]+), r1
> +** strh \1, \[r0\] @ unaligned
> +** bx lr
> +*/
> +void h_be (u8 *p, u32 v) { p[0] = v >> 8; p[1] = v; }
> +
> +/*
> +** w_le:
> +** str r1, \[r0\] @ unaligned
> +** bx lr
> +*/
> +void w_le (u8 *p, u32 v)
> +{ p[0] = v; p[1] = v >> 8; p[2] = v >> 16; p[3] = v >> 24; }
> +
> +/*
> +** w_be:
> +** rev (r[0-9]+), r1
> +** str \1, \[r0\] @ unaligned
> +** bx lr
> +*/
> +void w_be (u8 *p, u32 v)
> +{ p[0] = v >> 24; p[1] = v >> 16; p[2] = v >> 8; p[3] = v; }
> +
> +/*
> +** hh:
> +** str r1, \[r0\] @ unaligned
> +** bx lr
> +*/
> +void hh (u16 *p, u32 v) { p[0] = v; p[1] = v >> 16; }
> +
> +/*
> +** t3:
> +** lsr (r[0-9]+), r1, #16
> +** strh r1, \[r0\] @ unaligned
> +** strb \1, \[r0, #2\]
> +** bx lr
> +*/
> +void t3 (u8 *p, u32 v) { p[0] = v; p[1] = v >> 8; p[2] = v >> 16; }
> +
> +/*
> +** cp4:
> +** ldr (r[0-9]+), \[r1\] @ unaligned
> +** str \1, \[r0\] @ unaligned
> +** bx lr
> +*/
> +void cp4 (u8 *__restrict p, const u8 *__restrict q)
> +{ p[0] = q[0]; p[1] = q[1]; p[2] = q[2]; p[3] = q[3]; }
> +
> +/*
> +** cpa:
> +** ldr (r[0-9]+), \[r1\] @ unaligned
> +** str \1, \[r0\]
> +** bx lr
> +*/
> +void cpa (u8 *__restrict d, const u8 *__restrict q)
> +{
> + u8 *p = __builtin_assume_aligned (d, 4);
> + p[0] = q[0]; p[1] = q[1]; p[2] = q[2]; p[3] = q[3];
> +}
> +
> +struct ctx { u32 s[2]; };
> +
> +static void st (u8 *p, u32 v)
> +{ p[0] = v; p[1] = v >> 8; p[2] = v >> 16; p[3] = v >> 24; }
> +
> +/* C and OUT may alias, so only store merging can widen these. */
> +
> +/*
> +** w:
> +** ldr (r[0-9]+), \[r0\]
> +** str \1, \[r1\] @ unaligned
> +** ldr (r[0-9]+), \[r0, #4\]
> +** str \2, \[r1, #4\] @ unaligned
> +** bx lr
> +*/
> +void w (struct ctx *c, u8 *out) { st (out, c->s[0]); st (out + 4, c->s[1]); }
> --
> 2.55.0
>
