Addressed the comments in PATCH v2 as below.
https://gcc.gnu.org/pipermail/gcc-patches/2023-June/621789.html
Pan
-----Original Message-----
From: Gcc-patches <gcc-patches-bounces+pan2.li=intel....@gcc.gnu.org> On Behalf
Of Jeff Law via Gcc-patches
Sent: Thursday, June 15, 2023 3:11 AM
To: Robin Dapp <rdapp....@gmail.com>; juzhe.zh...@rivai.ai;
gcc-patches@gcc.gnu.org
Cc: kito.ch...@gmail.com; kito.ch...@sifive.com; pal...@dabbelt.com;
pal...@rivosinc.com
Subject: Re: [PATCH] RISC-V: Use merge approach to optimize vector permutation
On 6/14/23 09:00, Robin Dapp wrote:
> Hi Juzhe,
>
> the general method seems sane and useful (it's not very complicated).
> I was just distracted by
>
>> Selector = { 0, 17, 2, 19, 4, 21, 6, 23, 8, 9, 10, 27, 12, 29, 14, 31 }, the
>> common expression:
>> { 0, nunits + 1, 1, nunits + 2, 2, nunits + 3, ... }
>>
>> For this selector, we can use vmsltu + vmerge to optimize the codegen.
>
> because it's actually { 0, nunits + 1, 2, nunits + 3, ... } or maybe
> { 0, nunits, 0, nunits, ... } + { 0, 1, 2, 3, ..., nunits - 1 }.
>
> Because of the ascending/monotonic? selector structure we can use vmerge
> instead of vrgather.
>
>> +/* Recognize the patterns that we can use merge operation to shuffle the
>> + vectors. The value of Each element (index i) in selector can only be
>> + either i or nunits + i.
>> +
>> + E.g.
>> + v = VEC_PERM_EXPR (v0, v1, selector),
>> + selector = { 0, nunits + 1, 1, nunits + 2, 2, nunits + 3, ... }
>
> Same.
>
>> +
>> + We can transform such pattern into:
>> +
>> + v = vcond_mask (v0, v1, mask),
>> + mask = { 0, 1, 0, 1, 0, 1, ... }. */
>> +
>> +static bool
>> +shuffle_merge_patterns (struct expand_vec_perm_d *d)
>> +{
>> + machine_mode vmode = d->vmode;
>> + machine_mode sel_mode = related_int_vector_mode (vmode).require ();
>> + int n_patterns = d->perm.encoding ().npatterns ();
>> + poly_int64 vec_len = d->perm.length ();
>> +
>> + for (int i = 0; i < n_patterns; ++i)
>> + if (!known_eq (d->perm[i], i) && !known_eq (d->perm[i], vec_len + i))
>> + return false;
>> +
>> + for (int i = n_patterns; i < n_patterns * 2; i++)
>> + if (!d->perm.series_p (i, n_patterns, i, n_patterns)
>> + && !d->perm.series_p (i, n_patterns, vec_len + i, n_patterns))
>> + return false;
>
> Maybe add a comment that we check that the pattern is actually monotonic
> or however you prefet to call it?
>
> I didn't go through all tests in detail but skimmed several. All in all
> looks good to me.
So I think that means we want a V2 for the comment updates. But I think
we can go ahead and consider V2 pre-approved.
jeff
