On Fri, 2020-05-01 at 09:42 -0700, Carl Love via Gcc-patches wrote:
> GCC maintainers:
>
Hi,
subject: Re: [PATCH] rs6000, fix vec_first_match_index for nulls
^ See if you can include the pr94833 string in the subject somewhere.
> The following patch fixes PR94833, vec_first_match_index does not
> function as described in its description.
>
> The builtin does not handle vector elements which are zero
> correctly.
> The following patch fixes the issue and adds additional test cases to
> verify the vec_first_match_index builtin and related builtins work
> correctly with zero elements.
I wonder if referring to those as zero-valued elements would be better.
Depends if there is any possible confusion with number of elements in a
vector. dunno. :-)
>
> The patch has been compiled and tested on
>
> powerpc64le-unknown-linux-gnu (Power 9 LE)
>
> with no regression errors.
>
> Please let me know if the patch is acceptable for mainline and for
> backporting as needed.
>
> Thanks.
>
> Carl Love
> --------------------------------------------------------------
>
> gcc/ChangeLog
>
> 2020-04-30 Carl Love <c...@us.ibm.com>
>
> PR target/94833
> * config/rs6000/vsx.md (define_expand): Fix instruction
> generation for
> first_match_index_<mode>.
Probably want ("first_match_index_<mode>") instead of (define_expand).
Then "Fix instruction generation." is probably sufficient.
> * testsuite/gcc.target/powerpc/builtins-8-p9-runnable.c (main):
> Add
> additional test cases with zero vector elements.
ok
> ---
> gcc/config/rs6000/vsx.md | 4 +-
> .../powerpc/builtins-8-p9-runnable.c | 118 ++++++++++++++++++
> 2 files changed, 120 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index 1fcc1b03096..12a0d5e668c 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -4803,8 +4803,8 @@
> rtx cmp_result = gen_reg_rtx (<MODE>mode);
> rtx not_result = gen_reg_rtx (<MODE>mode);
>
> - emit_insn (gen_vcmpnez<VSX_EXTRACT_WIDTH> (cmp_result, operands[1],
> - operands[2]));
> + emit_insn (gen_vcmpne<VSX_EXTRACT_WIDTH> (cmp_result, operands[1],
> + operands[2]));
> emit_insn (gen_one_cmpl<mode>2 (not_result, cmp_result));
>
> sh = GET_MODE_SIZE (GET_MODE_INNER (<MODE>mode)) / 2;
ok
> diff --git a/gcc/testsuite/gcc.target/powerpc/builtins-8-p9-runnable.c
> b/gcc/testsuite/gcc.target/powerpc/builtins-8-p9-runnable.c
> index b2f7dc855e8..19457eebfc4 100644
> --- a/gcc/testsuite/gcc.target/powerpc/builtins-8-p9-runnable.c
> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-8-p9-runnable.c
<snip>
>
> @@ -1065,6 +1161,28 @@ int main() {
> The element index in BE order is returned for the first mismatch
> or the number of elements if there is no match. */
> /* char */
> + char_src1 = (vector signed char) {1, 2, 0, 4, -5, 6, 7, 8,
> + 9, 10, 11, 12, 13, 14, 15, 16};
> + char_src2 = (vector signed char) {1, 2, 0, 20, -5, 6, 7, 8,
> + 9, 10, 11, 12, 13, 14, 15, 16};
> + expected_result = 2;
> +
> + result = vec_first_mismatch_or_eos_index (char_src1, char_src2);
> +
> +#ifdef DEBUG2
> + print_signed_char("src1", char_src1);
> + print_signed_char("src2", char_src2);
> + printf("vec_first_mismatch_or_eos_index = %d\n\n", result);
> +#endif
> +
> + if (result != expected_result)
> +#ifdef DEBUG
> + printf("Error: char first mismatch or EOS result (%d) does not match
> expected result (%d)\n",
> + result, expected_result);
Line is a bit long, but i think thats ok for testcases.
Aside from subject/intro/changelog cosmetic bits, patch lgtm.
thanks
-Will
> +#else
> + abort();
> +#endif
> +
> char_src1 = (vector signed char) {-1, 2, 3, 4, -5, 6, 7, 8,
> 9, 10, 11, 12, 13, 14, 15, 16};
> char_src2 = (vector signed char) {-1, 2, 3, 20, -5, 6, 7, 8,
