Re: RFC: Allow "(mem:<vectype> (reg:<vectype>))"

On Thu, Jun 25, 2026 at 12:20 PM Andrew Stubbs <[email protected]> wrote:
>
> On 25/06/2026 10:05, Richard Biener wrote:
> > On Thu, Jun 25, 2026 at 10:52 AM Andrew Stubbs <[email protected]> wrote:
> >>
> >> On 23/06/2026 13:28, Michael Matz wrote:
> >>> Hello,
> >>>
> >>> On Mon, 22 Jun 2026, Andrew Stubbs wrote:
> >>>
> >>>>> TARGET_POINTER_MODE and TARGET_ADDR_SPACE_POINTER mode only return one mode,
> >>>>> but now we have multiple valid modes: DImode, V2DImode, V4DImode, etc. They
> >>>>> also return scalar_int_mode so I can't even invent a new address space for
> >>>>> the vector pointers.
> >>>>>
> >>>>> Same for TARGET_ADDR_SPACE_ADDRESS_MODE.
> >>>>>
> >>>>> TARGET_VALID_POINTER_MODE and TARGET_ADDR_SPACE_VALID_POINTER_MODE only
> >>>>> accept scalar_int_mode, so automatically exclude vector pointers.
> >>>>>
> >>>>> These can be patched to use plain machine_mode, perhaps, or keep them all
> >>>>> scalar and add a new TARGET_ADDR_SPACE_ALLOWS_VECTOR_POINTERS to fix the
> >>>>> problem at the call sites, which might make more sense?
> >>>>
> >>>> I've been working on this proposal some more. I've solved the above problem by
> >>>> keeping all the hooks returning pointer/address types scalar, but changing
> >>>> TARGET_ADDR_SPACE_VALID_POINTER_MODE to accept vector types, so the backend
> >>>> can approve them. When the target independent code encounters a vector of
> >>>> addresses it checks that the inner mode matches the appropriate hook values.
> >>>>
> >>>> I was hoping to post a patch series this week, but I've hit against another
> >>>> issue.....
> >>>>
> >>>> I have an insn that looks like this:
> >>>>
> >>>>     (set (reg:V64SI 123)
> >>>>          (vec_merge:V64SI
> >>>>             (mem:V64SI (reg:V64DI 456))
> >>>>             (reg:V64SI 123)
> >>>>             (reg:DI 789)))
> >>>
> >>> How would anything in RTL-land see that this load is masked?  There's no
> >>> info anywhere that makes it differ from
> >>>
> >>>      (set (reg:V64SI 123)
> >>>           (vec_merge:V64SI
> >>>              (mem:V64SI (reg:V64DI 456))
> >>>              (reg:V64SI 123)
> >>>              (reg:DI 789)))
> >>>
> >>> with me saying that this is a vector merge operation with a memory operand
> >>> that is loaded completely.  It's a verbatim copy of your RTL pattern.  See
> >>> my point? :)
> >>
> >> I'm pretty sure you just restated my point. ;-)
> >>
> >>>> I can workaround the "reload" in the backend by using unspecs (exactly
> >>>> what I was trying to avoid), but does anyone have a better suggestion?
> >>>
> >>> (a) unspec
> >>> (b) teaching RTL-land generally that vec_merge(...(mem)) is "special" and
> >>>       its operands cannot just be lifted out (nah)
> >>> (c) biting the bullet and introduce and set a flag on (mem)s that it is
> >>>       "special"/partial
> >>> (c') similar flag on the vec_merge
> >>> (d) biting a different bullet and introduce a new top-level RTL expression
> >>>       (partial_mem:mode (addr) (mask))  (with either target-defined methods
> >>>       to specify content of unselected bits/bytes, or with a third operand
> >>>       to specify them (which then makes this just a different vec_merge)
> >>>
> >>> I think (d) is most elegant and most involved, (c) or (c') are the "best"
> >>> on a cost/benefit basis, (b) the most hacky, but may be fine if done via a
> >>> target hook and (a) the easiest but most unelegant.  (a) has subcases:
> >>> where to put the unspec: around the (mem), around the (vec_merge), around
> >>> the whole (set).  IMHO, around the vec_merge makes "most sense", whatever
> >>> that means for unspecs, but, well, still unspecs :-/
> >>
> >> I've been working to implement an unspec solution in the backend, for
> >> now, so that I can move on with this and get the actually working bits
> >> proposed.
> >>
> >> Putting an unspec around the whole pattern doesn't help. Nor does
> >> replacing the vec_merge with an unspec. In both cases the register
> >> allocator happily tries to break out the MEM, just as before.
> >>
> >> I tried creating an unspec that matches a custom memory constraint (the
> >> idea being that it will drop into the existing vec_merge patterns), but
> >> the register allocator now breaks out the unspec into another step.  If
> >> I use a non-memory-specific constraint then the same thing happens.  If
> >> I place the mask inside the unspec and have a specific instruction
> >> handle that, then it works, but now I have a masked load feeding into a
> >> masked move for no good reason.
> >>
> >> Here's what it looks like now:
> >>
> >>       (set (reg:V64SI 123)
> >>            (vec_merge:V64SI
> >>               (mem:V64SI (unspec:V64DI [(reg:V64DI 456) (reg:DI 789)]
> >>                                       UNSPEC_PARTIAL_ADDRVEC))
> >>               (reg:V64SI 123)
> >>               (reg:DI 789)))
> >>
> >> Note that the mask register, r789, now also appears within the MEM. When
> >> reload tries to break it out, gen_movv64si sees the new unspec and
> >> expands to a masked load.  The new instruction is basically identical to
> >> the one that it came from, but reload seems happy now, and my program
> >> runs correctly without the memory fault.
> >
> > x86 gets away with
> >
> > (define_insn "*<avx512>_load<mode>_mask"
> >    [(set (match_operand:V48_AVX512VL 0 "register_operand" "=v")
> >      (vec_merge:V48_AVX512VL
> >        (unspec:V48_AVX512VL
> >          [(match_operand:V48_AVX512VL 1 "memory_operand" "m")]
> >          UNSPEC_MASKLOAD)
> >        (match_operand:V48_AVX512VL 2 "nonimm_or_0_operand" "0C")
> >        (match_operand:<avx512fmaskmode> 3 "register_operand" "Yk")))]
> >
> > so the (unspec ...) is wrapped around the MEM.
>
> I tried that; it just tried to reload the entire unspec.

Ah...  aarch64 has

(define_insn "*aarch64_maskload<mode><vpred>"
  [(set (match_operand:SVE_ALL 0 "register_operand" "=w")
    (unspec:SVE_ALL
      [(match_operand:<VPRED> 2 "register_operand" "Upl")
       (match_operand:SVE_ALL 1 "memory_operand" "m")
       (match_operand:SVE_ALL 3 "aarch64_maskload_else_operand")]
      UNSPEC_LD1_SVE))]
  "TARGET_SVE"
  "ld1<Vesize>\t%0.<Vctype>, %2/z, %1"
  [(set_attr "sve_type" "sve_load_1reg")]
)

so they wrap the whole SET_RHS.

I think we should document some canonical way to represent the
masking side-effect given there's no native RTL construct.

> Either way
> around would work, for loads, but I needed to record the mask in order
> to reassemble the load safely.
>
> For stores, I have not observed the same reloads breaking things, but as
> Michael said there also isn't anything in the pattern to say that the
> set can't be separated from the vec_merge, so I wanted to do the same
> fixup there too. In that case, I believe unspec is not a valid set
> destination (or at least, I've failed to get that working in the past),
> so putting the unspec inside the MEM was convenient. I've not done a
> full testrun yet, but it seems to work, so far.
>
> >> Obviously, I had to handle the new unspec when validating and printing
> >> the MEM, but that has been fairly painless.
> >>
> >> BTW, the root cause of the problem transformation here seems to be that
> >> my load patterns must have early-clobber on the destination (to handle
> >> the unlikely case that only some of the lanes encounter a page fault and
> >> the instruction needs to be restarted for all of them), and if it so
> >> happens that the compiler wants to allocate the same register to both
> >> input and output then LRA attempts to honor the attribute by pulling out
> >> the MEM.  (I'm a little unclear how that ultimately helps, given that
> >> the new load has the exact same constraints as the original, but somehow
> >> it comes right.)
>
> Probably the difference is that x86 doesn't have this early-clobber
> constraint?
>
> >>
> >> Andrew
>