On Wed, 2021-02-03 at 03:01 -0600, Xionghu Luo via Gcc-patches wrote:
Hi,
> v[k] will also be expanded to IFN VEC_SET if k is long type when
> built
> with -Og. -O0 didn't exposed the issue due to v is TREE_ADDRESSABLE,
> -O1 and above also didn't capture it because of v[k] is not optimized
> to
> VIEW_CONVERT_EXPR<int[4]>(v)[k_1].
> vec_insert defines the element argument type to be signed int by
> ELFv2
> ABI, so convert it to SImode if it wasn't for Power target
> requirements.
The intro paragraph seems to start mid sentence. Did something get cut
off?
The description here is specific to the reported testcase failure.
This should describe the patch behavior instead. Something like
"When
expanding a vector with a variable rtx, the rtx type needs to be SI"
...
(I defer to any other suggestions of better or improved wording).
>
> gcc/ChangeLog:
Reference "PR target/98914" somewhere in here.
>
> 2021-02-03 Xionghu Luo <luo...@linux.ibm.com>
>
> * config/rs6000/rs6000.c (rs6000_expand_vector_set): Convert
> elt_rtx to SImode if it wasn't.
s/if it wasn't//
>
> gcc/testsuite/ChangeLog:
>
> 2021-02-03 Xionghu Luo <luo...@linux.ibm.com>
>
> * gcc.target/powerpc/pr98914.c: New test.
> ---
> gcc/config/rs6000/rs6000.c | 17 ++++++++++-------
> gcc/testsuite/gcc.target/powerpc/pr98914.c | 11 +++++++++++
> 2 files changed, 21 insertions(+), 7 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/pr98914.c
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index ec068c58aa5..9f7f8da56c6 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -7000,8 +7000,6 @@ rs6000_expand_vector_set_var_p9 (rtx target,
> rtx val, rtx idx)
>
> gcc_assert (VECTOR_MEM_VSX_P (mode) && !CONST_INT_P (idx));
>
> - gcc_assert (GET_MODE (idx) == E_SImode);
> -
> machine_mode inner_mode = GET_MODE (val);
>
> rtx tmp = gen_reg_rtx (GET_MODE (idx));
This needs a changelog blurb.
> @@ -7047,8 +7045,6 @@ rs6000_expand_vector_set_var_p8 (rtx target,
> rtx val, rtx idx)
>
> gcc_assert (VECTOR_MEM_VSX_P (mode) && !CONST_INT_P (idx));
>
> - gcc_assert (GET_MODE (idx) == E_SImode);
> -
> machine_mode inner_mode = GET_MODE (val);
> HOST_WIDE_INT mode_mask = GET_MODE_MASK (inner_mode);
Same.
>
> @@ -7144,7 +7140,7 @@ rs6000_expand_vector_set (rtx target, rtx val,
> rtx elt_rtx)
> machine_mode mode = GET_MODE (target);
> machine_mode inner_mode = GET_MODE_INNER (mode);
> rtx reg = gen_reg_rtx (mode);
> - rtx mask, mem, x;
> + rtx mask, mem, x, elt_si;
> int width = GET_MODE_SIZE (inner_mode);
> int i;
>
> @@ -7154,16 +7150,23 @@ rs6000_expand_vector_set (rtx target, rtx
> val, rtx elt_rtx)
> {
> if (!CONST_INT_P (elt_rtx))
> {
> + /* elt_rtx should be SImode from ELFv2 ABI. */
> + elt_si = gen_reg_rtx (E_SImode);
> + if (GET_MODE (elt_rtx) != E_SImode)
> + convert_move (elt_si, elt_rtx, 0);
> + else
> + elt_si = elt_rtx;
> +
ok.
> /* For V2DI/V2DF, could leverage the P9 version to generate
> xxpermdi
> when elt_rtx is variable. */
> if ((TARGET_P9_VECTOR && TARGET_POWERPC64) || width == 8)
> {
> - rs6000_expand_vector_set_var_p9 (target, val, elt_rtx);
> + rs6000_expand_vector_set_var_p9 (target, val, elt_si);
> return;
> }
> else if (TARGET_P8_VECTOR && TARGET_DIRECT_MOVE_64BIT)
> {
> - rs6000_expand_vector_set_var_p8 (target, val, elt_rtx);
> + rs6000_expand_vector_set_var_p8 (target, val, elt_si);
> return;
> }
> }
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr98914.c
> b/gcc/testsuite/gcc.target/powerpc/pr98914.c
> new file mode 100644
> index 00000000000..e4d78e3e6b3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr98914.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> +/* { dg-options "-Og -mvsx" } */
> +
> +vector int
> +foo (vector int v)
> +{
> + for (long k = 0; k < 1; ++k)
> + v[k] = 0;
> + return v;
> +}
ok
thanks
-Will
