Re: [PATCH ver 4] rs6000: Add builtins for IEEE 128-bit floating point values
On Thu, 2023-06-15 at 14:23 +0800, Kewen.Lin wrote: > Hi Carl, > > on 2023/6/15 04:37, Carl Love wrote: > > Kewen, GCC maintainers: > > > > Version 4, added missing cases for new xxexpqp, xsxexpdp and > > xsxsigqp > > cases to rs6000_expand_builtin. Merged the new define_insn > > definitions > > with the existing definitions. Renamed the builtins by removing > > the > > __builtin_ prefix from the names. Fixed the documentation for the > > builtins. Updated the test files to check the desired instructions > > were generated. Retested patch on Power 10 with no regressions. > > > > Version 3, was able to get the overloaded version of > > scalar_insert_exp > > to work and the change to xsxexpqp_f128_ define instruction > > to > > work with the suggestions from Kewen. > > > > Version 2, I have addressed the various comments from Kewen. I had > > issues with adding an additional overloaded version of > > scalar_insert_exp with vector arguments. The overload > > infrastructure > > didn't work with a mix of scalar and vector arguments. I did > > rename > > the __builtin_insertf128_exp to __builtin_vsx_scalar_insert_exp_qp > > make > > it similar to the existing builtin. I also wasn't able to get the > > suggested merge of xsxexpqp_f128_ with xsxexpqp_ to > > work so > > I left the two simpler definitiions. > > > > The patch add three new builtins to extract the significand and > > exponent of an IEEE float 128-bit value where the builtin argument > > is a > > vector. Additionally, a builtin to insert the exponent into an > > IEEE > > float 128-bit vector argument is added. These builtins were > > requested > > since there is no clean and optimal way to transfer between a > > vector > > and a scalar IEEE 128 bit value. > > > > The patch has been tested on Power 10 with no regressions. Please > > let > > me know if the patch is acceptable or not. Thanks. > > I'd suggest you to test this on P9 BE as well to ensure the test case > to work well on BE too. Tested on P9 BE. Updated test cases for the correct expected BE and LE results. > > >Carl > > > > > > > > rs6000: Add builtins for IEEE 128-bit floating point values > > > > Add support for the following builtins: > > > > __vector unsigned long long int scalar_extract_exp_to_vec > > (__ieee128); > > __vector unsigned __int128 scalar_extract_sig_to_vec (__ieee128); > > __ieee128 scalar_insert_exp (__vector unsigned __int128, > > __vector unsigned long long); > > > > These builtins were requesed since there is no clean and performant > > way to > > s/requesed/requested/ Fixed. > > > transfer a value from a vector type and scalar type, despite the > > fact > > Describe it oppositely? As the related existing bifs returns scalar > type, > the users want them in vector type, so it's "from scalar type to > vector > type"? Updated the description. > > > that they both reside in vector registers. > > the fact is the related hardware insns have vsx registers > destination. > > > gcc/ > > * config/rs6000/rs6000-builtin.cc (rs6000_expand_builtin): > > Rename CCDE_FOR_xsxexpqp_tf to CODE_FOR_xsxexpqp_tf_di. > > Rename CODE_FOR_xsxexpqp_kf to CODE_FOR_xsxexpqp_kf_di. > > (CODE_FOR_xsxexpqp_kf_v2di, CODE_FOR_xsxsigqp_kf_v1ti, > > CODE_FOR_xsiexpqp_kf_v2di ): Add case statements. > > unnecessary tab. Fixed. > > > * config/rs6000/rs6000-buildin.def (__builtin_extractf128_exp, > > __builtin_extractf128_sig, __builtin_insertf128_exp): Add new > > builtin definitions. > > Rename xsxexpqp_kf, xsxsigqp_kf, xxsiexpqp_kf to xsexpqp_kf_di, > > typo, xxsiexpqp_kf => xsiexpqp_kf > > > xsxsigqp_kf_ti, xsiexpqp_kf_di respectively. > > * config/rs6000/rs6000-c.cc > > (altivec_resolve_overloaded_builtin): > > Add else if for MODE_VECTOR_INT. Update comments. > > May be better with "Update RS6000_OVLD_VEC_VSIE handling for > MODE_VECTOR_INT > which is used for newly added overloaded instance"? Changed. > > > * config/rs6000/rs6000-overload.def > > (__builtin_vec_scalar_insert_exp): Add new overload definition > > with > > vector arguments. > > (scalar_extract_exp_to_vec, scalar_extract_sig_to_vec): New > > odverloaded definitions. > > s/odverloaded/overloaded/ Fixed. > > > * config/vsx.md (VSEEQP_DI, VSESQP_TI): New mode iterators. > > (VSEEQP_DI_base): New mode attribute definition. > > Rename xsxexpqp_ to > > sxexpqp__. > > Rename xsxsigqp_ to > > xsxsigqp__. > > Rename xsiexpqp_ to > > xsiexpqp__. > > (xsxsigqp_f128_, xsiexpqpf_f128_): Add define_insn > > for > > new builtins. > > * doc/extend.texi (__builtin_extractf128_exp, > > __builtin_extractf128_sig): Add documentation for new builtins. > > (scalar_insert_exp): Add new overloaded builtin definition. > > > > gcc/testsuite/ > > * gcc.target/powerpc/bfp/extract-exp-1.c: New test case.
Re: [PATCH ver 4] rs6000: Add builtins for IEEE 128-bit floating point values
Hi Carl, on 2023/6/15 04:37, Carl Love wrote: > Kewen, GCC maintainers: > > Version 4, added missing cases for new xxexpqp, xsxexpdp and xsxsigqp > cases to rs6000_expand_builtin. Merged the new define_insn definitions > with the existing definitions. Renamed the builtins by removing the > __builtin_ prefix from the names. Fixed the documentation for the > builtins. Updated the test files to check the desired instructions > were generated. Retested patch on Power 10 with no regressions. > > Version 3, was able to get the overloaded version of scalar_insert_exp > to work and the change to xsxexpqp_f128_ define instruction to > work with the suggestions from Kewen. > > Version 2, I have addressed the various comments from Kewen. I had > issues with adding an additional overloaded version of > scalar_insert_exp with vector arguments. The overload infrastructure > didn't work with a mix of scalar and vector arguments. I did rename > the __builtin_insertf128_exp to __builtin_vsx_scalar_insert_exp_qp make > it similar to the existing builtin. I also wasn't able to get the > suggested merge of xsxexpqp_f128_ with xsxexpqp_ to work so > I left the two simpler definitiions. > > The patch add three new builtins to extract the significand and > exponent of an IEEE float 128-bit value where the builtin argument is a > vector. Additionally, a builtin to insert the exponent into an IEEE > float 128-bit vector argument is added. These builtins were requested > since there is no clean and optimal way to transfer between a vector > and a scalar IEEE 128 bit value. > > The patch has been tested on Power 10 with no regressions. Please let > me know if the patch is acceptable or not. Thanks. I'd suggest you to test this on P9 BE as well to ensure the test case to work well on BE too. > >Carl > > > > rs6000: Add builtins for IEEE 128-bit floating point values > > Add support for the following builtins: > > __vector unsigned long long int scalar_extract_exp_to_vec (__ieee128); > __vector unsigned __int128 scalar_extract_sig_to_vec (__ieee128); > __ieee128 scalar_insert_exp (__vector unsigned __int128, > __vector unsigned long long); > > These builtins were requesed since there is no clean and performant way to s/requesed/requested/ > transfer a value from a vector type and scalar type, despite the fact Describe it oppositely? As the related existing bifs returns scalar type, the users want them in vector type, so it's "from scalar type to vector type"? > that they both reside in vector registers. the fact is the related hardware insns have vsx registers destination. > > gcc/ > * config/rs6000/rs6000-builtin.cc (rs6000_expand_builtin): > Rename CCDE_FOR_xsxexpqp_tf to CODE_FOR_xsxexpqp_tf_di. > Rename CODE_FOR_xsxexpqp_kf to CODE_FOR_xsxexpqp_kf_di. > (CODE_FOR_xsxexpqp_kf_v2di, CODE_FOR_xsxsigqp_kf_v1ti, > CODE_FOR_xsiexpqp_kf_v2di ): Add case statements. unnecessary tab. > * config/rs6000/rs6000-buildin.def (__builtin_extractf128_exp, >__builtin_extractf128_sig, __builtin_insertf128_exp): Add new > builtin definitions. > Rename xsxexpqp_kf, xsxsigqp_kf, xxsiexpqp_kf to xsexpqp_kf_di, typo, xxsiexpqp_kf => xsiexpqp_kf > xsxsigqp_kf_ti, xsiexpqp_kf_di respectively. > * config/rs6000/rs6000-c.cc (altivec_resolve_overloaded_builtin): > Add else if for MODE_VECTOR_INT. Update comments. May be better with "Update RS6000_OVLD_VEC_VSIE handling for MODE_VECTOR_INT which is used for newly added overloaded instance"? > * config/rs6000/rs6000-overload.def > (__builtin_vec_scalar_insert_exp): Add new overload definition with > vector arguments. > (scalar_extract_exp_to_vec, scalar_extract_sig_to_vec): New > odverloaded definitions. s/odverloaded/overloaded/ > * config/vsx.md (VSEEQP_DI, VSESQP_TI): New mode iterators. > (VSEEQP_DI_base): New mode attribute definition. > Rename xsxexpqp_ to sxexpqp__. > Rename xsxsigqp_ to xsxsigqp__. > Rename xsiexpqp_ to xsiexpqp__. > (xsxsigqp_f128_, xsiexpqpf_f128_): Add define_insn for > new builtins. > * doc/extend.texi (__builtin_extractf128_exp, > __builtin_extractf128_sig): Add documentation for new builtins. > (scalar_insert_exp): Add new overloaded builtin definition. > > gcc/testsuite/ > * gcc.target/powerpc/bfp/extract-exp-1.c: New test case. > * gcc.target/powerpc/bfp/extract-sig-1.c: New test case. > * gcc.target/powerpc/bfp/insert-exp-1.c: New test case. May be better just with the existing base name and increasing number: extract-exp-1.c => scalar-extract-exp-8.c extract-sig-1.c => scalar-extract-sig-8.c insert-exp-1.c => scalar-insert-exp-16.c > --- > gcc/config/rs6000/rs6000-builtin.cc | 21 +++-- > gcc/config/rs6000/rs6000-builtins.def | 15
[PATCH ver 4] rs6000: Add builtins for IEEE 128-bit floating point values
Kewen, GCC maintainers: Version 4, added missing cases for new xxexpqp, xsxexpdp and xsxsigqp cases to rs6000_expand_builtin. Merged the new define_insn definitions with the existing definitions. Renamed the builtins by removing the __builtin_ prefix from the names. Fixed the documentation for the builtins. Updated the test files to check the desired instructions were generated. Retested patch on Power 10 with no regressions. Version 3, was able to get the overloaded version of scalar_insert_exp to work and the change to xsxexpqp_f128_ define instruction to work with the suggestions from Kewen. Version 2, I have addressed the various comments from Kewen. I had issues with adding an additional overloaded version of scalar_insert_exp with vector arguments. The overload infrastructure didn't work with a mix of scalar and vector arguments. I did rename the __builtin_insertf128_exp to __builtin_vsx_scalar_insert_exp_qp make it similar to the existing builtin. I also wasn't able to get the suggested merge of xsxexpqp_f128_ with xsxexpqp_ to work so I left the two simpler definitiions. The patch add three new builtins to extract the significand and exponent of an IEEE float 128-bit value where the builtin argument is a vector. Additionally, a builtin to insert the exponent into an IEEE float 128-bit vector argument is added. These builtins were requested since there is no clean and optimal way to transfer between a vector and a scalar IEEE 128 bit value. The patch has been tested on Power 10 with no regressions. Please let me know if the patch is acceptable or not. Thanks. Carl rs6000: Add builtins for IEEE 128-bit floating point values Add support for the following builtins: __vector unsigned long long int scalar_extract_exp_to_vec (__ieee128); __vector unsigned __int128 scalar_extract_sig_to_vec (__ieee128); __ieee128 scalar_insert_exp (__vector unsigned __int128, __vector unsigned long long); These builtins were requesed since there is no clean and performant way to transfer a value from a vector type and scalar type, despite the fact that they both reside in vector registers. gcc/ * config/rs6000/rs6000-builtin.cc (rs6000_expand_builtin): Rename CCDE_FOR_xsxexpqp_tf to CODE_FOR_xsxexpqp_tf_di. Rename CODE_FOR_xsxexpqp_kf to CODE_FOR_xsxexpqp_kf_di. (CODE_FOR_xsxexpqp_kf_v2di, CODE_FOR_xsxsigqp_kf_v1ti, CODE_FOR_xsiexpqp_kf_v2di ): Add case statements. * config/rs6000/rs6000-buildin.def (__builtin_extractf128_exp, __builtin_extractf128_sig, __builtin_insertf128_exp): Add new builtin definitions. Rename xsxexpqp_kf, xsxsigqp_kf, xxsiexpqp_kf to xsexpqp_kf_di, xsxsigqp_kf_ti, xsiexpqp_kf_di respectively. * config/rs6000/rs6000-c.cc (altivec_resolve_overloaded_builtin): Add else if for MODE_VECTOR_INT. Update comments. * config/rs6000/rs6000-overload.def (__builtin_vec_scalar_insert_exp): Add new overload definition with vector arguments. (scalar_extract_exp_to_vec, scalar_extract_sig_to_vec): New odverloaded definitions. * config/vsx.md (VSEEQP_DI, VSESQP_TI): New mode iterators. (VSEEQP_DI_base): New mode attribute definition. Rename xsxexpqp_ to sxexpqp__. Rename xsxsigqp_ to xsxsigqp__. Rename xsiexpqp_ to xsiexpqp__. (xsxsigqp_f128_, xsiexpqpf_f128_): Add define_insn for new builtins. * doc/extend.texi (__builtin_extractf128_exp, __builtin_extractf128_sig): Add documentation for new builtins. (scalar_insert_exp): Add new overloaded builtin definition. gcc/testsuite/ * gcc.target/powerpc/bfp/extract-exp-1.c: New test case. * gcc.target/powerpc/bfp/extract-sig-1.c: New test case. * gcc.target/powerpc/bfp/insert-exp-1.c: New test case. --- gcc/config/rs6000/rs6000-builtin.cc | 21 +++-- gcc/config/rs6000/rs6000-builtins.def | 15 ++- gcc/config/rs6000/rs6000-c.cc | 10 +- gcc/config/rs6000/rs6000-overload.def | 10 ++ gcc/config/rs6000/vsx.md | 26 +++-- gcc/doc/extend.texi | 21 - .../gcc.target/powerpc/bfp/extract-exp-1.c| 53 +++ .../gcc.target/powerpc/bfp/extract-sig-1.c| 60 .../gcc.target/powerpc/bfp/insert-exp-1.c | 94 +++ 9 files changed, 284 insertions(+), 26 deletions(-) create mode 100644 gcc/testsuite/gcc.target/powerpc/bfp/extract-exp-1.c create mode 100644 gcc/testsuite/gcc.target/powerpc/bfp/extract-sig-1.c create mode 100644 gcc/testsuite/gcc.target/powerpc/bfp/insert-exp-1.c diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc index 534698e7d3e..a8f291c6a72 100644 --- a/gcc/config/rs6000/rs6000-builtin.cc +++ b/gcc/config/rs6000/rs6000-builtin.cc @@
