Re: [PATCH ver 2] rs6000, fix vec_replace_unaligned builtin arguments

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Carl,

on 2023/6/16 00:00, Carl Love wrote:
> GCC maintainers:
> 
> 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 <wschmidt@linux.ibm.com>
>    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 builtin arguments
> 
> The first argument of the vec_replace_unaligned builtin should always be
> unsigned char, as specified in gcc/doc/extend.texi.
> 
> This patch fixes the builtin definitions and updates the testcases to use
> the correct arguments.  The expected instruction counts for the testcase
> are updated.
> 
> gcc/ChangeLog:
> 	* config/rs6000/rs6000-overload.def (__builtin_vec_replace_un):
> 	Fix first argument type.
> 
> gcc/testsuite/ChangeLog:
> 	* gcc.target/powerpc/ver-replace-word-runnable.c

Wrong case name, s/ver/vec/

Sorry that I didn't catch this during the previous review.
How do you generate changelog?  I guess you don't manually type this
but use something like gcc-{commit}-mklog?  that should get this name
right, it's not perfect though. :)

Excepting for the vinsd/vinsw expected insn counts thing, that was
replied in another thread, the others look good to me.

BR,
Kewen