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 <wschm...@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
