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.

> 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.)
>
> Andrew

Reply via email to