> On 23 Sep 2025, at 12:55, Kyrylo Tkachov <[email protected]> wrote:
>
>
>
>> On 23 Sep 2025, at 12:34, Tamar Christina <[email protected]> wrote:
>>
>> Hi Jennifer
>>
>>>> However the vast majority of these are 1, except for 2 entries
>>>>
>>>>> +
>>>>> + case NEOVERSEV2_DISPATCH_V_V13:
>>>>> + constraints.safe_push (std::make_pair (7, 2)); /* v pipelines (2
>>>>> slots). */
>>>>> + break;
>>>>> +
>>>>
>>>> like this one. How about dropping the std::pair and instead just adding
>>>> the element twice to
>>>> the constaints list.
>>>>
>>>> case NEOVERSEV2_DISPATCH_V_V13:
>>>> constraints.safe_push (7); /* v pipelines. */
>>>> constraints.safe_push (7); /* v pipelines. */
>>>> break;
>>>>
>>>> That would simplify the code in patch 2 slightly too.
>>> Using elements multiple times makes the function
>>> dispatch_window::fits_window more complicated because it would have to keep
>>> track of non-unique constraints to give a correct decision. Also, other
>>> CPUs might have more multiple-slots constraints, e.g. for reduction
>>> instructions or vector instructions that use multiple registers.
>>> So, currently I left it with pairs.
>>> What do you think?
>>
>> Ah, I see, it's because the fits_window would otherwise become quadratic in
>> the current setup.
>>
>> I've done some internal runs and haven't noticed anything regressions on
>> workloads we track.
>> I think there are a number of improvements we can make later to this that
>> should help scalar
>> and Adv. SIMD.
>>
>> But for now, the patch series is OK unless anyone has any objections.
>
> Yes, they’re fine with me too (I’ve reviewed various iterations of them
> internally).
> Thanks for the extra evaluation.
> Kyrill
>
Thanks, I pushed the patch series to master: 41c95d5e53e..0088e4a419e
And thank you, Tamar, for helping with performance evaluation.
Jennifer
>
>>
>> Thanks,
>> Tamar
>>
>>>>
>>>> I think we should also use an enum here (just use implicit conversion to
>>>> cast the enum back to int)
>>>> such that the numbers mean something.
>>>>
>>>> case NEOVERSEV2_DISPATCH_V_V13:
>>>> constraints.safe_push (V_PIPE); /* v pipelines. */
>>>> constraints.safe_push (V_PIPE); /* v pipelines. */
>>>> break;
>>> Done.
>>>>
>>>> Aside from these I think we're mostly there.
>>>>
>>>> Thanks,
>>>> Tamar