On Thu, Sep 11, 2025 at 6:06 PM Robin Dapp <rdapp....@gmail.com> wrote:
>
> > Hmm, so the existing "punning" code for VMAT_STRIDED_SLP does
> >
> > tree vtype
> > = vector_vector_composition_type (vectype, const_nunits / n,
> > &ptype);
> > if (vtype != NULL_TREE)
> > {
> > dr_alignment_support dr_align;
> > int mis_align = 0;
> > if (VECTOR_TYPE_P (ptype))
> > {
> > mis_align = dr_misalignment (first_dr_info, ptype);
> > if (maybe_gt (vf, 1u)
> > && !multiple_p (DR_STEP_ALIGNMENT
> > (first_dr_info->dr),
> > DR_TARGET_ALIGNMENT
> > (first_dr_info)))
> > mis_align = -1;
> > dr_align
> > = vect_supportable_dr_alignment (vinfo, dr_info,
> > ptype,
> > mis_align);
> >
> > so the change would pessimize the handling in this case unnecessarily?
> > Isn't the issue that we now pass in the "wrong" element type (already for
> > above, but I guess it's unused when !is_gather_scatter?)? I think
> > clarifying the hook documentation would be good. In particular the
> > 'misalignment' is in bytes, and IIRC is_packed specifies whether the
> > vector load is element aligned. That's possibly from times where we
> > had misalignment specified in elements, not bytes, so it somehow
> > feels redundant (likewise passing an element type here at all).
> >
> > I think we want to handle this in a separate patch, it requires review of
> > the existing implementations of the hook.
>
> Ack for the separate patch but I guess I'm confused:
>
> So misalignment in bytes, this can and should be documented of course.
>
> Regarding the type argument: Power checks if the type's size is 32/64 so
> even though the misalignment is unknown, we still know that we access a 32/64
> element which is apparently fine, but only when the access is not packed?
IIRC power has naturally aligned 'double' but in a struct it's aligned
to 4 bytes.
See rs6000_vector_alignment_reachable.
Or something like that. I think the intent - when 'misalignment' was in
elements, was to distinguish "unknown" from element aligned. So
is_packed should probably be true if the vector access isn't aligned
according to element size (or element mode alignment?). But as said,
this all is somewhat redundant - and even inconsistent as in how we
pass vector modes with element modes not equal to the types mode
we pass ...
Also misalignment is relative to target_alignment of the access, not
necessarily relative to the passed vector mode alignment.
> gcn checks
> return misalignment % TYPE_ALIGN_UNIT (type) == 0;
> but that looks pretty redundant (the comment above it already notes that).
> Also looks like this is copied from arm.
>
> arm additionally allows a packed access if TYPE_ALIGN_UNIT (type) == 1.
> In that case we wouldn't have passed is_packed = true anyway?
No idea ;)
> aarch64 just passes type to the default implementation.
>
> So apart from the "packed but still at least a 32bit access" use case, type
> seems redundant.
And wrong when we pun the vector element types.
> If we leave the is_packed semantics as is wouldn't the targets have to check
> whether MODE's alignment fit TYPE's themselves? Or should we use misalignment
> for the known mismatch between element size and vector unit size? Doesn't
> seem
> reasonable either. If is_packed specifies whether the vector load is
> element
> aligned that would fit the current patch?
To me it seems we can elide both 'type' and 'is_packed', the vector
mode specifies
the element mode and size, misalignment can be used to compute whether the
access is element aligned or not.
I wonder in which cases we have misalignment == -1 but !is_packed?
> As we never set is_packed in the STRIDED_SLP case I'm pretty sure the current
> riscv hook implementation would, wrongly, return true/supported. Luckily we
> never implemented vec_init for vectors and scalar-to-vector move costs are too
> high to ever run into that problem ;)
>
> --
> Regards
> Robin
>
