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

Andrew Stubbs <[email protected]> Fri, 3 Jul 2026 12:43:02 +0100
Newsgroups gmane.comp.gcc.devel
Message-ID <[email protected]>
On 25/06/2026 18:22, Andrew Stubbs wrote:
> On 25/06/2026 12:22, Richard Biener wrote:
>> 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.
> 
> Again, I think this one only works if operand 1 never gets reloaded, 
> which may be OK as there's no early clobber.
> 
> I had to modify my scheme though; it turns out (mem (unspec)) doesn't 
> permit LRA to see the base/offset registers, so it only works if the 
> registers within happen to get assigned to the right classes by other 
> instructions.  However, there's provision in there to "add" a machine- 
> specific unspec (known as "segment" in rtlanal.cc), so I can use that.  
> My addresses now look like this:
> 
> (mem:V64SI (plus:V64DI
>               (unspec:V64DI [(reg:DI mask_reg)] UNSPEC_MASK)
>               (plus:V64DI
>                 (reg:V64DI base_reg)
>                 (const_int offset))))
> 
> The mask_reg is only ever used to force new load instructions to use the 
> vec_merge pattern, when something (a reload) tries to use the MEM out of 
> context.

I had to give up on this scheme. It solved the immediate problem quite 
neatly, but the extra "mask_reg" element is causing reloads by itself. 
There's no way to specify that it should match the mask operand of 
vec_merge (or indeed add any constraint at all), so it was getting 
assigned to an independent register. Sometimes this was fine, but 
sometimes it made a mess of the codegen.

I've now got a specific insn for masked loads that has bare address, 
offset, and address space operands (no explicit MEM). This isn't what I 
wanted, but I'm still using vector mem for the unmasked loads and all 
stores.

Andrew