On Thu, Jan 30, 2025 at 11:34:16AM +0100, Richard Biener wrote:
> When MEM_REF expansion of a non-MEM falls back to a stack temporary
> we fail to handle the case where the offset adjusted reference to
> the temporary is not aligned according to the requirement of the
> mode. We have to go through bitfield extraction or movmisalign
> in this case. Fortunately there's a helper for this.
>
> This fixes an ICE observed on arm which has sanity checks in its
> move patterns for this.
>
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>
> OK?
>
> PR middle-end/118695
> * expr.cc (expand_expr_real_1): When expanding a MEM_REF
> to a non-MEM by committing it to a stack temporary make
> sure to handle misaligned accesses correctly.
>
> * gcc.dg/pr118695.c: New testcase.
> ---
> gcc/expr.cc | 11 +++++++++++
> gcc/testsuite/gcc.dg/pr118695.c | 9 +++++++++
> 2 files changed, 20 insertions(+)
> create mode 100644 gcc/testsuite/gcc.dg/pr118695.c
>
> diff --git a/gcc/expr.cc b/gcc/expr.cc
> index 10467f82c0d..0136678a40b 100644
> --- a/gcc/expr.cc
> +++ b/gcc/expr.cc
> @@ -11796,6 +11796,7 @@ expand_expr_real_1 (tree exp, rtx target,
> machine_mode tmode,
> && known_eq (GET_MODE_BITSIZE (DECL_MODE (base)), type_size))
> return expand_expr (build1 (VIEW_CONVERT_EXPR, type, base),
> target, tmode, modifier);
> + unsigned align;
> if (TYPE_MODE (type) == BLKmode || maybe_lt (offset, 0))
> {
> temp = assign_stack_temp (DECL_MODE (base),
> @@ -11804,6 +11805,16 @@ expand_expr_real_1 (tree exp, rtx target,
> machine_mode tmode,
> temp = adjust_address (temp, TYPE_MODE (type), offset);
> if (TYPE_MODE (type) == BLKmode)
> set_mem_size (temp, int_size_in_bytes (type));
> + /* When the original ref was misaligned so will be the
> + access to the stack temporary. Not all targets handle
> + this correctly, some will ICE in sanity checking.
> + Handle this by doing bitfield extraction when necessary. */
> + else if ((align = get_object_alignment (exp))
> + < GET_MODE_ALIGNMENT (TYPE_MODE (type)))
> + temp = expand_misaligned_mem_ref
> + (temp, TYPE_MODE (type), unsignedp, align,
> + modifier == EXPAND_STACK_PARM ? NULL_RTX : target,
> + NULL);
I don't like very much this kind of formatting (function name on one line,
open paren on another). Of course there are cases where there is no other
choice,
but in this case
temp
= expand_misaligned_mem_ref (temp, TYPE_MODE (type),
unsignedp, align,
modifier == EXPAND_STACK_PARM
? NULL_RTX : target, NULL);
doesn't look too bad.
Ok for trunk either way.
Jakub
