On Thu, 30 Jan 2025, Jakub Jelinek wrote:
> 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.
Pushed with formatting as you suggested.
Richard.
