On 3/3/26 14:31, Robin Dapp wrote:
Hi Victor,
- if (*memory_access_type == VMAT_ELEMENTWISE
- || *memory_access_type == VMAT_GATHER_SCATTER_LEGACY
- || *memory_access_type == VMAT_STRIDED_SLP
- || *memory_access_type == VMAT_INVARIANT)
+ /* Even though individual VMAT_ELEMENTWISE accesses do not cause alignment
+ problems, loading the whole vector's worth of values in a speculative
+ early-break context might cross a page boundary. Set the alignment scheme
+ to `dr_aligned' here in order to force checking of whether such accesses
+ meet alignment criteria. */
+ if (dr_safe_speculative_read_required (stmt_info)
+ && !DR_SCALAR_KNOWN_BOUNDS (STMT_VINFO_DR_INFO (stmt_info))
+ && *memory_access_type != VMAT_INVARIANT)
+ {
+ if (mat_gather_scatter_p (*memory_access_type)
+ && !first_dr_info)
+ *misalignment = DR_MISALIGNMENT_UNKNOWN;
+ else
+ *misalignment = dr_misalignment (first_dr_info, vectype, *poffset);
+
+ *alignment_support_scheme
+ = vect_supportable_dr_alignment
+ (vinfo, first_dr_info, vectype, *misalignment,
+ mat_gather_scatter_p (*memory_access_type));
+
+ if (*alignment_support_scheme != dr_unaligned_unsupported)
+ *alignment_support_scheme = dr_aligned;
+ }
+ else if (*memory_access_type == VMAT_ELEMENTWISE
+ ||*memory_access_type == VMAT_GATHER_SCATTER_LEGACY
+ || *memory_access_type == VMAT_STRIDED_SLP
+ || *memory_access_type == VMAT_INVARIANT)
{
*alignment_support_scheme = dr_unaligned_supported;
*misalignment = DR_MISALIGNMENT_UNKNOWN;
I'm afraid the new version version isn't much clearer and I'll take blame for
that. What I'd go for is the hunk from v2 with the comment added and
STMT_VINFO_GATHER_SCATTER removed (or did I miss something and we do need it?).
The intent of my last comment was a separation of concerns:
The existing if-else takes care of alignment settings for individual element
misalignment and now we're adding "block misalignment" to the mix.
IMHO, generally, block misalignment matters for every memory access type but
we don't support most of them for early break anyway. Given that, maybe it
doesn't really help code clarity to make this separation explicit.
What might help slightly is leaving the if-else untouched and just adding the
v2 hunk after it, as "second pass for block-level alignment". But that's very
much bike-shedding at this point.
For your proposed second pass for block-level alignment, and
considering that this is important particularly for early break, I
think we could quite easily absorb the logic from the V2 hunk into the
following pre-existing section of code, making the intent of the new
code quite clear:
/* Check if we support the operation if early breaks are needed. Here we
must ensure that we don't access any more than the scalar code would
have. A masked operation would ensure this, so for these load types
force masking. */
if (loop_vinfo
&& dr_safe_speculative_read_required (stmt_info)
&& LOOP_VINFO_EARLY_BREAKS (loop_vinfo)
...
We'd keep the existing logic contained in the appropriate
if (mat_gather_scatter_p (*memory_access_type)
|| *memory_access_type == VMAT_STRIDED_SLP)
block and add the new `*alignment_support_scheme = dr_unaligned_supported'
to an `else if (*memory_access_type == VMAT_ELEMENTWISE && !inbounds)'
block.
So we'd go from
if (loop_vinfo
&& dr_safe_speculative_read_required (stmt_info)
&& LOOP_VINFO_EARLY_BREAKS (loop_vinfo)
&& (mat_gather_scatter_p (*memory_access_type)
|| *memory_access_type == VMAT_STRIDED_SLP))
{
<existing logic>
}
to something like:
if (loop_vinfo
&& dr_safe_speculative_read_required (stmt_info)
&& LOOP_VINFO_EARLY_BREAKS (loop_vinfo))
{
if (mat_gather_scatter_p (*memory_access_type)
|| *memory_access_type == VMAT_STRIDED_SLP)
{
<existing logic>
}
else if (*memory_access_type == VMAT_ELEMENTWISE && !inbounds)
*alignment_support_scheme = dr_aligned;
}
Any thoughts?
Cheers
