Re: [PATCH ver 6] rs6000: Add builtins for IEEE 128-bit floating point values
Hi Carl, on 2023/6/20 02:54, Carl Love wrote: > > Kewen, GCC maintainers: > > Version 6, Fixed missing change log entry. Changed builtin id names as > requested. Missed making the change on the last version. Fixed > comment in the three test cases. Reran regression suite on Power 10, > no regressions. > > Version 5, Tested the patch on P9 BE per request. Fixed up test case > to get the correct expected values for BE and LE. Fixed typos. > Updated the doc/extend.texi to clarify the vector arguments. Changed > test file names per request. Moved builtin defs next to related > definitions. Renamed new mode_attr. Removed new mode_iterator, used > existing iterator instead. Renamed mode_iterator VSEEQP_DI to V2DI_DI. > Fixed up overloaded definitions per request. > > 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 9 BE and Power 10 LE with no > regressions. Please let me know if the patch is acceptable or not. > Thanks. OK for trunk with some nits fixed in changelog (sorry that I didn't catch all of them in previous review, but I don't think you need to post a new version). 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); > > The instructions used in the builtins operate on vector registers. Thus > the result must be moved to a scalar type. There is no clean, performant > way to do this. The user code typically needs the result as a vector > anyway. > > 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. Miss "Rename CODE_FOR_xsxsigqp_tf to CODE_FOR_xsxsigqp_tf_ti." "Rename CODE_FOR_xsxsigqp_kf to CODE_FOR_xsxsigqp_kf_ti." "Rename CODE_FOR_xsiexpqp_tf to CODE_FOR_xsiexpqp_tf_di." "Rename CODE_FOR_xsiexpqp_kf to CODE_FOR_xsiexpqp_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. Should be with correct names: (__builtin_vsx_scalar_extract_exp_to_vec, __builtin_vsx_scalar_extract_sig_to_vec, __builtin_vsx_scalar_insert_exp_vqp): > Rename xsxexpqp_kf, xsxsigqp_kf, xsiexpqp_kf to xsexpqp_kf_di, > xsxsigqp_kf_ti, xsiexpqp_kf_di respectively. > * config/rs6000/rs6000-c.cc (altivec_resolve_overloaded_builtin): > Update case RS6000_OVLD_VEC_VSIE to handle MODE_VECTOR_INT for new > overloaded instance. 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 > overloaded definitions. > * config/vsx.md (V2DI_DI): New mode iterator. > (DI_to_TI): New mode attribute. > Rename xsxexpqp_ to sxexpqp__. > Rename xsxsigqp_ to xsxsigqp__. >
[PATCH ver 6] rs6000: Add builtins for IEEE 128-bit floating point values
Kewen, GCC maintainers: Version 6, Fixed missing change log entry. Changed builtin id names as requested. Missed making the change on the last version. Fixed comment in the three test cases. Reran regression suite on Power 10, no regressions. Version 5, Tested the patch on P9 BE per request. Fixed up test case to get the correct expected values for BE and LE. Fixed typos. Updated the doc/extend.texi to clarify the vector arguments. Changed test file names per request. Moved builtin defs next to related definitions. Renamed new mode_attr. Removed new mode_iterator, used existing iterator instead. Renamed mode_iterator VSEEQP_DI to V2DI_DI. Fixed up overloaded definitions per request. 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 9 BE and Power 10 LE 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); The instructions used in the builtins operate on vector registers. Thus the result must be moved to a scalar type. There is no clean, performant way to do this. The user code typically needs the result as a vector anyway. 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, xsiexpqp_kf to xsexpqp_kf_di, xsxsigqp_kf_ti, xsiexpqp_kf_di respectively. * config/rs6000/rs6000-c.cc (altivec_resolve_overloaded_builtin): Update case RS6000_OVLD_VEC_VSIE to handle MODE_VECTOR_INT for new overloaded instance. 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 overloaded definitions. * config/vsx.md (V2DI_DI): New mode iterator. (DI_to_TI): New mode attribute. Rename xsxexpqp_ to sxexpqp__. Rename xsxsigqp_ to xsxsigqp__. Rename xsiexpqp_ to xsiexpqp__. * 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-8.c: New test case. * gcc.target/powerpc/bfp/extract-sig-8.c: New test case. * gcc.target/powerpc/bfp/insert-exp-16.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 | 12 ++ gcc/config/rs6000/vsx.md | 25 +++-- gcc/doc/extend.texi | 24 +++-
