Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer
On Thu, Oct 26, 2023 at 4:18 AM wrote: > > From: Pan Li > > Update in v2: > > * Fix one ICE of type assertion. > * Adjust some test cases for aarch64 sve and riscv vector. > > Original log: > > The vectoriable_call has one restriction of the size of data type. > Aka DF to DI is allowed but SF to DI isn't. You may see below message > when try to vectorize function call like lrintf. > > void > test_lrintf (long *out, float *in, unsigned count) > { > for (unsigned i = 0; i < count; i++) > out[i] = __builtin_lrintf (in[i]); > } > > lrintf.c:5:26: missed: couldn't vectorize loop > lrintf.c:5:26: missed: not vectorized: unsupported data-type > > Then the standard name pattern like lrintmn2 cannot work for different > data type size like SF => DI. This patch would like to remove this data > type size check and unblock the standard name like lrintmn2. > > The below test are passed for this patch. > > * The x86 bootstrap and regression test. > * The aarch64 regression test. > * The risc-v regression tests. > > gcc/ChangeLog: > > * internal-fn.cc (expand_fn_using_insn): Add vector int assertion. > * tree-vect-stmts.cc (vectorizable_call): Remove size check. > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/sve/clrsb_1.c: Adjust checker. > * gcc.target/aarch64/sve/clz_1.c: Ditto. > * gcc.target/aarch64/sve/popcount_1.c: Ditto. > * gcc.target/riscv/rvv/autovec/unop/popcount.c: Ditto. > > Signed-off-by: Pan Li > --- > gcc/internal-fn.cc | 3 ++- > gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c | 3 +-- > gcc/testsuite/gcc.target/aarch64/sve/clz_1.c| 3 +-- > gcc/testsuite/gcc.target/aarch64/sve/popcount_1.c | 3 +-- > .../gcc.target/riscv/rvv/autovec/unop/popcount.c| 2 +- > gcc/tree-vect-stmts.cc | 13 - > 6 files changed, 6 insertions(+), 21 deletions(-) > > diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc > index 61d5a9e4772..17c0f4c3805 100644 > --- a/gcc/internal-fn.cc > +++ b/gcc/internal-fn.cc > @@ -281,7 +281,8 @@ expand_fn_using_insn (gcall *stmt, insn_code icode, > unsigned int noutputs, > emit_move_insn (lhs_rtx, ops[0].value); >else > { > - gcc_checking_assert (INTEGRAL_TYPE_P (TREE_TYPE (lhs))); > + gcc_checking_assert (INTEGRAL_TYPE_P (TREE_TYPE (lhs)) > + || VECTOR_INTEGER_TYPE_P (TREE_TYPE (lhs))); Can you explain why this is necessary? In particular what is lhs_rtx mode vs ops[0].value mode? > convert_move (lhs_rtx, ops[0].value, 0); I'm not sure convert_move handles vector modes correctly. Richard probably added this code, CCed. Richard. > } > } > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c > b/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c > index bdc9856faaf..940d08bbc7b 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c > @@ -18,5 +18,4 @@ clrsb_64 (unsigned int *restrict dst, uint64_t *restrict > src, int size) > } > > /* { dg-final { scan-assembler-times {\tcls\tz[0-9]+\.s, p[0-7]/m, > z[0-9]+\.s\n} 1 } } */ > -/* { dg-final { scan-assembler-times {\tcls\tz[0-9]+\.d, p[0-7]/m, > z[0-9]+\.d\n} 2 } } */ > -/* { dg-final { scan-assembler-times {\tuzp1\tz[0-9]+\.s, z[0-9]+\.s, > z[0-9]+\.s\n} 1 } } */ > +/* { dg-final { scan-assembler-times {\tcls\tz[0-9]+\.d, p[0-7]/m, > z[0-9]+\.d\n} 1 } } */ > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/clz_1.c > b/gcc/testsuite/gcc.target/aarch64/sve/clz_1.c > index 0c7a4e6d768..58b8ff406d2 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/clz_1.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/clz_1.c > @@ -18,5 +18,4 @@ clz_64 (unsigned int *restrict dst, uint64_t *restrict src, > int size) > } > > /* { dg-final { scan-assembler-times {\tclz\tz[0-9]+\.s, p[0-7]/m, > z[0-9]+\.s\n} 1 } } */ > -/* { dg-final { scan-assembler-times {\tclz\tz[0-9]+\.d, p[0-7]/m, > z[0-9]+\.d\n} 2 } } */ > -/* { dg-final { scan-assembler-times {\tuzp1\tz[0-9]+\.s, z[0-9]+\.s, > z[0-9]+\.s\n} 1 } } */ > +/* { dg-final { scan-assembler-times {\tclz\tz[0-9]+\.d, p[0-7]/m, > z[0-9]+\.d\n} 1 } } */ > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/popcount_1.c > b/gcc/testsuite/gcc.target/aarch64/sve/popcount_1.c > index dfb6f4ac7a5..0eba898307c 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/popcount_1.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/popcount_1.c > @@ -18,5 +18,4 @@ popcount_64 (unsigned int *restrict dst, uint64_t *restrict > src, int size) > } > > /* { dg-final { scan-assembler-times {\tcnt\tz[0-9]+\.s, p[0-7]/m, > z[0-9]+\.s\n} 1 } } */ > -/* { dg-final { scan-assembler-times {\tcnt\tz[0-9]+\.d, p[0-7]/m, > z[0-9]+\.d\n} 2 } } */ > -/* { dg-final { scan-assembler-times {\tuzp1\tz[0-9]+\.s, z[0-9]+\.s, > z[0-9]+\.s\n} 1 } } */ > +/* { dg-final { scan-assembler-times {\tcnt\tz[0-9]+\.d, p[0-7]/m, >
RE: [PATCH v2] VECT: Remove the type size restriction of vectorizer
Thanks Richard for comments. > Can you explain why this is necessary? In particular what is lhs_rtx > mode vs ops[0].value mode? For testcase gcc.target/aarch64/sve/popcount_1.c, the rtl are list as below. The lhs_rtx is (reg:VNx2SI 98 [ vect__5.36 ]). The ops[0].value is (reg:VNx2DI 104). The restriction removing make the vector rtl enter expand_fn_using_insn and of course hit the INTEGER_P assertion. Pan -Original Message- From: Richard Biener Sent: Thursday, October 26, 2023 4:38 PM To: Li, Pan2 Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang, Yanzhang ; kito.ch...@gmail.com; Liu, Hongtao ; Richard Sandiford Subject: Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer On Thu, Oct 26, 2023 at 4:18 AM wrote: > > From: Pan Li > > Update in v2: > > * Fix one ICE of type assertion. > * Adjust some test cases for aarch64 sve and riscv vector. > > Original log: > > The vectoriable_call has one restriction of the size of data type. > Aka DF to DI is allowed but SF to DI isn't. You may see below message > when try to vectorize function call like lrintf. > > void > test_lrintf (long *out, float *in, unsigned count) > { > for (unsigned i = 0; i < count; i++) > out[i] = __builtin_lrintf (in[i]); > } > > lrintf.c:5:26: missed: couldn't vectorize loop > lrintf.c:5:26: missed: not vectorized: unsupported data-type > > Then the standard name pattern like lrintmn2 cannot work for different > data type size like SF => DI. This patch would like to remove this data > type size check and unblock the standard name like lrintmn2. > > The below test are passed for this patch. > > * The x86 bootstrap and regression test. > * The aarch64 regression test. > * The risc-v regression tests. > > gcc/ChangeLog: > > * internal-fn.cc (expand_fn_using_insn): Add vector int assertion. > * tree-vect-stmts.cc (vectorizable_call): Remove size check. > > gcc/testsuite/ChangeLog: > > * gcc.target/aarch64/sve/clrsb_1.c: Adjust checker. > * gcc.target/aarch64/sve/clz_1.c: Ditto. > * gcc.target/aarch64/sve/popcount_1.c: Ditto. > * gcc.target/riscv/rvv/autovec/unop/popcount.c: Ditto. > > Signed-off-by: Pan Li > --- > gcc/internal-fn.cc | 3 ++- > gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c | 3 +-- > gcc/testsuite/gcc.target/aarch64/sve/clz_1.c| 3 +-- > gcc/testsuite/gcc.target/aarch64/sve/popcount_1.c | 3 +-- > .../gcc.target/riscv/rvv/autovec/unop/popcount.c| 2 +- > gcc/tree-vect-stmts.cc | 13 - > 6 files changed, 6 insertions(+), 21 deletions(-) > > diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc > index 61d5a9e4772..17c0f4c3805 100644 > --- a/gcc/internal-fn.cc > +++ b/gcc/internal-fn.cc > @@ -281,7 +281,8 @@ expand_fn_using_insn (gcall *stmt, insn_code icode, > unsigned int noutputs, > emit_move_insn (lhs_rtx, ops[0].value); >else > { > - gcc_checking_assert (INTEGRAL_TYPE_P (TREE_TYPE (lhs))); > + gcc_checking_assert (INTEGRAL_TYPE_P (TREE_TYPE (lhs)) > + || VECTOR_INTEGER_TYPE_P (TREE_TYPE (lhs))); Can you explain why this is necessary? In particular what is lhs_rtx mode vs ops[0].value mode? > convert_move (lhs_rtx, ops[0].value, 0); I'm not sure convert_move handles vector modes correctly. Richard probably added this code, CCed. Richard. > } > } > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c > b/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c > index bdc9856faaf..940d08bbc7b 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c > @@ -18,5 +18,4 @@ clrsb_64 (unsigned int *restrict dst, uint64_t *restrict > src, int size) > } > > /* { dg-final { scan-assembler-times {\tcls\tz[0-9]+\.s, p[0-7]/m, > z[0-9]+\.s\n} 1 } } */ > -/* { dg-final { scan-assembler-times {\tcls\tz[0-9]+\.d, p[0-7]/m, > z[0-9]+\.d\n} 2 } } */ > -/* { dg-final { scan-assembler-times {\tuzp1\tz[0-9]+\.s, z[0-9]+\.s, > z[0-9]+\.s\n} 1 } } */ > +/* { dg-final { scan-assembler-times {\tcls\tz[0-9]+\.d, p[0-7]/m, > z[0-9]+\.d\n} 1 } } */ > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/clz_1.c > b/gcc/testsuite/gcc.target/aarch64/sve/clz_1.c > index 0c7a4e6d768..58b8ff406d2 100644 > --- a/gcc/testsuite/gcc.target/aarch64/sve/clz_1.c > +++ b/gcc/testsuite/gcc.target/aarch64/sve/clz_1.c > @@ -18,5 +18,4 @@ clz_64 (unsigned int *restrict dst, uint64_t *restrict src, > int size) > } > > /* { dg-final { scan-assembler-times {\tclz\tz[0-9]+\.s, p[0-7]/m, > z[0-
Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer
> Am 26.10.2023 um 13:59 schrieb Li, Pan2 : > > Thanks Richard for comments. > >> Can you explain why this is necessary? In particular what is lhs_rtx >> mode vs ops[0].value mode? > > For testcase gcc.target/aarch64/sve/popcount_1.c, the rtl are list as below. > > The lhs_rtx is (reg:VNx2SI 98 [ vect__5.36 ]). > The ops[0].value is (reg:VNx2DI 104). > > The restriction removing make the vector rtl enter expand_fn_using_insn and > of course hit the INTEGER_P assertion. But I think this shows we mid-selected the optab, a convert_move is certainly not correct unconditionally here (the target might not support that) > Pan > > -Original Message- > From: Richard Biener > Sent: Thursday, October 26, 2023 4:38 PM > To: Li, Pan2 > Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang, Yanzhang > ; kito.ch...@gmail.com; Liu, Hongtao > ; Richard Sandiford > Subject: Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer > >> On Thu, Oct 26, 2023 at 4:18 AM wrote: >> >> From: Pan Li >> >> Update in v2: >> >> * Fix one ICE of type assertion. >> * Adjust some test cases for aarch64 sve and riscv vector. >> >> Original log: >> >> The vectoriable_call has one restriction of the size of data type. >> Aka DF to DI is allowed but SF to DI isn't. You may see below message >> when try to vectorize function call like lrintf. >> >> void >> test_lrintf (long *out, float *in, unsigned count) >> { >> for (unsigned i = 0; i < count; i++) >>out[i] = __builtin_lrintf (in[i]); >> } >> >> lrintf.c:5:26: missed: couldn't vectorize loop >> lrintf.c:5:26: missed: not vectorized: unsupported data-type >> >> Then the standard name pattern like lrintmn2 cannot work for different >> data type size like SF => DI. This patch would like to remove this data >> type size check and unblock the standard name like lrintmn2. >> >> The below test are passed for this patch. >> >> * The x86 bootstrap and regression test. >> * The aarch64 regression test. >> * The risc-v regression tests. >> >> gcc/ChangeLog: >> >>* internal-fn.cc (expand_fn_using_insn): Add vector int assertion. >>* tree-vect-stmts.cc (vectorizable_call): Remove size check. >> >> gcc/testsuite/ChangeLog: >> >>* gcc.target/aarch64/sve/clrsb_1.c: Adjust checker. >>* gcc.target/aarch64/sve/clz_1.c: Ditto. >>* gcc.target/aarch64/sve/popcount_1.c: Ditto. >>* gcc.target/riscv/rvv/autovec/unop/popcount.c: Ditto. >> >> Signed-off-by: Pan Li >> --- >> gcc/internal-fn.cc | 3 ++- >> gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c | 3 +-- >> gcc/testsuite/gcc.target/aarch64/sve/clz_1.c| 3 +-- >> gcc/testsuite/gcc.target/aarch64/sve/popcount_1.c | 3 +-- >> .../gcc.target/riscv/rvv/autovec/unop/popcount.c| 2 +- >> gcc/tree-vect-stmts.cc | 13 - >> 6 files changed, 6 insertions(+), 21 deletions(-) >> >> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc >> index 61d5a9e4772..17c0f4c3805 100644 >> --- a/gcc/internal-fn.cc >> +++ b/gcc/internal-fn.cc >> @@ -281,7 +281,8 @@ expand_fn_using_insn (gcall *stmt, insn_code icode, >> unsigned int noutputs, >>emit_move_insn (lhs_rtx, ops[0].value); >> else >>{ >> - gcc_checking_assert (INTEGRAL_TYPE_P (TREE_TYPE (lhs))); >> + gcc_checking_assert (INTEGRAL_TYPE_P (TREE_TYPE (lhs)) >> + || VECTOR_INTEGER_TYPE_P (TREE_TYPE (lhs))); > > Can you explain why this is necessary? In particular what is lhs_rtx > mode vs ops[0].value mode? > >> convert_move (lhs_rtx, ops[0].value, 0); > > I'm not sure convert_move handles vector modes correctly. Richard > probably added this code, CCed. > > Richard. > >>} >> } >> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c >> b/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c >> index bdc9856faaf..940d08bbc7b 100644 >> --- a/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c >> +++ b/gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c >> @@ -18,5 +18,4 @@ clrsb_64 (unsigned int *restrict dst, uint64_t *restrict >> src, int size) >> } >> >> /* { dg-final { scan-assembler-times {\tcls\tz[0-9]+\.s, p[0-7]/m, >> z[0-9]+\.s\n} 1 } } */ >> -/* { dg-final { scan-assembler-times {\tcls\tz[0-9]+\.d, p[0-
RE: [PATCH v2] VECT: Remove the type size restriction of vectorizer
> But I think this shows we mid-selected the optab, a convert_move is certainly > not correct unconditionally here (the target might not support that) Make sense, we can wait a while for the confirmation from Richard S. If convert_move is not designed for Vector (looks like mostly up to a point), I am not sure if we can fix the assertion like below ... else If (VECTOR_INTERGER_TYPE (TREE_TYPE(lhs))) return; else { gcc_checking_assert (INTEGRAL_TYPE_TYPE_P (TREE_TYPE (lhs))); convert_move (lhs_rtx, ops[0].value, 0); } Aka bypass the vector here, but I am afraid this change may make the llrintf (SF => DI) not working on standard name. Let me have a try and keep you posted. Pan -Original Message- From: Richard Biener Sent: Thursday, October 26, 2023 10:00 PM To: Li, Pan2 Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang, Yanzhang ; kito.ch...@gmail.com; Liu, Hongtao ; Richard Sandiford Subject: Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer > Am 26.10.2023 um 13:59 schrieb Li, Pan2 : > > Thanks Richard for comments. > >> Can you explain why this is necessary? In particular what is lhs_rtx >> mode vs ops[0].value mode? > > For testcase gcc.target/aarch64/sve/popcount_1.c, the rtl are list as below. > > The lhs_rtx is (reg:VNx2SI 98 [ vect__5.36 ]). > The ops[0].value is (reg:VNx2DI 104). > > The restriction removing make the vector rtl enter expand_fn_using_insn and > of course hit the INTEGER_P assertion. But I think this shows we mid-selected the optab, a convert_move is certainly not correct unconditionally here (the target might not support that) > Pan > > -Original Message- > From: Richard Biener > Sent: Thursday, October 26, 2023 4:38 PM > To: Li, Pan2 > Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang, Yanzhang > ; kito.ch...@gmail.com; Liu, Hongtao > ; Richard Sandiford > Subject: Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer > >> On Thu, Oct 26, 2023 at 4:18 AM wrote: >> >> From: Pan Li >> >> Update in v2: >> >> * Fix one ICE of type assertion. >> * Adjust some test cases for aarch64 sve and riscv vector. >> >> Original log: >> >> The vectoriable_call has one restriction of the size of data type. >> Aka DF to DI is allowed but SF to DI isn't. You may see below message >> when try to vectorize function call like lrintf. >> >> void >> test_lrintf (long *out, float *in, unsigned count) >> { >> for (unsigned i = 0; i < count; i++) >>out[i] = __builtin_lrintf (in[i]); >> } >> >> lrintf.c:5:26: missed: couldn't vectorize loop >> lrintf.c:5:26: missed: not vectorized: unsupported data-type >> >> Then the standard name pattern like lrintmn2 cannot work for different >> data type size like SF => DI. This patch would like to remove this data >> type size check and unblock the standard name like lrintmn2. >> >> The below test are passed for this patch. >> >> * The x86 bootstrap and regression test. >> * The aarch64 regression test. >> * The risc-v regression tests. >> >> gcc/ChangeLog: >> >>* internal-fn.cc (expand_fn_using_insn): Add vector int assertion. >>* tree-vect-stmts.cc (vectorizable_call): Remove size check. >> >> gcc/testsuite/ChangeLog: >> >>* gcc.target/aarch64/sve/clrsb_1.c: Adjust checker. >>* gcc.target/aarch64/sve/clz_1.c: Ditto. >>* gcc.target/aarch64/sve/popcount_1.c: Ditto. >>* gcc.target/riscv/rvv/autovec/unop/popcount.c: Ditto. >> >> Signed-off-by: Pan Li >> --- >> gcc/internal-fn.cc | 3 ++- >> gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c | 3 +-- >> gcc/testsuite/gcc.target/aarch64/sve/clz_1.c| 3 +-- >> gcc/testsuite/gcc.target/aarch64/sve/popcount_1.c | 3 +-- >> .../gcc.target/riscv/rvv/autovec/unop/popcount.c| 2 +- >> gcc/tree-vect-stmts.cc | 13 - >> 6 files changed, 6 insertions(+), 21 deletions(-) >> >> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc >> index 61d5a9e4772..17c0f4c3805 100644 >> --- a/gcc/internal-fn.cc >> +++ b/gcc/internal-fn.cc >> @@ -281,7 +281,8 @@ expand_fn_using_insn (gcall *stmt, insn_code icode, >> unsigned int noutputs, >>emit_move_insn (lhs_rtx, ops[0].value); >> else >>{ >> - gcc_checking_assert (INTEGRAL_TYPE_P (TREE_TYPE (lhs))); >> + gcc_checking_assert (INTEGRAL_TYPE_P (TREE_TYPE (lhs)) >> +
Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer
Richard Biener writes: >> Am 26.10.2023 um 13:59 schrieb Li, Pan2 : >> >> Thanks Richard for comments. >> >>> Can you explain why this is necessary? In particular what is lhs_rtx >>> mode vs ops[0].value mode? >> >> For testcase gcc.target/aarch64/sve/popcount_1.c, the rtl are list as below. >> >> The lhs_rtx is (reg:VNx2SI 98 [ vect__5.36 ]). >> The ops[0].value is (reg:VNx2DI 104). >> >> The restriction removing make the vector rtl enter expand_fn_using_insn and >> of course hit the INTEGER_P assertion. > > But I think this shows we mid-selected the optab, a convert_move is certainly > not correct unconditionally here (the target might not support that) Agreed. Allowing TYPE_SIZE (vectype_in) != TYPE_SIZE (vectype_out) makes sense if the called function allows the input and output modes to vary. That's true for internal functions that eventually map to two-mode optabs. But we can't remove the condition for calls to other functions, at least not without some fix-ups. ISTM that the problem being hit is the one described by the removed comment. In other words, I don't think simply removing the test from the vectoriser is correct. It needs to be replaced by something more selective. Thanks, Richard >> Pan >> >> -Original Message- >> From: Richard Biener >> Sent: Thursday, October 26, 2023 4:38 PM >> To: Li, Pan2 >> Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang, Yanzhang >> ; kito.ch...@gmail.com; Liu, Hongtao >> ; Richard Sandiford >> Subject: Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer >> >>> On Thu, Oct 26, 2023 at 4:18 AM wrote: >>> >>> From: Pan Li >>> >>> Update in v2: >>> >>> * Fix one ICE of type assertion. >>> * Adjust some test cases for aarch64 sve and riscv vector. >>> >>> Original log: >>> >>> The vectoriable_call has one restriction of the size of data type. >>> Aka DF to DI is allowed but SF to DI isn't. You may see below message >>> when try to vectorize function call like lrintf. >>> >>> void >>> test_lrintf (long *out, float *in, unsigned count) >>> { >>> for (unsigned i = 0; i < count; i++) >>>out[i] = __builtin_lrintf (in[i]); >>> } >>> >>> lrintf.c:5:26: missed: couldn't vectorize loop >>> lrintf.c:5:26: missed: not vectorized: unsupported data-type >>> >>> Then the standard name pattern like lrintmn2 cannot work for different >>> data type size like SF => DI. This patch would like to remove this data >>> type size check and unblock the standard name like lrintmn2. >>> >>> The below test are passed for this patch. >>> >>> * The x86 bootstrap and regression test. >>> * The aarch64 regression test. >>> * The risc-v regression tests. >>> >>> gcc/ChangeLog: >>> >>>* internal-fn.cc (expand_fn_using_insn): Add vector int assertion. >>>* tree-vect-stmts.cc (vectorizable_call): Remove size check. >>> >>> gcc/testsuite/ChangeLog: >>> >>>* gcc.target/aarch64/sve/clrsb_1.c: Adjust checker. >>>* gcc.target/aarch64/sve/clz_1.c: Ditto. >>>* gcc.target/aarch64/sve/popcount_1.c: Ditto. >>>* gcc.target/riscv/rvv/autovec/unop/popcount.c: Ditto. >>> >>> Signed-off-by: Pan Li >>> --- >>> gcc/internal-fn.cc | 3 ++- >>> gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c | 3 +-- >>> gcc/testsuite/gcc.target/aarch64/sve/clz_1.c| 3 +-- >>> gcc/testsuite/gcc.target/aarch64/sve/popcount_1.c | 3 +-- >>> .../gcc.target/riscv/rvv/autovec/unop/popcount.c| 2 +- >>> gcc/tree-vect-stmts.cc | 13 - >>> 6 files changed, 6 insertions(+), 21 deletions(-) >>> >>> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc >>> index 61d5a9e4772..17c0f4c3805 100644 >>> --- a/gcc/internal-fn.cc >>> +++ b/gcc/internal-fn.cc >>> @@ -281,7 +281,8 @@ expand_fn_using_insn (gcall *stmt, insn_code icode, >>> unsigned int noutputs, >>>emit_move_insn (lhs_rtx, ops[0].value); >>> else >>>{ >>> - gcc_checking_assert (INTEGRAL_TYPE_P (TREE_TYPE (lhs))); >>> + gcc_checking_assert (INTEGRAL_TYPE_P (TREE_TYPE (lhs)) >>> + || VECTOR_INTEGER_TY
RE: [PATCH v2] VECT: Remove the type size restriction of vectorizer
Thanks Richard S for comments. > In other words, I don't think simply removing the test from the vectoriser > is correct. It needs to be replaced by something more selective. Does it mean we need to check if the internal fun allow different modes/sizes here? For example, standard name lrintmn2 (m, n mode) is allowed here, while rintm2 (only m mode) isn't. Pan -Original Message- From: Richard Sandiford Sent: Friday, October 27, 2023 1:47 AM To: Richard Biener Cc: Li, Pan2 ; gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang, Yanzhang ; kito.ch...@gmail.com; Liu, Hongtao Subject: Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer Richard Biener writes: >> Am 26.10.2023 um 13:59 schrieb Li, Pan2 : >> >> Thanks Richard for comments. >> >>> Can you explain why this is necessary? In particular what is lhs_rtx >>> mode vs ops[0].value mode? >> >> For testcase gcc.target/aarch64/sve/popcount_1.c, the rtl are list as below. >> >> The lhs_rtx is (reg:VNx2SI 98 [ vect__5.36 ]). >> The ops[0].value is (reg:VNx2DI 104). >> >> The restriction removing make the vector rtl enter expand_fn_using_insn and >> of course hit the INTEGER_P assertion. > > But I think this shows we mid-selected the optab, a convert_move is certainly > not correct unconditionally here (the target might not support that) Agreed. Allowing TYPE_SIZE (vectype_in) != TYPE_SIZE (vectype_out) makes sense if the called function allows the input and output modes to vary. That's true for internal functions that eventually map to two-mode optabs. But we can't remove the condition for calls to other functions, at least not without some fix-ups. ISTM that the problem being hit is the one described by the removed comment. In other words, I don't think simply removing the test from the vectoriser is correct. It needs to be replaced by something more selective. Thanks, Richard >> Pan >> >> -Original Message- >> From: Richard Biener >> Sent: Thursday, October 26, 2023 4:38 PM >> To: Li, Pan2 >> Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang, Yanzhang >> ; kito.ch...@gmail.com; Liu, Hongtao >> ; Richard Sandiford >> Subject: Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer >> >>> On Thu, Oct 26, 2023 at 4:18 AM wrote: >>> >>> From: Pan Li >>> >>> Update in v2: >>> >>> * Fix one ICE of type assertion. >>> * Adjust some test cases for aarch64 sve and riscv vector. >>> >>> Original log: >>> >>> The vectoriable_call has one restriction of the size of data type. >>> Aka DF to DI is allowed but SF to DI isn't. You may see below message >>> when try to vectorize function call like lrintf. >>> >>> void >>> test_lrintf (long *out, float *in, unsigned count) >>> { >>> for (unsigned i = 0; i < count; i++) >>>out[i] = __builtin_lrintf (in[i]); >>> } >>> >>> lrintf.c:5:26: missed: couldn't vectorize loop >>> lrintf.c:5:26: missed: not vectorized: unsupported data-type >>> >>> Then the standard name pattern like lrintmn2 cannot work for different >>> data type size like SF => DI. This patch would like to remove this data >>> type size check and unblock the standard name like lrintmn2. >>> >>> The below test are passed for this patch. >>> >>> * The x86 bootstrap and regression test. >>> * The aarch64 regression test. >>> * The risc-v regression tests. >>> >>> gcc/ChangeLog: >>> >>>* internal-fn.cc (expand_fn_using_insn): Add vector int assertion. >>>* tree-vect-stmts.cc (vectorizable_call): Remove size check. >>> >>> gcc/testsuite/ChangeLog: >>> >>>* gcc.target/aarch64/sve/clrsb_1.c: Adjust checker. >>>* gcc.target/aarch64/sve/clz_1.c: Ditto. >>>* gcc.target/aarch64/sve/popcount_1.c: Ditto. >>>* gcc.target/riscv/rvv/autovec/unop/popcount.c: Ditto. >>> >>> Signed-off-by: Pan Li >>> --- >>> gcc/internal-fn.cc | 3 ++- >>> gcc/testsuite/gcc.target/aarch64/sve/clrsb_1.c | 3 +-- >>> gcc/testsuite/gcc.target/aarch64/sve/clz_1.c| 3 +-- >>> gcc/testsuite/gcc.target/aarch64/sve/popcount_1.c | 3 +-- >>> .../gcc.target/riscv/rvv/autovec/unop/popcount.c| 2 +- >>> gcc/tree-vect-stmts.cc | 13 - >>> 6 fi
Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer
On Fri, Oct 27, 2023 at 4:17 AM Li, Pan2 wrote: > > Thanks Richard S for comments. > > > In other words, I don't think simply removing the test from the vectoriser > > is correct. It needs to be replaced by something more selective. > > Does it mean we need to check if the internal fun allow different modes/sizes > here? > > For example, standard name lrintmn2 (m, n mode) is allowed here, while rintm2 > (only m mode) isn't. We need to check whether the "size" of the LHS is somehow participating in the optab query. I think the direct_internal_fn_info type0/1 members, when -1 say that. If none is -1 (and -2) then the LHS has to match one of the arguments (if there are two different I'm not sure which we'd pick). So patch-wise the existing check can probably be skipped when vectorizable_internal_function returns an IFN but that function should have the very same check when vectype_out isn't participating in the optab selection. Richard. > > Pan > > -Original Message- > From: Richard Sandiford > Sent: Friday, October 27, 2023 1:47 AM > To: Richard Biener > Cc: Li, Pan2 ; gcc-patches@gcc.gnu.org; > juzhe.zh...@rivai.ai; Wang, Yanzhang ; > kito.ch...@gmail.com; Liu, Hongtao > Subject: Re: [PATCH v2] VECT: Remove the type size restriction of vectorizer > > Richard Biener writes: > >> Am 26.10.2023 um 13:59 schrieb Li, Pan2 : > >> > >> Thanks Richard for comments. > >> > >>> Can you explain why this is necessary? In particular what is lhs_rtx > >>> mode vs ops[0].value mode? > >> > >> For testcase gcc.target/aarch64/sve/popcount_1.c, the rtl are list as > >> below. > >> > >> The lhs_rtx is (reg:VNx2SI 98 [ vect__5.36 ]). > >> The ops[0].value is (reg:VNx2DI 104). > >> > >> The restriction removing make the vector rtl enter expand_fn_using_insn > >> and of course hit the INTEGER_P assertion. > > > > But I think this shows we mid-selected the optab, a convert_move is > > certainly not correct unconditionally here (the target might not support > > that) > > Agreed. Allowing TYPE_SIZE (vectype_in) != TYPE_SIZE (vectype_out) > makes sense if the called function allows the input and output modes > to vary. That's true for internal functions that eventually map to > two-mode optabs. But we can't remove the condition for calls to > other functions, at least not without some fix-ups. > > ISTM that the problem being hit is the one described by the removed > comment. > > In other words, I don't think simply removing the test from the vectoriser > is correct. It needs to be replaced by something more selective. > > Thanks, > Richard > > >> Pan > >> > >> -Original Message- > >> From: Richard Biener > >> Sent: Thursday, October 26, 2023 4:38 PM > >> To: Li, Pan2 > >> Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang, Yanzhang > >> ; kito.ch...@gmail.com; Liu, Hongtao > >> ; Richard Sandiford > >> Subject: Re: [PATCH v2] VECT: Remove the type size restriction of > >> vectorizer > >> > >>> On Thu, Oct 26, 2023 at 4:18 AM wrote: > >>> > >>> From: Pan Li > >>> > >>> Update in v2: > >>> > >>> * Fix one ICE of type assertion. > >>> * Adjust some test cases for aarch64 sve and riscv vector. > >>> > >>> Original log: > >>> > >>> The vectoriable_call has one restriction of the size of data type. > >>> Aka DF to DI is allowed but SF to DI isn't. You may see below message > >>> when try to vectorize function call like lrintf. > >>> > >>> void > >>> test_lrintf (long *out, float *in, unsigned count) > >>> { > >>> for (unsigned i = 0; i < count; i++) > >>>out[i] = __builtin_lrintf (in[i]); > >>> } > >>> > >>> lrintf.c:5:26: missed: couldn't vectorize loop > >>> lrintf.c:5:26: missed: not vectorized: unsupported data-type > >>> > >>> Then the standard name pattern like lrintmn2 cannot work for different > >>> data type size like SF => DI. This patch would like to remove this data > >>> type size check and unblock the standard name like lrintmn2. > >>> > >>> The below test are passed for this patch. > >>> > >>> * The x86 bootstrap and regression test. > >>> * The aarch64 regression test. > >>> * The risc-v regression tests. &