Re: [PATCH 6/8] vect: Add vector_mode paramater to simd_clone_usable

["Andre Vieira (lists)" <andre.simoesdiasvieira@arm.com>]

On 31/08/2023 07:39, Richard Biener wrote:
> On Wed, Aug 30, 2023 at 5:02 PM Andre Vieira (lists)
> <andre.simoesdiasvieira@arm.com> wrote:
>>
>>
>>
>> On 30/08/2023 14:01, Richard Biener wrote:
>>> On Wed, Aug 30, 2023 at 11:15 AM Andre Vieira (lists) via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>
>>>> This patch adds a machine_mode parameter to the TARGET_SIMD_CLONE_USABLE
>>>> hook to enable rejecting SVE modes when the target architecture does not
>>>> support SVE.
>>>
>>> How does the graph node of the SIMD clone lack this information?  That is, it
>>> should have information on the types (and thus modes) for all formal arguments
>>> and return values already, no?  At least the target would know how to
>>> instantiate
>>> it if it's not readily available at the point of use.
>>>
>>
>> Yes it does, but that's the modes the simd clone itself uses, it does
>> not know what vector_mode we are currently vectorizing for. Which is
>> exactly why we need the vinfo's vector_mode to make sure the simd clone
>> and its types are compatible with the vector mode.
>>
>> In practice, to make sure that a SVE simd clones are only used in loops
>> being vectorized for SVE modes. Having said that... I just realized that
>> the simdlen check already takes care of that currently...
>>
>> by simdlen check I mean the one that writes off simdclones that match:
>>           if (!constant_multiple_p (vf, n->simdclone->simdlen, &num_calls)
>>
>> However, when using -msve-vector-bits this will become an issue, as the
>> VF will be constant and we will match NEON simdclones.  This requires
>> some further attention though given that we now also reject the use of
>> SVE simdclones when using -msve-vector-bits, and I'm not entirely sure
>> we should...
> 
> Hmm, but vectorizable_simdclone should check for compatible types here
> and if they are compatible why should we reject them?  Are -msve-vector-bits
> "SVE" modes different from "NEON" modes?  I suppose not, because otherwise
> the type compatibility check would say incompatible.
> 
Prior to transformation we do all checks on the original scalar values, 
not the vector types. But I do believe you are right in that we don't 
need to pass the vector_mode. The simdlen check should be enough and if 
the length is the same or a multiple of the rest of the could should be 
able to deal with that and any conversions when dealing with things like 
SVE types that require the attribute.

I'll update the patch series soon and after that I'll look at how this 
reacts to -msve-vector-bits in more detail.

Thanks,
Andre