On 2/11/26 20:56, Robin Dapp wrote:
Hi Victor,
- if ((vf.is_constant () && pow2p_hwi (new_alignment.to_constant ()))
- || (!vf.is_constant () && pow2p_hwi (align_factor_c)))
+ if ((vf.is_constant () && !pow2p_hwi (new_alignment.to_constant ()))
+ || (!vf.is_constant () && !pow2p_hwi (align_factor_c)))
+ new_alignment
+ = HOST_WIDE_INT_1U << ceil_log2 (new_alignment.to_constant ());
Is this safe when !vf.is_constant ()? I.e. with new_alignment = vf * ... won't
to_constant () fail?
- if (*memory_access_type == VMAT_ELEMENTWISE
- || *memory_access_type == VMAT_GATHER_SCATTER_LEGACY
+ if (*memory_access_type == VMAT_ELEMENTWISE)
+ {
+ if (dr_safe_speculative_read_required (stmt_info)
+ && !STMT_VINFO_GATHER_SCATTER_P (stmt_info)
+ && !DR_SCALAR_KNOWN_BOUNDS (STMT_VINFO_DR_INFO (stmt_info)))
+ *alignment_support_scheme = dr_unaligned_unsupported;
+ else
+ *alignment_support_scheme = dr_unaligned_supported;
+ *misalignment = DR_MISALIGNMENT_UNKNOWN;
+ }
+
+ else if (*memory_access_type == VMAT_GATHER_SCATTER_LEGACY
|| *memory_access_type == VMAT_STRIDED_SLP
|| *memory_access_type == VMAT_INVARIANT)
{
Not being fully acquainted with the early-break flow, I'm not exactly sure
what's happening here. Won't setting the alignment to dr_unaligned_unsupported
just going to cause a "return false" soon after? If so, can't we fold the
check into the early-break checks we already have (around line 2564), thus
making it slightly more explicit?
Thanks for the sanity check. Unfortunately, due to a fundamental
misunderstanding on my part re. the classification of
dr_unaligned_unsupported
c.f. dr_unaligned_supported I apply some bad logic here.
This patch should have more helpfully have been classified as an RFC
to be honest. The eagerness to get the bug fixed asap seems to have
clouded my judgement - We don't wish to reject some of these cases
outright, as is being done here, rather first attempting to peel for
alignment.
I'll go back to the drawing board and add a test that ensures that we do
peel for alignment when this fixes the alignment issue in the context of
VMAT_ELEMENTWISE.
Thanks.
BTW not related to your patch at all but I noticed that we have
/* Scatter-gather and invariant accesses continue to address individual
scalars, so vector-level alignment is irrelevant. */
if (STMT_VINFO_GATHER_SCATTER_P (stmt_info)
|| integer_zerop (DR_STEP (dr_info->dr)))
return false;
in vect_relevant_for_alignment_p and that's not universally true anymore :/
