пн, 14 сент. 2026 г. в 15:02, Richard Biener <[email protected]>: > > On Mon, 14 Sep 2026, Oleg Tolmatcev wrote: > > > Targets that cannot realign the stack cap their stack alignment. Win64 SEH > > caps MAX_STACK_ALIGNMENT at 128 bits; aarch64 does not define > > MAX_STACK_ALIGNMENT at all and is capped at STACK_BOUNDARY, also 128. > > assign_stack_local silently ignored any alignment beyond that cap. > > > > A 256-bit vector requires 32-byte alignment, its slot gets the capped 16 > > bytes, and MEM_ALIGN is set to 32 anyway. x86 selects vmovaps from > > MEM_ALIGN and faults on the under-aligned slot (PR54412, > > MSYS2/MINGW-packages#1209). Even with MEM_ALIGN consistent, an escaped > > pointer carries only the alignment of the type, and the callee uses an > > aligned access on the under-aligned object. aarch64 does not crash only > > because its loads and stores do not fault on a misaligned address. > > > > Make assign_stack_local_1 honour an explicit alignment beyond the cap by > > overallocating the slot and aligning an address inside it at run time, as > > assign_parm_setup_block already did for BLKmode parameters and > > expand_stack_vars does for locals. Such a slot has no fixed frame offset, > > so > > assign_stack_temp_for_type keeps an over-aligned temporary out of the temp > > slot list, and caps the merely preferred alignment of other temporaries so > > they can still be reused. > > That is because we cannot release the full slot (I don't remember > of we have the "free" corresponding to assign_stack_local)? The > oversized whole block should still be aligned to > MAX_SUPPORTED_STACK_ALIGNMENT, no?
The problem with reuse is demonstrated by pr54412-highpart-extract-rt.C: if ((unsigned) index < 4) return Vec4q (z0).extract (index); return Vec4q (z1).extract (index - 4); GCC emits code in the first branch to compute the dynamically aligned address of the stack slot used by Vec4q (z0) and stores that address in a pseudo-register. It then reuses the slot and the pseudo-register for Vec4q (z1) in the second branch without recomputing the aligned address. The pseudo-register is undefined along the second path because its defining instructions are in the first branch, and the code faults. I will fix it in the third version of the patch, following your suggestion below. The whole block will still be allocated with an alignment of MAX_SUPPORTED_STACK_ALIGNMENT; only the address of the over-aligned object will receive greater alignment, by padding the block and aligning the address within it. > > Callers that can do with less pass ASLK_REDUCE_ALIGN: LRA spill slots, which > > are allocated after register allocation, and the sjlj function context, > > whose > > alignment is a deliberate overestimate. > > > > Regtested on x86_64-pc-linux-gnu and, under QEMU, on > > aarch64-unknown-linux-gnu, > > with no new failures. > > > > gcc/ChangeLog: > > > > PR target/54412 > > * except.cc (sjlj_build_landing_pads): Pass ASLK_REDUCE_ALIGN. > > * function.cc (assign_stack_local_1): Honour a positive ALIGN beyond > > MAX_SUPPORTED_STACK_ALIGNMENT unless KIND has ASLK_REDUCE_ALIGN, by > > overallocating the slot and aligning its address at run time. > > (assign_stack_temp_for_type): Allocate a temporary whose required > > alignment exceeds MAX_SUPPORTED_STACK_ALIGNMENT without a temp_slot > > entry. Cap the preferred alignment of other temporaries at > > MAX_SUPPORTED_STACK_ALIGNMENT and adjust the BLKmode assert. > > (assign_parm_setup_block): Let assign_stack_local align the slot. > > * lra-spills.cc (assign_mem_slot): Pass ASLK_REDUCE_ALIGN. > > > > Signed-off-by: Oleg Tolmatcev <[email protected]> > > --- > > gcc/except.cc | 12 ++++----- > > gcc/function.cc | 68 +++++++++++++++++++++++++++++++++-------------- > > gcc/lra-spills.cc | 5 ++-- > > 3 files changed, 57 insertions(+), 28 deletions(-) > > > > diff --git a/gcc/except.cc b/gcc/except.cc > > index 98876833ee..e3b8645733 100644 > > --- a/gcc/except.cc > > +++ b/gcc/except.cc > > @@ -1478,9 +1478,9 @@ sjlj_build_landing_pads (void) > > TYPE_MODE (sjlj_fc_type_node), > > TYPE_ALIGN (sjlj_fc_type_node)); > > crtl->eh.sjlj_fc > > - = assign_stack_local (TYPE_MODE (sjlj_fc_type_node), > > - int_size_in_bytes (sjlj_fc_type_node), > > - align); > > + = assign_stack_local_1 (TYPE_MODE (sjlj_fc_type_node), > > + int_size_in_bytes (sjlj_fc_type_node), > > + align, ASLK_RECORD_PAD | ASLK_REDUCE_ALIGN); > > > > sjlj_mark_call_sites (); > > sjlj_emit_function_enter (dispatch_label); > > @@ -1496,9 +1496,9 @@ sjlj_build_landing_pads (void) > > TYPE_MODE (sjlj_fc_type_node), > > TYPE_ALIGN (sjlj_fc_type_node)); > > crtl->eh.sjlj_fc > > - = assign_stack_local (TYPE_MODE (sjlj_fc_type_node), > > - int_size_in_bytes (sjlj_fc_type_node), > > - align); > > + = assign_stack_local_1 (TYPE_MODE (sjlj_fc_type_node), > > + int_size_in_bytes (sjlj_fc_type_node), > > + align, ASLK_RECORD_PAD | ASLK_REDUCE_ALIGN); > > > > sjlj_mark_call_sites (); > > sjlj_emit_function_enter (NULL); > > diff --git a/gcc/function.cc b/gcc/function.cc > > index 9358787b26..fbe341b96e 100644 > > --- a/gcc/function.cc > > +++ b/gcc/function.cc > > @@ -370,6 +370,10 @@ add_frame_space (poly_int64 start, poly_int64 end) > > -2 means use BITS_PER_UNIT, > > positive specifies alignment boundary in bits. > > > > + A positive ALIGN that exceeds MAX_SUPPORTED_STACK_ALIGNMENT is honoured > > + by overallocating the slot and aligning its address at run time, with > > + insns emitted at the current position, unless KIND has > > ASLK_REDUCE_ALIGN. > > + > > KIND has ASLK_REDUCE_ALIGN bit set if it is OK to reduce > > alignment and ASLK_RECORD_PAD bit set if we should remember > > extra space we allocated for alignment purposes. When we are > > @@ -385,7 +389,7 @@ assign_stack_local_1 (machine_mode mode, poly_int64 > > size, > > rtx x, addr; > > poly_int64 bigend_correction = 0; > > poly_int64 slot_offset = 0, old_frame_offset; > > - unsigned int alignment, alignment_in_bits; > > + unsigned int alignment, alignment_in_bits, dynamic_align = 0; > > > > if (align == 0) > > { > > @@ -404,9 +408,19 @@ assign_stack_local_1 (machine_mode mode, poly_int64 > > size, > > > > alignment_in_bits = alignment * BITS_PER_UNIT; > > > > - /* Ignore alignment if it exceeds MAX_SUPPORTED_STACK_ALIGNMENT. */ > > + /* The frame cannot be aligned beyond MAX_SUPPORTED_STACK_ALIGNMENT. If > > more > > + is requested, pad the slot so that an over-aligned object still fits. > > */ > > if (alignment_in_bits > MAX_SUPPORTED_STACK_ALIGNMENT) > > { > > + if (align > 0 && !(kind & ASLK_REDUCE_ALIGN) && maybe_ne (size, 0)) > > can you elaborate on maybe_ne (size, 0)? I am surprised we get here > for a known_eq (size, 0) maybe_ne (size, 0) is purely defensive. I do not know of a caller that requests an alignment beyond the cap when known_eq (size, 0). Without the check, we would add alignment padding to a known-zero size, make it nonzero, and reserve stack space where none was previously reserved. > > + { > > + gcc_assert (can_create_pseudo_p ()); > > Can we try asserting currently_expanding_to_rtl instead? Yes, I will do it in the third version of the patch. > And when called later we still get a wrong MEM_ALIGN downstream? > Given we do a > > set_mem_align (x, alignment_in_bits); > > here, so currently with the capped MAX_SUPPORTED_STACK_ALIGNMENT value, > maybe we should refuse to override this with a larger value later. > We could mark the MEM with an additional flag, indicating a larger > alignment cannot be guaranteed.Yes, that is concise and technically correct. > I would only make “required” slightly more precise: With this patch, `MEM_ALIGN` is no longer wrong. If an alignment beyond the cap is explicitly required, `assign_stack_local_1` aligns the address at run time and sets `MEM_ALIGN` to the requested alignment. `ASLK_REDUCE_ALIGN` is used only where reduced alignment is acceptable and `MEM_ALIGN` is not subsequently increased, so I do not think an additional flag is necessary. > > + dynamic_align = alignment_in_bits; > > + /* Pad from the alignment the frame base is guaranteed to have. */ > > + size += ((alignment_in_bits > > + - REGNO_POINTER_ALIGN (VIRTUAL_STACK_VARS_REGNUM)) > > + / BITS_PER_UNIT); > > + } > > alignment_in_bits = MAX_SUPPORTED_STACK_ALIGNMENT; > > alignment = MAX_SUPPORTED_STACK_ALIGNMENT / BITS_PER_UNIT; > > } > > @@ -506,7 +520,7 @@ assign_stack_local_1 (machine_mode mode, poly_int64 > > size, > > found_space: > > /* On a big-endian machine, if we are allocating more space than we will > > use, > > use the least significant bytes of those that are allocated. */ > > - if (mode != BLKmode) > > + if (mode != BLKmode && !dynamic_align) > > { > > /* The slot size can sometimes be smaller than the mode size; > > e.g. the rs6000 port allocates slots with a vector mode > > @@ -531,6 +545,13 @@ assign_stack_local_1 (machine_mode mode, poly_int64 > > size, > > (slot_offset + bigend_correction, > > Pmode)); > > > > + if (dynamic_align) > > + { > > + addr = align_dynamic_address (addr, dynamic_align); > > + mark_reg_pointer (addr, dynamic_align); > > + alignment_in_bits = dynamic_align; > > + } > > + > > x = gen_rtx_MEM (mode, addr); > > set_mem_align (x, alignment_in_bits); > > MEM_NOTRAP_P (x) = 1; > > @@ -801,7 +822,24 @@ assign_stack_temp_for_type (machine_mode mode, > > poly_int64 size, tree type) > > > > gcc_assert (known_size_p (size)); > > > > - align = get_stack_local_alignment (type, mode); > > + unsigned int required_align > > + = (mode == BLKmode ? BITS_PER_UNIT : GET_MODE_ALIGNMENT (mode)); > > + if (type) > > + required_align = MAX (required_align, TYPE_ALIGN (type)); > > + > > + /* Alignment beyond what the frame can guarantee needs an address aligned > > + at run time, so such a temporary is never reused or freed. */ > > So I think this means we should avoid creating such over-aligned > stack temps. At least code expects we can free/reuse them and > as explained I see no reason we couldn't do that? But yes, > assign_stack_local_1 does not get us the pointer to the > allocated block and size - but it could, as alternate output? > Or maybe the use of assign_stack_local_1 below is not really > the best API as seen by the comment before the call. I will implement your suggestion in v3 by adding assign_stack_local_2, which also returns the address before run-time alignment. The temporary-slot code can store that address and use it to recompute the dynamically aligned address whenever the slot is reused. > > + if (required_align > MAX_SUPPORTED_STACK_ALIGNMENT) > > + { > > + slot = assign_stack_local_1 (mode, size, required_align, 0); > > + align = MEM_ALIGN (slot); > > + goto finalize_slot; > > + } > > + > > + /* Cap the preferred alignment at MAX_SUPPORTED_STACK_ALIGNMENT so that > > the > > + slot can be reused like any other temporary. */ > > + align = MIN (get_stack_local_alignment (type, mode), > > + MAX_SUPPORTED_STACK_ALIGNMENT); > > > > /* Try to find an available, already-allocated temporary of the proper > > mode which meets the size and alignment requirements. Choose the > > @@ -883,7 +921,9 @@ assign_stack_temp_for_type (machine_mode mode, > > poly_int64 size, tree type) > > So for requests which depended on the rounding of SIZE, we go ahead > > and round it now. We also make sure ALIGNMENT is at least > > BIGGEST_ALIGNMENT. */ > > - gcc_assert (mode != BLKmode || align == BIGGEST_ALIGNMENT); > > + gcc_assert (mode != BLKmode > > + || align == MIN (BIGGEST_ALIGNMENT, > > + MAX_SUPPORTED_STACK_ALIGNMENT)); > > p->slot = assign_stack_local_1 (mode, > > (mode == BLKmode > > ? aligned_upper_bound (size, > > @@ -935,6 +975,7 @@ assign_stack_temp_for_type (machine_mode mode, > > poly_int64 size, tree type) > > slot = gen_rtx_MEM (mode, XEXP (p->slot, 0)); > > vec_safe_push (stack_slot_list, slot); > > > > + finalize_slot: > > /* If we know the alias set for the memory that will be used, use > > it. If there's no TYPE, then we don't know anything about the > > alias set for the memory. */ > > @@ -2948,21 +2989,8 @@ assign_parm_setup_block (struct assign_parm_data_all > > *all, > > ? MAX (DECL_ALIGN (parm), BITS_PER_WORD) : DECL_ALIGN (parm)); > > > > SET_DECL_ALIGN (parm, parm_align); > > - if (DECL_ALIGN (parm) > MAX_SUPPORTED_STACK_ALIGNMENT) > > - { > > - rtx allocsize = gen_int_mode (size_stored, Pmode); > > - get_dynamic_stack_size (&allocsize, 0, DECL_ALIGN (parm), NULL); > > This has the side-effect of eventually doing > > /* We can't attempt to minimize alignment necessary, because we don't > know the final value of preferred_stack_boundary yet while executing > this code. */ > if (crtl->preferred_stack_boundary < PREFERRED_STACK_BOUNDARY) > crtl->preferred_stack_boundary = PREFERRED_STACK_BOUNDARY; > > we might want to preserve that? > I will preserve this behavior in v3. Oleg > I'll add as general disclaimer that while I touched some of the > alignment code before I have nowhere from a complete picture of > this, so I welcome input from others. > > Overall I think not simply ignoring an alignment request is the > correct thing to do. > > So thanks for keeping to work on this, > Richard. > > > - stack_parm = assign_stack_local (BLKmode, UINTVAL (allocsize), > > - MAX_SUPPORTED_STACK_ALIGNMENT); > > - rtx addr = align_dynamic_address (XEXP (stack_parm, 0), > > - DECL_ALIGN (parm)); > > - mark_reg_pointer (addr, DECL_ALIGN (parm)); > > - stack_parm = gen_rtx_MEM (GET_MODE (stack_parm), addr); > > - MEM_NOTRAP_P (stack_parm) = 1; > > - } > > - else > > - stack_parm = assign_stack_local (BLKmode, size_stored, > > - DECL_ALIGN (parm)); > > + stack_parm = assign_stack_local (BLKmode, size_stored, > > + DECL_ALIGN (parm)); > > if (known_eq (GET_MODE_SIZE (GET_MODE (entry_parm)), size)) > > PUT_MODE (stack_parm, GET_MODE (entry_parm)); > > set_mem_attributes (stack_parm, parm, 1); > > diff --git a/gcc/lra-spills.cc b/gcc/lra-spills.cc > > index 07801e39d5..f55839f50b 100644 > > --- a/gcc/lra-spills.cc > > +++ b/gcc/lra-spills.cc > > @@ -146,8 +146,9 @@ assign_mem_slot (int i) > > x = slots[slot_num].mem; > > if (!x) > > { > > - x = assign_stack_local (BLKmode, slots[slot_num].size, > > - slots[slot_num].align); > > + x = assign_stack_local_1 (BLKmode, slots[slot_num].size, > > + slots[slot_num].align, > > + ASLK_RECORD_PAD | ASLK_REDUCE_ALIGN); > > slots[slot_num].mem = x; > > } > > > > > > -- > Richard Biener <[email protected]> > SUSE Software Solutions Germany GmbH, > Frankenstrasse 146, 90461 Nuernberg, Germany; > GF: Stefan Gaiser, Jochen Jaser, Abhinav Puri; (HRB 36809, AG Nuernberg)
