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?

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?

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.

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?

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

Reply via email to