on 2021/7/13 下午8:42, Richard Biener wrote:
> On Tue, Jul 13, 2021 at 12:25 PM Kewen.Lin <li...@linux.ibm.com> wrote:
>>
>> Hi Richi,
>>
>> Thanks for the comments!
>>
>> on 2021/7/13 下午5:35, Richard Biener wrote:
>>> On Tue, Jul 13, 2021 at 10:53 AM Kewen.Lin <li...@linux.ibm.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> When I added the support for Power10 newly introduced multiply
>>>> highpart instrutions, I noticed that currently vectorizer
>>>> doesn't try to vectorize multiply highpart pattern, I hope
>>>> this isn't intentional?
>>>>
>>>> This patch is to extend the existing pattern mulhs handlings
>>>> to cover multiply highpart. Another alternative seems to
>>>> recog mul_highpart operation in a general place applied for
>>>> scalar code when the target supports the optab for the scalar
>>>> operation, it's based on the assumption that one target which
>>>> supports vector version of multiply highpart should have the
>>>> scalar version. I noticed that the function can_mult_highpart_p
>>>> can check/handle mult_highpart well even without mul_highpart
>>>> optab support, I think to recog this pattern in vectorizer
>>>> is better. Is it on the right track?
>>>
>>> I think it's on the right track, using IFN_LAST is a bit awkward
>>> in case yet another case pops up so maybe you can use
>>> a code_helper instance instead which unifies tree_code,
>>> builtin_code and internal_fn?
>>>
>>
>> If there is one new requirement which doesn't have/introduce IFN
>> stuffs but have one existing tree_code, can we add one more field
>> with type tree_code, then for the IFN_LAST path we can check the
>> different requirements under the guard with that tree_code variable?
>>
>>> I also notice that can_mult_highpart_p will return true if
>>> only vec_widen_[us]mult_{even,odd,hi,lo} are available,
>>> but then the result might be less optimal (or even not
>>> handled later)?
>>>
>>
>> I think it will be handled always? The expander calls
>>
>> rtx
>> expand_mult_highpart (machine_mode mode, rtx op0, rtx op1,
>> rtx target, bool uns_p)
>>
>> which will further check with can_mult_highpart_p.
>>
>> For the below case,
>>
>> #define SHT_CNT 16
>>
>> __attribute__ ((noipa)) void
>> test ()
>> {
>> for (int i = 0; i < N; i++)
>> sh_c[i] = ((SI) sh_a[i] * (SI) sh_b[i]) >> 16;
>> }
>>
>> Without this patch, it use widen_mult like below:
>>
>> vect__1.5_19 = MEM <vector(8) short int> [(short int *)&sh_a + ivtmp.18_24
>> * 1];
>> vect__3.8_14 = MEM <vector(8) short int> [(short int *)&sh_b + ivtmp.18_24
>> * 1];
>> vect_patt_22.9_13 = WIDEN_MULT_LO_EXPR <vect__3.8_14, vect__1.5_19>;
>> vect_patt_22.9_9 = WIDEN_MULT_HI_EXPR <vect__3.8_14, vect__1.5_19>;
>> vect__6.10_25 = vect_patt_22.9_13 >> 16;
>> vect__6.10_26 = vect_patt_22.9_9 >> 16;
>> vect__7.11_27 = VEC_PACK_TRUNC_EXPR <vect__6.10_25, vect__6.10_26>;
>> MEM <vector(8) short int> [(short int *)&sh_c + ivtmp.18_24 * 1] =
>> vect__7.11_27;
>>
>> .L2:
>> lxvx 33,7,9
>> lxvx 32,8,9
>> vmulosh 13,0,1 // widen mult
>> vmulesh 0,0,1
>> xxmrglw 33,32,45 // merge
>> xxmrghw 32,32,45
>> vsraw 1,1,12 // shift
>> vsraw 0,0,12
>> vpkuwum 0,0,1 // pack
>> stxvx 32,10,9
>> addi 9,9,16
>> bdnz .L2
>>
>>
>> With this patch, it ends up with:
>>
>> vect__1.5_14 = MEM <vector(8) short int> [(short int *)&sh_a + ivtmp.17_24
>> * 1];
>> vect__3.8_8 = MEM <vector(8) short int> [(short int *)&sh_b + ivtmp.17_24
>> * 1];
>> vect_patt_21.9_25 = vect__3.8_8 h* vect__1.5_14;
>> MEM <vector(8) short int> [(short int *)&sh_c + ivtmp.17_24 * 1] =
>> vect_patt_21.9_25;
>
> Yes, so I'm curious what it ends up with/without the patch on x86_64 which
> can do vec_widen_[us]mult_{even,odd} but not [us]mul_highpart.
>
For test case:
```
#define N 32
typedef signed int bigType;
typedef signed short smallType;
#define SH_CNT 16
extern smallType small_a[N], small_b[N], small_c[N];
__attribute__((noipa)) void test_si(int n) {
for (int i = 0; i < n; i++)
small_c[i] = ((bigType)small_a[i] * (bigType)small_b[i]) >> SH_CNT;
}
```
on x86_64, with option set: -mfpmath=sse -msse2 -O2 -ftree-vectorize
1) without this patch, the pattern isn't recognized, the IR looks like:
<bb 4> [local count: 94607391]:
bnd.5_34 = niters.4_25 >> 3;
_13 = (sizetype) bnd.5_34;
_29 = _13 * 16;
<bb 5> [local count: 378429566]:
# ivtmp.34_4 = PHI <ivtmp.34_5(5), 0(4)>
vect__1.10_40 = MEM <vector(8) short int> [(short int *)&small_a + ivtmp.34_4
* 1];
vect__3.13_43 = MEM <vector(8) short int> [(short int *)&small_b + ivtmp.34_4
* 1];
vect_patt_18.14_44 = WIDEN_MULT_LO_EXPR <vect__1.10_40, vect__3.13_43>;
vect_patt_18.14_45 = WIDEN_MULT_HI_EXPR <vect__1.10_40, vect__3.13_43>;
vect__6.15_46 = vect_patt_18.14_44 >> 16;
vect__6.15_47 = vect_patt_18.14_45 >> 16;
vect__7.16_48 = VEC_PACK_TRUNC_EXPR <vect__6.15_46, vect__6.15_47>;
MEM <vector(8) short int> [(short int *)&small_c + ivtmp.34_4 * 1] =
vect__7.16_48;
ivtmp.34_5 = ivtmp.34_4 + 16;
if (ivtmp.34_5 != _29)
goto <bb 5>; [75.00%]
else
goto <bb 6>; [25.00%]
...
*asm*:
.L4:
movdqu small_b(%rax), %xmm3
movdqu small_a(%rax), %xmm1
addq $16, %rax
movdqu small_a-16(%rax), %xmm2
pmullw %xmm3, %xmm1
pmulhw %xmm3, %xmm2
movdqa %xmm1, %xmm0
punpcklwd %xmm2, %xmm0
punpckhwd %xmm2, %xmm1
psrad $16, %xmm1
psrad $16, %xmm0
movdqa %xmm0, %xmm2
punpcklwd %xmm1, %xmm0
punpckhwd %xmm1, %xmm2
movdqa %xmm0, %xmm1
punpckhwd %xmm2, %xmm1
punpcklwd %xmm2, %xmm0
punpcklwd %xmm1, %xmm0
movups %xmm0, small_c-16(%rax)
cmpq %rdx, %rax
jne .L4
movl %edi, %eax
andl $-8, %eax
testb $7, %dil
je .L14
*insn dist in loop*
1 addq
3 movdqa
3 movdqu
1 movups
1 pmulhw
1 pmullw
2 psrad
3 punpckhwd
4 punpcklwd
2) with this patch but make the mul_highpart optab query return false, the IR
looks
like:
<bb 4> [local count: 94607391]:
bnd.5_37 = niters.4_22 >> 3;
_13 = (sizetype) bnd.5_37;
_32 = _13 * 16;
<bb 5> [local count: 378429566]:
# ivtmp.33_4 = PHI <ivtmp.33_5(5), 0(4)>
vect__1.10_43 = MEM <vector(8) short int> [(short int *)&small_a + ivtmp.33_4
* 1];
vect__3.13_46 = MEM <vector(8) short int> [(short int *)&small_b + ivtmp.33_4
* 1];
vect_patt_26.14_47 = vect__1.10_43 h* vect__3.13_46;
MEM <vector(8) short int> [(short int *)&small_c + ivtmp.33_4 * 1] =
vect_patt_26.14_47;
ivtmp.33_5 = ivtmp.33_4 + 16;
if (ivtmp.33_5 != _32)
goto <bb 5>; [75.00%]
else
goto <bb 6>; [25.00%]
*asm*:
.L4:
movdqu small_b(%rax), %xmm3
movdqu small_a(%rax), %xmm1
addq $16, %rax
movdqu small_a-16(%rax), %xmm2
pmullw %xmm3, %xmm1
pmulhw %xmm3, %xmm2
movdqa %xmm1, %xmm0
punpcklwd %xmm2, %xmm0
punpckhwd %xmm2, %xmm1
movdqa %xmm0, %xmm2
punpcklwd %xmm1, %xmm0
punpckhwd %xmm1, %xmm2
movdqa %xmm0, %xmm1
punpckhwd %xmm2, %xmm1
punpcklwd %xmm2, %xmm0
punpckhwd %xmm1, %xmm0
movups %xmm0, small_c-16(%rax)
cmpq %rdx, %rax
jne .L4
movl %edi, %eax
andl $-8, %eax
testb $7, %dil
je .L14
*insn dist*:
1 addq
3 movdqa
3 movdqu
1 movups
1 pmulhw
1 pmullw
4 punpckhwd
3 punpcklwd
I know little on i386 asm, but this seems better from insn distribution,
the one without this patch uses two more psrad (s).
3) FWIW with this patch as well as existing optab supports, the IR looks like:
<bb 4> [local count: 94607391]:
bnd.5_40 = niters.4_19 >> 3;
_75 = (sizetype) bnd.5_40;
_91 = _75 * 16;
<bb 5> [local count: 378429566]:
# ivtmp.48_53 = PHI <ivtmp.48_45(5), 0(4)>
vect__1.10_48 = MEM <vector(8) short int> [(short int *)&small_a +
ivtmp.48_53 * 1];
vect__3.13_51 = MEM <vector(8) short int> [(short int *)&small_b +
ivtmp.48_53 * 1];
vect_patt_26.14_52 = vect__1.10_48 h* vect__3.13_51;
MEM <vector(8) short int> [(short int *)&small_c + ivtmp.48_53 * 1] =
vect_patt_26.14_52;
ivtmp.48_45 = ivtmp.48_53 + 16;
if (ivtmp.48_45 != _91)
goto <bb 5>; [75.00%]
else
goto <bb 6>; [25.00%]
<bb 6> [local count: 94607391]:
niters_vector_mult_vf.6_41 = niters.4_19 & 4294967288;
tmp.7_42 = (int) niters_vector_mult_vf.6_41;
_61 = niters.4_19 & 7;
if (_61 == 0)
goto <bb 12>; [12.50%]
else
goto <bb 7>; [87.50%]
<bb 7> [local count: 93293400]:
# i_38 = PHI <tmp.7_42(6), 0(3)>
# _44 = PHI <niters_vector_mult_vf.6_41(6), 0(3)>
niters.18_60 = niters.4_19 - _44;
_76 = niters.18_60 + 4294967295;
if (_76 <= 2)
goto <bb 9>; [10.00%]
else
goto <bb 8>; [90.00%]
<bb 8> [local count: 167928121]:
_85 = (sizetype) _44;
_86 = _85 * 2;
vectp_small_a.23_84 = &small_a + _86;
vectp_small_b.26_90 = &small_b + _86;
vectp_small_c.31_98 = &small_c + _86;
vect__9.24_89 = MEM <vector(4) short int> [(short int *)vectp_small_a.23_84];
vect__28.27_95 = MEM <vector(4) short int> [(short int *)vectp_small_b.26_90];
vect_patt_23.28_96 = vect__9.24_89 h* vect__28.27_95;
MEM <vector(4) short int> [(short int *)vectp_small_c.31_98] =
vect_patt_23.28_96;
niters_vector_mult_vf.20_80 = niters.18_60 & 4294967292;
_82 = (int) niters_vector_mult_vf.20_80;
tmp.21_81 = i_38 + _82;
_74 = niters.18_60 & 3;
if (_74 == 0)
goto <bb 11>; [25.00%]
else
goto <bb 9>; [75.00%]
...
*asm*:
.L4:
movdqu small_a(%rax), %xmm0
movdqu small_b(%rax), %xmm2
addq $16, %rax
pmulhw %xmm2, %xmm0
movups %xmm0, small_c-16(%rax)
cmpq %rdx, %rax
jne .L4
movl %ecx, %edx
andl $-8, %edx
movl %edx, %eax
testb $7, %cl
je .L19
.L3:
movl %ecx, %esi
subl %edx, %esi
leal -1(%rsi), %edi
cmpl $2, %edi
jbe .L6
movq small_a(%rdx,%rdx), %xmm0
movq small_b(%rdx,%rdx), %xmm1
pmulhw %xmm1, %xmm0
movq %xmm0, small_c(%rdx,%rdx)
movl %esi, %edx
andl $-4, %edx
addl %edx, %eax
andl $3, %esi
je .L1
I guess the proposed IFN would be directly mapped for [us]mul_highpart?
or do you expect it will also cover what we support in can_mult_highpart_p?
BR,
Kewen
