[PATCH 0/2] LoongArch: Fix PR113033 and clean up code

public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed

* [PATCH 0/2] LoongArch: Fix PR113033 and clean up code
@ 2023-12-19  6:59 Xi Ruoyao
  2023-12-19  6:59 ` [PATCH 1/2] LoongArch: Use force_reg instead of gen_reg_rtx + emit_move_insn in vec_init expander [PR113033] Xi Ruoyao
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Xi Ruoyao @ 2023-12-19  6:59 UTC (permalink / raw)
  To: gcc-patches; +Cc: chenglulu, i, xuchenghua, c, Xi Ruoyao

Superseds
https://gcc.gnu.org/pipermail/gcc-patches/2023-December/640871.html.

Per Jakub's response, vec_init patterns do not have a predicate on the
input operand so the operand can be *anything*.  It's not safe to simply
move it into an reg, and we have to use force_reg instead.

The code clean up is separated into the 2nd patch to make reviewing
easier.

Bootstrapped and regtested on loongarch64-linux-gnu.  Ok for trunk?

Xi Ruoyao (2):
  LoongArch: Use force_reg instead of gen_reg_rtx + emit_move_insn in
    vec_init expander [PR113033]
  LoongArch: Clean up vec_init expander

 gcc/config/loongarch/loongarch.cc             | 54 +++++++------------
 gcc/testsuite/gcc.target/loongarch/pr113033.c | 23 ++++++++
 2 files changed, 43 insertions(+), 34 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/loongarch/pr113033.c

-- 
2.43.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] LoongArch: Use force_reg instead of gen_reg_rtx + emit_move_insn in vec_init expander [PR113033]
  2023-12-19  6:59 [PATCH 0/2] LoongArch: Fix PR113033 and clean up code Xi Ruoyao
@ 2023-12-19  6:59 ` Xi Ruoyao
  2023-12-19  6:59 ` [PATCH 2/2] LoongArch: Clean up vec_init expander Xi Ruoyao
  2023-12-19  7:26 ` [PATCH 0/2] LoongArch: Fix PR113033 and clean up code chenglulu
  2 siblings, 0 replies; 5+ messages in thread
From: Xi Ruoyao @ 2023-12-19  6:59 UTC (permalink / raw)
  To: gcc-patches; +Cc: chenglulu, i, xuchenghua, c, Xi Ruoyao

Jakub says:

    Then that seems like a bug in the loongarch vec_init pattern(s).
    Those really don't have a predicate in any of the backends on the
    input operand, so they need to force_reg it if it is something it
    can't handle. I've looked e.g. at i386 vec_init and that is exactly
    what it does, see the various tests + force_reg calls in
    ix86_expand_vector_init*.

So replace gen_reg_rtx + emit_move_insn with force_reg to fix PR 113033.

gcc/ChangeLog:

	PR target/113033
	* config/loongarch/loongarch.cc
	(loongarch_expand_vector_init_same): Replace gen_reg_rtx +
	emit_move_insn with force_reg.
	(loongarch_expand_vector_init): Likewise.

gcc/testsuite/ChangeLog:

	PR target/113033
	* gcc.target/loongarch/pr113033.c: New test.
---
 gcc/config/loongarch/loongarch.cc             | 38 ++++++-------------
 gcc/testsuite/gcc.target/loongarch/pr113033.c | 23 +++++++++++
 2 files changed, 35 insertions(+), 26 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/loongarch/pr113033.c

diff --git a/gcc/config/loongarch/loongarch.cc b/gcc/config/loongarch/loongarch.cc
index 256fa7d048d..ef81414342d 100644
--- a/gcc/config/loongarch/loongarch.cc
+++ b/gcc/config/loongarch/loongarch.cc
@@ -10770,7 +10770,7 @@ loongarch_expand_vector_init_same (rtx target, rtx vals, unsigned nvar)
 	  gcc_unreachable ();
 	}
     }
-  temp = gen_reg_rtx (imode);
+
   if (imode == GET_MODE (same))
     temp2 = same;
   else if (GET_MODE_SIZE (imode) >= UNITS_PER_WORD)
@@ -10795,7 +10795,8 @@ loongarch_expand_vector_init_same (rtx target, rtx vals, unsigned nvar)
       else
 	temp2 = lowpart_subreg (imode, same, GET_MODE (same));
     }
-  emit_move_insn (temp, temp2);
+
+  temp = force_reg (imode, temp2);
 
   switch (vmode)
     {
@@ -11017,35 +11018,29 @@ loongarch_expand_vector_init (rtx target, rtx vals)
 			 to reduce the number of instructions.  */
 		      if (i == 1)
 			{
-			  op0 = gen_reg_rtx (imode);
-			  emit_move_insn (op0, val_hi[0]);
-			  op1 = gen_reg_rtx (imode);
-			  emit_move_insn (op1, val_hi[1]);
+			  op0 = force_reg (imode, val_hi[0]);
+			  op1 = force_reg (imode, val_hi[1]);
 			  emit_insn (
 			    loongarch_vec_repl2_256 (target_hi, op0, op1));
 			}
 		      else if (i > 1)
 			{
-			  op0 = gen_reg_rtx (imode);
-			  emit_move_insn (op0, val_hi[i]);
+			  op0 = force_reg (imode, val_hi[i]);
 			  emit_insn (
 			    loongarch_vec_set256 (target_hi, op0, GEN_INT (i)));
 			}
 		    }
 		  else
 		    {
+		      op0 = force_reg (imode, val_hi[i]);
 		      /* Assign the lowest element of val_hi to all elements
 			 of target_hi.  */
 		      if (i == 0)
 			{
-			  op0 = gen_reg_rtx (imode);
-			  emit_move_insn (op0, val_hi[0]);
 			  emit_insn (loongarch_vec_repl1_256 (target_hi, op0));
 			}
 		      else if (!rtx_equal_p (val_hi[i], val_hi[0]))
 			{
-			  op0 = gen_reg_rtx (imode);
-			  emit_move_insn (op0, val_hi[i]);
 			  emit_insn (
 			    loongarch_vec_set256 (target_hi, op0, GEN_INT (i)));
 			}
@@ -11053,18 +11048,15 @@ loongarch_expand_vector_init (rtx target, rtx vals)
 		}
 	      if (!lo_same && !half_same)
 		{
+		  op0 = force_reg (imode, val_lo[i]);
 		  /* Assign the lowest element of val_lo to all elements
 		     of target_lo.  */
 		  if (i == 0)
 		    {
-		      op0 = gen_reg_rtx (imode);
-		      emit_move_insn (op0, val_lo[0]);
 		      emit_insn (loongarch_vec_repl1_128 (target_lo, op0));
 		    }
 		  else if (!rtx_equal_p (val_lo[i], val_lo[0]))
 		    {
-		      op0 = gen_reg_rtx (imode);
-		      emit_move_insn (op0, val_lo[i]);
 		      emit_insn (
 			loongarch_vec_set128 (target_lo, op0, GEN_INT (i)));
 		    }
@@ -11096,16 +11088,13 @@ loongarch_expand_vector_init (rtx target, rtx vals)
 		     reduce the number of instructions.  */
 		  if (i == 1)
 		    {
-		      op0 = gen_reg_rtx (imode);
-		      emit_move_insn (op0, val[0]);
-		      op1 = gen_reg_rtx (imode);
-		      emit_move_insn (op1, val[1]);
+		      op0 = force_reg (imode, val[0]);
+		      op1 = force_reg (imode, val[1]);
 		      emit_insn (loongarch_vec_repl2_128 (target, op0, op1));
 		    }
 		  else if (i > 1)
 		    {
-		      op0 = gen_reg_rtx (imode);
-		      emit_move_insn (op0, val[i]);
+		      op0 = force_reg (imode, val[i]);
 		      emit_insn (
 			loongarch_vec_set128 (target, op0, GEN_INT (i)));
 		    }
@@ -11118,18 +11107,15 @@ loongarch_expand_vector_init (rtx target, rtx vals)
 			loongarch_vec_mirror (target, target, const0_rtx));
 		      return;
 		    }
+		  op0 = force_reg (imode, val[i]);
 		  /* Assign the lowest element of val to all elements of
 		     target.  */
 		  if (i == 0)
 		    {
-		      op0 = gen_reg_rtx (imode);
-		      emit_move_insn (op0, val[0]);
 		      emit_insn (loongarch_vec_repl1_128 (target, op0));
 		    }
 		  else if (!rtx_equal_p (val[i], val[0]))
 		    {
-		      op0 = gen_reg_rtx (imode);
-		      emit_move_insn (op0, val[i]);
 		      emit_insn (
 			loongarch_vec_set128 (target, op0, GEN_INT (i)));
 		    }
diff --git a/gcc/testsuite/gcc.target/loongarch/pr113033.c b/gcc/testsuite/gcc.target/loongarch/pr113033.c
new file mode 100644
index 00000000000..4ccd037d846
--- /dev/null
+++ b/gcc/testsuite/gcc.target/loongarch/pr113033.c
@@ -0,0 +1,23 @@
+/* PR target/113033: ICE with vector left rotate */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mlasx" } */
+
+typedef unsigned __attribute__ ((vector_size (16))) v4si;
+typedef unsigned __attribute__ ((vector_size (32))) v8si;
+typedef unsigned long long __attribute__ ((vector_size (16))) v2di;
+typedef unsigned long long __attribute__ ((vector_size (32))) v4di;
+
+#define TEST(tp) \
+extern tp data_##tp; \
+tp \
+test_##tp (int x) \
+{ \
+  const int bit = sizeof (data_##tp[0]) * __CHAR_BIT__; \
+  data_##tp = data_##tp << (x & (bit - 1)) \
+	      | data_##tp >> (bit - x & (bit - 1)); \
+}
+
+TEST (v4si)
+TEST (v8si)
+TEST (v2di)
+TEST (v4di)
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] LoongArch: Clean up vec_init expander
  2023-12-19  6:59 [PATCH 0/2] LoongArch: Fix PR113033 and clean up code Xi Ruoyao
  2023-12-19  6:59 ` [PATCH 1/2] LoongArch: Use force_reg instead of gen_reg_rtx + emit_move_insn in vec_init expander [PR113033] Xi Ruoyao
@ 2023-12-19  6:59 ` Xi Ruoyao
  2023-12-19  7:26 ` [PATCH 0/2] LoongArch: Fix PR113033 and clean up code chenglulu
  2 siblings, 0 replies; 5+ messages in thread
From: Xi Ruoyao @ 2023-12-19  6:59 UTC (permalink / raw)
  To: gcc-patches; +Cc: chenglulu, i, xuchenghua, c, Xi Ruoyao

Non functional change, clean up the code.

gcc/ChangeLog:

	* config/loongarch/loongarch.cc
	(loongarch_expand_vector_init_same): Remove "temp2" and reuse
	"temp" instead.
	(loongarch_expand_vector_init): Use gcc_unreachable () instead
	of gcc_assert (0), and fix the comment for it.
---
 gcc/config/loongarch/loongarch.cc | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/gcc/config/loongarch/loongarch.cc b/gcc/config/loongarch/loongarch.cc
index ef81414342d..5ffd06ce9be 100644
--- a/gcc/config/loongarch/loongarch.cc
+++ b/gcc/config/loongarch/loongarch.cc
@@ -10748,7 +10748,7 @@ loongarch_expand_vector_init_same (rtx target, rtx vals, unsigned nvar)
   machine_mode vmode = GET_MODE (target);
   machine_mode imode = GET_MODE_INNER (vmode);
   rtx same = XVECEXP (vals, 0, 0);
-  rtx temp, temp2;
+  rtx temp;
 
   if (CONST_INT_P (same) && nvar == 0
       && loongarch_signed_immediate_p (INTVAL (same), 10, 0))
@@ -10772,17 +10772,17 @@ loongarch_expand_vector_init_same (rtx target, rtx vals, unsigned nvar)
     }
 
   if (imode == GET_MODE (same))
-    temp2 = same;
+    temp = same;
   else if (GET_MODE_SIZE (imode) >= UNITS_PER_WORD)
     {
       if (GET_CODE (same) == MEM)
 	{
 	  rtx reg_tmp = gen_reg_rtx (GET_MODE (same));
 	  loongarch_emit_move (reg_tmp, same);
-	  temp2 = simplify_gen_subreg (imode, reg_tmp, GET_MODE (reg_tmp), 0);
+	  temp = simplify_gen_subreg (imode, reg_tmp, GET_MODE (reg_tmp), 0);
 	}
       else
-	temp2 = simplify_gen_subreg (imode, same, GET_MODE (same), 0);
+	temp = simplify_gen_subreg (imode, same, GET_MODE (same), 0);
     }
   else
     {
@@ -10790,13 +10790,13 @@ loongarch_expand_vector_init_same (rtx target, rtx vals, unsigned nvar)
 	{
 	  rtx reg_tmp = gen_reg_rtx (GET_MODE (same));
 	  loongarch_emit_move (reg_tmp, same);
-	  temp2 = lowpart_subreg (imode, reg_tmp, GET_MODE (reg_tmp));
+	  temp = lowpart_subreg (imode, reg_tmp, GET_MODE (reg_tmp));
 	}
       else
-	temp2 = lowpart_subreg (imode, same, GET_MODE (same));
+	temp = lowpart_subreg (imode, same, GET_MODE (same));
     }
 
-  temp = force_reg (imode, temp2);
+  temp = force_reg (imode, temp);
 
   switch (vmode)
     {
@@ -11142,8 +11142,8 @@ loongarch_expand_vector_init (rtx target, rtx vals)
       return;
     }
 
-  /* Loongson is the only cpu with vectors with more elements.  */
-  gcc_assert (0);
+  /* No LoongArch CPU supports vectors with more elements as at now.  */
+  gcc_unreachable ();
 }
 
 /* Implement HARD_REGNO_CALLER_SAVE_MODE.  */
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] LoongArch: Fix PR113033 and clean up code
  2023-12-19  6:59 [PATCH 0/2] LoongArch: Fix PR113033 and clean up code Xi Ruoyao
  2023-12-19  6:59 ` [PATCH 1/2] LoongArch: Use force_reg instead of gen_reg_rtx + emit_move_insn in vec_init expander [PR113033] Xi Ruoyao
  2023-12-19  6:59 ` [PATCH 2/2] LoongArch: Clean up vec_init expander Xi Ruoyao
@ 2023-12-19  7:26 ` chenglulu
  2023-12-20  3:52   ` Chenghui Pan
  2 siblings, 1 reply; 5+ messages in thread
From: chenglulu @ 2023-12-19  7:26 UTC (permalink / raw)
  To: Xi Ruoyao, gcc-patches; +Cc: i, xuchenghua, c, Chenghui Pan, Guo Jie

We will read and test these patches as soon as possible.

Thanks!

在 2023/12/19 下午2:59, Xi Ruoyao 写道:
> Superseds
> https://gcc.gnu.org/pipermail/gcc-patches/2023-December/640871.html.
>
> Per Jakub's response, vec_init patterns do not have a predicate on the
> input operand so the operand can be *anything*.  It's not safe to simply
> move it into an reg, and we have to use force_reg instead.
>
> The code clean up is separated into the 2nd patch to make reviewing
> easier.
>
> Bootstrapped and regtested on loongarch64-linux-gnu.  Ok for trunk?
>
> Xi Ruoyao (2):
>    LoongArch: Use force_reg instead of gen_reg_rtx + emit_move_insn in
>      vec_init expander [PR113033]
>    LoongArch: Clean up vec_init expander
>
>   gcc/config/loongarch/loongarch.cc             | 54 +++++++------------
>   gcc/testsuite/gcc.target/loongarch/pr113033.c | 23 ++++++++
>   2 files changed, 43 insertions(+), 34 deletions(-)
>   create mode 100644 gcc/testsuite/gcc.target/loongarch/pr113033.c
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 0/2] LoongArch: Fix PR113033 and clean up code
  2023-12-19  7:26 ` [PATCH 0/2] LoongArch: Fix PR113033 and clean up code chenglulu
@ 2023-12-20  3:52   ` Chenghui Pan
  0 siblings, 0 replies; 5+ messages in thread
From: Chenghui Pan @ 2023-12-20  3:52 UTC (permalink / raw)
  To: chenglulu, Xi Ruoyao, gcc-patches; +Cc: i, xuchenghua, c, Guo Jie

The patches are look ok and no problem in spec2017 correctness check.

On 2023/12/19 15:26, chenglulu wrote:
> We will read and test these patches as soon as possible.
>
> Thanks!
>
> 在 2023/12/19 下午2:59, Xi Ruoyao 写道:
>> Superseds
>> https://gcc.gnu.org/pipermail/gcc-patches/2023-December/640871.html.
>>
>> Per Jakub's response, vec_init patterns do not have a predicate on the
>> input operand so the operand can be *anything*.  It's not safe to simply
>> move it into an reg, and we have to use force_reg instead.
>>
>> The code clean up is separated into the 2nd patch to make reviewing
>> easier.
>>
>> Bootstrapped and regtested on loongarch64-linux-gnu.  Ok for trunk?
>>
>> Xi Ruoyao (2):
>>    LoongArch: Use force_reg instead of gen_reg_rtx + emit_move_insn in
>>      vec_init expander [PR113033]
>>    LoongArch: Clean up vec_init expander
>>
>>   gcc/config/loongarch/loongarch.cc             | 54 +++++++------------
>>   gcc/testsuite/gcc.target/loongarch/pr113033.c | 23 ++++++++
>>   2 files changed, 43 insertions(+), 34 deletions(-)
>>   create mode 100644 gcc/testsuite/gcc.target/loongarch/pr113033.c
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-12-20  3:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-19  6:59 [PATCH 0/2] LoongArch: Fix PR113033 and clean up code Xi Ruoyao
2023-12-19  6:59 ` [PATCH 1/2] LoongArch: Use force_reg instead of gen_reg_rtx + emit_move_insn in vec_init expander [PR113033] Xi Ruoyao
2023-12-19  6:59 ` [PATCH 2/2] LoongArch: Clean up vec_init expander Xi Ruoyao
2023-12-19  7:26 ` [PATCH 0/2] LoongArch: Fix PR113033 and clean up code chenglulu
2023-12-20  3:52   ` Chenghui Pan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).