Ping^2: [PATCH v2] rs6000: Convert the vector element register to SImode [PR98914]

[Xionghu Luo]

On 2021/3/17 15:53, Jakub Jelinek wrote:
> On Wed, Mar 17, 2021 at 11:35:18AM +0800, Xionghu Luo wrote:
>> +  machine_mode idx_mode = GET_MODE (idx);
>> +  if (idx_mode != DImode)
>> +    idx = convert_modes (DImode, idx_mode, idx, 1);
> 
> Segher mentioned you can remove the if (idx_mode != DImode) too,
> convert_modes has an early if (mode == oldmode) return x;

OK.

> 
>> +
>> +  /*  idx = idx * width.  */
>> +  rtx tmp = gen_reg_rtx (DImode);
>> +  emit_insn (gen_muldi3 (tmp, idx, GEN_INT (width)));
> 
> And I have to wonder, when rs6000_expand_vector_set_var_p9
> uses here a left shift, can't rs6000_expand_vector_set_var_p8
> do the same or is there some reason I'm missing?
> gen_muldi3 will just emit a multiplication, are you hoping that
> combiner or some other pass will optimize it away later (for width 1)
> or change the multiplication into left shift otherwise?
> 
Yes, should also use shift here to avoid multiplication, thanks for the catch.
Attached updated patch.


Xionghu
-------------- next part --------------
From 693b40d5f521c9a28e9923bea6d65bf71afa8c01 Mon Sep 17 00:00:00 2001
From: Xionghu Luo <luoxhu@linux.ibm.com>
Date: Tue, 16 Mar 2021 21:23:14 -0500
Subject: [PATCH] rs6000: Convert the vector set variable idx to DImode
 [PR98914]

vec_insert defines the element argument type to be signed int by ELFv2
ABI.  When expanding a vector with a variable rtx, convert the rtx type
to DImode to support both intrinsic usage and other callers from
rs6000_expand_vector_init produced by v[k] = val when k is long type.

gcc/ChangeLog:

2021-03-18  Xionghu Luo  <luoxhu@linux.ibm.com>

	PR target/98914
	* config/rs6000/rs6000.c (rs6000_expand_vector_set_var_p9):
	Convert idx to DImode.
	(rs6000_expand_vector_set_var_p8): Likewise.

gcc/testsuite/ChangeLog:

2021-03-18  Xionghu Luo  <luoxhu@linux.ibm.com>

	PR target/98914
	* gcc.target/powerpc/pr98914.c: New test.
---
 gcc/config/rs6000/rs6000.c                 | 39 ++++++++++------------
 gcc/testsuite/gcc.target/powerpc/pr98914.c | 11 ++++++
 2 files changed, 29 insertions(+), 21 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..d5b8a0e00d1 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -7000,21 +7000,21 @@ 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));
   int width = GET_MODE_SIZE (inner_mode);
 
   gcc_assert (width >= 1 && width <= 8);
 
   int shift = exact_log2 (width);
+
+  machine_mode idx_mode = GET_MODE (idx);
+  idx = convert_modes (DImode, idx_mode, idx, 1);
+
   /* Generate the IDX for permute shift, width is the vector element size.
      idx = idx * width.  */
-  emit_insn (gen_ashlsi3 (tmp, idx, GEN_INT (shift)));
-
-  tmp = convert_modes (DImode, SImode, tmp, 1);
+  rtx tmp = gen_reg_rtx (DImode);
+  emit_insn (gen_ashldi3 (tmp, idx, GEN_INT (shift)));
 
   /*  lvsr    v1,0,idx.  */
   rtx pcvr = gen_reg_rtx (V16QImode);
@@ -7047,28 +7047,26 @@ 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);
 
-  rtx tmp = gen_reg_rtx (GET_MODE (idx));
   int width = GET_MODE_SIZE (inner_mode);
-
   gcc_assert (width >= 1 && width <= 4);
 
+  int shift = exact_log2 (width);
+
+  machine_mode idx_mode = GET_MODE (idx);
+  idx = convert_modes (DImode, idx_mode, idx, 1);
+
+  /*  idx = idx * width.  */
+  rtx tmp = gen_reg_rtx (DImode);
+  emit_insn (gen_ashldi3 (tmp, idx, GEN_INT (shift)));
+
+  /*  For LE:  idx = idx + 8.  */
   if (!BYTES_BIG_ENDIAN)
-    {
-      /*  idx = idx * width.  */
-      emit_insn (gen_mulsi3 (tmp, idx, GEN_INT (width)));
-      /*  idx = idx + 8.  */
-      emit_insn (gen_addsi3 (tmp, tmp, GEN_INT (8)));
-    }
+    emit_insn (gen_adddi3 (tmp, tmp, GEN_INT (8)));
   else
-    {
-      emit_insn (gen_mulsi3 (tmp, idx, GEN_INT (width)));
-      emit_insn (gen_subsi3 (tmp, GEN_INT (24 - width), tmp));
-    }
+    emit_insn (gen_subdi3 (tmp, GEN_INT (24 - width), tmp));
 
   /*  lxv vs33, mask.
       DImode: 0xffffffffffffffff0000000000000000
@@ -7118,7 +7116,6 @@ rs6000_expand_vector_set_var_p8 (rtx target, rtx val, rtx idx)
   emit_insn (gen_rtx_SET (val_v16qi, sub_val));
 
   /*  lvsl    13,0,idx.  */
-  tmp = convert_modes (DImode, SImode, tmp, 1);
   rtx pcv = gen_reg_rtx (V16QImode);
   emit_insn (gen_altivec_lvsl_reg (pcv, tmp));
 
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;
+}
-- 
2.25.1