Re: [PATCH ver 3] rs6000, fix vec_replace_unaligned built-in arguments
On Thu, 2023-07-13 at 17:41 +0800, Kewen.Lin wrote: > Hi Carl, > > on 2023/7/8 04:18, Carl Love wrote: > > GCC maintainers: > > > > Version 3, added code to altivec_resolve_overloaded_builtin so the > > correct instruction is selected for the size of the second > > argument. > > This restores the instruction counts to the original values where > > the > > correct instructions were originally being generated. The naming > > of > > Nice, I have some comments inlined below. > > > the overloaded builtin instances and builtin definitions were > > changed > > to reflect the type of the second argument since the type of the > > first > > argument is now the same for all overloaded instances. A new > > builtin > > test file was added for the case where the first argument is cast > > to > > the unsigned long long type. This test requires the -flax-vector- > > conversions gcc command line option. Since the other tests do not > > require this option, I felt that the new test needed to be in a > > separate file. Finally some formatting fixes were made in the > > original > > test file. Patch has been retested on Power 10 with no > > regressions. > > > > Version 2, fixed various typos. Updated the change log body to say > > the > > instruction counts were updated. The instruction counts changed as > > a > > result of changing the first argument of the vec_replace_unaligned > > builtin call from vector unsigned long long (vull) to vector > > unsigned > > char (vuc). When the first argument was vull the builtin call > > generated the vinsd instruction for the two test cases. The > > updated > > call with vuc as the first argument generates two vinsw > > instructions > > instead. Patch was retested on Power 10 with no regressions. > > > > The following patch fixes the first argument in the builtin > > definition > > and the corresponding test cases. Initially, the builtin > > specification > > was wrong due to a cut and past error. The documentation was fixed > > in: > > > >commit ed3fea09b18f67e757b5768b42cb6e816626f1db > >Author: Bill Schmidt > >Date: Fri Feb 4 13:07:17 2022 -0600 > > > >rs6000: Correct function prototypes for > > vec_replace_unaligned > > > >Due to a pasto error in the documentation, > > vec_replace_unaligned > > was > >implemented with the same function prototypes as > > vec_replace_elt. It was > >intended that vec_replace_unaligned always specify output > > vectors as having > >type vector unsigned char, to emphasize that elements are > > potentially > >misaligned by this built-in function. This patch corrects > > the > >misimplementation. > > > > > > This patch fixes the arguments in the definitions and updates the > > testcases accordingly. Additionally, a few minor spacing issues > > are > > fixed. > > > > The patch has been tested on Power 10 with no regressions. Please > > let > > me know if the patch is acceptable for mainline. Thanks. > > > > Carl > > > > -- > > rs6000, fix vec_replace_unaligned built-in arguments > > > > The first argument of the vec_replace_unaligned built-in should > > always be > > unsigned char, as specified in gcc/doc/extend.texi. > > Maybe "be with type vector unsigned char"? Changed to The first argument of the vec_replace_unaligned built-in should always be of type unsigned char, > > > This patch fixes the builtin definitions and updates the test cases > > to use > > the correct arguments. The original test file is renamed and a > > second test > > file is added for a new test case. > > > > gcc/ChangeLog: > > * config/rs6000/rs6000-builtins.def: Rename > > __builtin_altivec_vreplace_un_uv2di as > > __builtin_altivec_vreplace_un_udi > > __builtin_altivec_vreplace_un_uv4si as > > __builtin_altivec_vreplace_un_usi > > __builtin_altivec_vreplace_un_v2df as > > __builtin_altivec_vreplace_un_df > > __builtin_altivec_vreplace_un_v2di as > > __builtin_altivec_vreplace_un_di > > __builtin_altivec_vreplace_un_v4sf as > > __builtin_altivec_vreplace_un_sf > > __builtin_altivec_vreplace_un_v4si as > > __builtin_altivec_vreplace_un_si. > > Rename VREPLACE_UN_UV2DI as VREPLACE_UN_UDI, VREPLACE_UN_UV4SI > > as > > VREPLACE_UN_USI, VREPLACE_UN_V2DF as VREPLACE_UN_DF, > > VREPLACE_UN_V2DI as VREPLACE_UN_DI, VREPLACE_UN_V4SF as > > VREPLACE_UN_SF, VREPLACE_UN_V4SI as VREPLACE_UN_SI. > > Rename vreplace_un_v2di as vreplace_un_di, vreplace_un_v4si as > > vreplace_un_si, vreplace_un_v2df as vreplace_un_df, > > vreplace_un_v2di as vreplace_un_di, vreplace_un_v4sf as > > vreplace_un_sf, vreplace_un_v4si as vreplace_un_si. > > * config/rs6000/rs6000-c.cc (find_instance): Add new argument > > nargs. Add nargs check. Extend function to handle three > > arguments. > > (altivec_resolve_overloaded_builtin): Add new argument nargs to > > function calls. Add
Re: [PATCH ver 3] rs6000, fix vec_replace_unaligned built-in arguments
Hi Carl, on 2023/7/8 04:18, Carl Love wrote: > > GCC maintainers: > > Version 3, added code to altivec_resolve_overloaded_builtin so the > correct instruction is selected for the size of the second argument. > This restores the instruction counts to the original values where the > correct instructions were originally being generated. The naming of Nice, I have some comments inlined below. > the overloaded builtin instances and builtin definitions were changed > to reflect the type of the second argument since the type of the first > argument is now the same for all overloaded instances. A new builtin > test file was added for the case where the first argument is cast to > the unsigned long long type. This test requires the -flax-vector- > conversions gcc command line option. Since the other tests do not > require this option, I felt that the new test needed to be in a > separate file. Finally some formatting fixes were made in the original > test file. Patch has been retested on Power 10 with no regressions. > > Version 2, fixed various typos. Updated the change log body to say the > instruction counts were updated. The instruction counts changed as a > result of changing the first argument of the vec_replace_unaligned > builtin call from vector unsigned long long (vull) to vector unsigned > char (vuc). When the first argument was vull the builtin call > generated the vinsd instruction for the two test cases. The updated > call with vuc as the first argument generates two vinsw instructions > instead. Patch was retested on Power 10 with no regressions. > > The following patch fixes the first argument in the builtin definition > and the corresponding test cases. Initially, the builtin specification > was wrong due to a cut and past error. The documentation was fixed in: > >commit ed3fea09b18f67e757b5768b42cb6e816626f1db >Author: Bill Schmidt >Date: Fri Feb 4 13:07:17 2022 -0600 > >rs6000: Correct function prototypes for vec_replace_unaligned > >Due to a pasto error in the documentation, vec_replace_unaligned > was >implemented with the same function prototypes as > vec_replace_elt. It was >intended that vec_replace_unaligned always specify output > vectors as having >type vector unsigned char, to emphasize that elements are > potentially >misaligned by this built-in function. This patch corrects the >misimplementation. > > > This patch fixes the arguments in the definitions and updates the > testcases accordingly. Additionally, a few minor spacing issues are > fixed. > > The patch has been tested on Power 10 with no regressions. Please let > me know if the patch is acceptable for mainline. Thanks. > > Carl > > -- > rs6000, fix vec_replace_unaligned built-in arguments > > The first argument of the vec_replace_unaligned built-in should always be > unsigned char, as specified in gcc/doc/extend.texi. Maybe "be with type vector unsigned char"? > > This patch fixes the builtin definitions and updates the test cases to use > the correct arguments. The original test file is renamed and a second test > file is added for a new test case. > > gcc/ChangeLog: > * config/rs6000/rs6000-builtins.def: Rename > __builtin_altivec_vreplace_un_uv2di as __builtin_altivec_vreplace_un_udi > __builtin_altivec_vreplace_un_uv4si as __builtin_altivec_vreplace_un_usi > __builtin_altivec_vreplace_un_v2df as __builtin_altivec_vreplace_un_df > __builtin_altivec_vreplace_un_v2di as __builtin_altivec_vreplace_un_di > __builtin_altivec_vreplace_un_v4sf as __builtin_altivec_vreplace_un_sf > __builtin_altivec_vreplace_un_v4si as __builtin_altivec_vreplace_un_si. > Rename VREPLACE_UN_UV2DI as VREPLACE_UN_UDI, VREPLACE_UN_UV4SI as > VREPLACE_UN_USI, VREPLACE_UN_V2DF as VREPLACE_UN_DF, > VREPLACE_UN_V2DI as VREPLACE_UN_DI, VREPLACE_UN_V4SF as > VREPLACE_UN_SF, VREPLACE_UN_V4SI as VREPLACE_UN_SI. > Rename vreplace_un_v2di as vreplace_un_di, vreplace_un_v4si as > vreplace_un_si, vreplace_un_v2df as vreplace_un_df, > vreplace_un_v2di as vreplace_un_di, vreplace_un_v4sf as > vreplace_un_sf, vreplace_un_v4si as vreplace_un_si. > * config/rs6000/rs6000-c.cc (find_instance): Add new argument > nargs. Add nargs check. Extend function to handle three arguments. > (altivec_resolve_overloaded_builtin): Add new argument nargs to > function calls. Add case RS6000_OVLD_VEC_REPLACE_UN. > * config/rs6000/rs6000-overload.def (__builtin_vec_replace_un): > Fix first argument type. Rename VREPLACE_UN_UV4SI as > VREPLACE_UN_USI, VREPLACE_UN_V4SI as VREPLACE_UN_SI, > VREPLACE_UN_UV2DI as VREPLACE_UN_UDI, VREPLACE_UN_V2DI as > VREPLACE_UN_DI, VREPLACE_UN_V4SF as VREPLACE_UN_SF, > VREPLACE_UN_V2DF as VREPLACE_UN_DF. > * config/rs6000/vsx.md (VEC_RU): New mode
[PATCH ver 3] rs6000, fix vec_replace_unaligned built-in arguments
GCC maintainers: Version 3, added code to altivec_resolve_overloaded_builtin so the correct instruction is selected for the size of the second argument. This restores the instruction counts to the original values where the correct instructions were originally being generated. The naming of the overloaded builtin instances and builtin definitions were changed to reflect the type of the second argument since the type of the first argument is now the same for all overloaded instances. A new builtin test file was added for the case where the first argument is cast to the unsigned long long type. This test requires the -flax-vector- conversions gcc command line option. Since the other tests do not require this option, I felt that the new test needed to be in a separate file. Finally some formatting fixes were made in the original test file. Patch has been retested on Power 10 with no regressions. Version 2, fixed various typos. Updated the change log body to say the instruction counts were updated. The instruction counts changed as a result of changing the first argument of the vec_replace_unaligned builtin call from vector unsigned long long (vull) to vector unsigned char (vuc). When the first argument was vull the builtin call generated the vinsd instruction for the two test cases. The updated call with vuc as the first argument generates two vinsw instructions instead. Patch was retested on Power 10 with no regressions. The following patch fixes the first argument in the builtin definition and the corresponding test cases. Initially, the builtin specification was wrong due to a cut and past error. The documentation was fixed in: commit ed3fea09b18f67e757b5768b42cb6e816626f1db Author: Bill Schmidt Date: Fri Feb 4 13:07:17 2022 -0600 rs6000: Correct function prototypes for vec_replace_unaligned Due to a pasto error in the documentation, vec_replace_unaligned was implemented with the same function prototypes as vec_replace_elt. It was intended that vec_replace_unaligned always specify output vectors as having type vector unsigned char, to emphasize that elements are potentially misaligned by this built-in function. This patch corrects the misimplementation. This patch fixes the arguments in the definitions and updates the testcases accordingly. Additionally, a few minor spacing issues are fixed. The patch has been tested on Power 10 with no regressions. Please let me know if the patch is acceptable for mainline. Thanks. Carl -- rs6000, fix vec_replace_unaligned built-in arguments The first argument of the vec_replace_unaligned built-in should always be unsigned char, as specified in gcc/doc/extend.texi. This patch fixes the builtin definitions and updates the test cases to use the correct arguments. The original test file is renamed and a second test file is added for a new test case. gcc/ChangeLog: * config/rs6000/rs6000-builtins.def: Rename __builtin_altivec_vreplace_un_uv2di as __builtin_altivec_vreplace_un_udi __builtin_altivec_vreplace_un_uv4si as __builtin_altivec_vreplace_un_usi __builtin_altivec_vreplace_un_v2df as __builtin_altivec_vreplace_un_df __builtin_altivec_vreplace_un_v2di as __builtin_altivec_vreplace_un_di __builtin_altivec_vreplace_un_v4sf as __builtin_altivec_vreplace_un_sf __builtin_altivec_vreplace_un_v4si as __builtin_altivec_vreplace_un_si. Rename VREPLACE_UN_UV2DI as VREPLACE_UN_UDI, VREPLACE_UN_UV4SI as VREPLACE_UN_USI, VREPLACE_UN_V2DF as VREPLACE_UN_DF, VREPLACE_UN_V2DI as VREPLACE_UN_DI, VREPLACE_UN_V4SF as VREPLACE_UN_SF, VREPLACE_UN_V4SI as VREPLACE_UN_SI. Rename vreplace_un_v2di as vreplace_un_di, vreplace_un_v4si as vreplace_un_si, vreplace_un_v2df as vreplace_un_df, vreplace_un_v2di as vreplace_un_di, vreplace_un_v4sf as vreplace_un_sf, vreplace_un_v4si as vreplace_un_si. * config/rs6000/rs6000-c.cc (find_instance): Add new argument nargs. Add nargs check. Extend function to handle three arguments. (altivec_resolve_overloaded_builtin): Add new argument nargs to function calls. Add case RS6000_OVLD_VEC_REPLACE_UN. * config/rs6000/rs6000-overload.def (__builtin_vec_replace_un): Fix first argument type. Rename VREPLACE_UN_UV4SI as VREPLACE_UN_USI, VREPLACE_UN_V4SI as VREPLACE_UN_SI, VREPLACE_UN_UV2DI as VREPLACE_UN_UDI, VREPLACE_UN_V2DI as VREPLACE_UN_DI, VREPLACE_UN_V4SF as VREPLACE_UN_SF, VREPLACE_UN_V2DF as VREPLACE_UN_DF. * config/rs6000/vsx.md (VEC_RU): New mode iterator. (VEC_RU_char): New mode attribute. (vreplace_un_): Change iterator and mode attribute. gcc/testsuite/ChangeLog: * gcc.target/powerpc/vec-replace-word-runnable.c: Renamed vec-replace-word-runnable_1.c. *
