The 07/27/2026 08:21, H.J. Lu wrote:
> When the size of record or array is smaller than the target alignment,
> don't over align them and set DR_TARGET_ALIGNMENT to the reduced target
> alignment. If the natural alignment isn't lower than the reduced target
> alignment, set base_misaligned to false.
>
Definitely need Richi's input here, but I think changing DR_TARGET_ALIGNMENT
overloads this value which is used for more than the alignment of the object
but also used for this like peeling and versioning and runtime checks, so
changing it to something that's less than
targetm.vectorize.preferred_vector_alignment
could cause problems.
I think your original patch was more correct, i.e. just refuse to change the
alignment in vect_compute_data_ref_alignment and so leave it unaligned.
Was there a specific reason you went with this approach instead?
Also I'm not sure we want to do this unconditionally, as some targets may
genuinely require the overalignment for correctness or ABI related reasons
or performance.
So I don't know if we already have a way to ask a target this. But at least it
feels like this shouldn't be unconditional for targets that have like
-mstrict-align.
> diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
> index 0e0754769ae..64080098b64 100644
> --- a/gcc/tree-vect-data-refs.cc
> +++ b/gcc/tree-vect-data-refs.cc
> @@ -1646,26 +1646,71 @@ vect_compute_data_ref_alignment (vec_info *vinfo,
> dr_vec_info *dr_info,
> {
> unsigned int max_alignment;
> tree base = get_base_for_alignment (drb->base_address, &max_alignment);
> - if (max_alignment < vect_align_c
> - || (loop_vinfo && LOOP_VINFO_EPILOGUE_P (loop_vinfo))
> - || !vect_can_force_dr_alignment_p (base,
> - vect_align_c * BITS_PER_UNIT))
> + bool base_misaligned = true;
> +
> + tree type = TREE_TYPE (base);
> + if (AGGREGATE_TYPE_P (type))
> + {
> + poly_uint64 type_size;
> + if (poly_int_tree_p (TYPE_SIZE_UNIT (type), &type_size)
> + && type_size.is_constant ())
> + {
> + unsigned HOST_WIDE_INT size = type_size.to_constant ();
> + if (size && size < vect_align_c)
> + {
> + /* Don't over align record and array if data size is
> + smaller than the target alignment. */
> +
> + do
> + {
> + vect_align_c /= 2;
> + }
> + while (size < vect_align_c);
> +
> + if (base_alignment >= vect_align_c)
> + {
> + /* Set base_misaligned to false if the natural
> + alignment is higher than the reduced target
> + alignment. */
> + base_misaligned = false;
> + if (dump_enabled_p ())
> + dump_printf_loc
> + (MSG_NOTE, vect_location,
> + "no need to force alignment of ref: %T\n",
> + ref);
> + }
> +
> + /* Set DR_TARGET_ALIGNMENT to the reduced target
> + alignment. */
> + SET_DR_TARGET_ALIGNMENT (dr_info, vect_align_c);
> + }
> + }
> + }
> +
> + if (base_misaligned)
> {
> + if (max_alignment < vect_align_c
> + || (loop_vinfo && LOOP_VINFO_EPILOGUE_P (loop_vinfo))
> + || !vect_can_force_dr_alignment_p
> + (base, vect_align_c * BITS_PER_UNIT))
> + {
> + if (dump_enabled_p ())
> + dump_printf_loc (MSG_NOTE, vect_location,
> + "can't force alignment of ref: %T\n",
> + ref);
> + return;
> + }
> +
> + /* Force the alignment of the decl.
> + NOTE: This is the only change to the code we make during
> + the analysis phase, before deciding to vectorize the loop. */
> if (dump_enabled_p ())
> dump_printf_loc (MSG_NOTE, vect_location,
> - "can't force alignment of ref: %T\n", ref);
> - return;
> + "force alignment of %T\n", ref);
> }
>
> - /* Force the alignment of the decl.
> - NOTE: This is the only change to the code we make during
> - the analysis phase, before deciding to vectorize the loop. */
> - if (dump_enabled_p ())
> - dump_printf_loc (MSG_NOTE, vect_location,
> - "force alignment of %T\n", ref);
> -
> dr_info->base_decl = base;
> - dr_info->base_misaligned = true;
> + dr_info->base_misaligned = base_misaligned;
> base_misalignment = 0;
> }
> poly_int64 misalignment
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 488b9754bed..54d24c75663 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -8298,8 +8298,15 @@ vectorizable_store (vec_info *vinfo,
> && memory_access_type != VMAT_STRIDED_SLP
> && memory_access_type != VMAT_INVARIANT
> && alignment_support_scheme != dr_aligned)
> - dump_printf_loc (MSG_NOTE, vect_location,
> - "Vectorizing an unaligned access.\n");
> + {
> + /* DR_TARGET_ALIGNMENT may be reduced if data size is smaller
> + than the vector alignment. */
> + unsigned int align = TYPE_ALIGN_UNIT (vectype);
> + poly_uint64 target_align = DR_TARGET_ALIGNMENT (dr_info);
> + if (known_ge (target_align, align))
> + dump_printf_loc (MSG_NOTE, vect_location,
> + "Vectorizing an unaligned access.\n");
> + }
This message is referring to whether the access is unaligned vs the
vector alignment, which in this case it is.
So this hunk doesn't seem right because reducing the size of the object
alignment doesn't make the access aligned to the vector load.
Thanks,
Tamar
> }
>
> /* Transform. */
> @@ -9967,8 +9974,16 @@ vectorizable_load (vec_info *vinfo,
> && memory_access_type != VMAT_STRIDED_SLP
> && memory_access_type != VMAT_INVARIANT
> && alignment_support_scheme != dr_aligned)
> - dump_printf_loc (MSG_NOTE, vect_location,
> - "Vectorizing an unaligned access.\n");
> + {
> + /* DR_TARGET_ALIGNMENT may be reduced if data size is smaller
> + than the vector alignment. */
> + dr_vec_info *dr_info = STMT_VINFO_DR_INFO (stmt_info);
> + unsigned int align = TYPE_ALIGN_UNIT (vectype);
> + poly_uint64 target_align = DR_TARGET_ALIGNMENT (dr_info);
> + if (known_ge (target_align, align))
> + dump_printf_loc (MSG_NOTE, vect_location,
> + "Vectorizing an unaligned access.\n");
> + }
>
> if (memory_access_type == VMAT_LOAD_STORE_LANES)
> vinfo->any_known_not_updated_vssa = true;
> --
> 2.55.0
>
--
