diff mbox series

Further rs6000 asm modifier tweaks (PR target/88188)

Message ID 20181204085645.GG12380@tucnak
State New
Headers show
Series Further rs6000 asm modifier tweaks (PR target/88188) | expand

Commit Message

Jakub Jelinek Dec. 4, 2018, 8:56 a.m. UTC 
Hi!

This patch adds further tweaks discussed in the last PR88188 mail.
ccr_bit now returns -1 instead of failing assertions, so that the caller
can decide what to do with it (fail assertion or output_operand_lossage
with some reasonable diagnostics.
For D and t it now requires CR_REGNO_P, so that it doesn't print completely
meaningless numbers if used with other registers.
And use REG_P macros a little bit.

Bootstrapped/regtested on powerpc64{,le}-linux, ok for trunk?

2018-12-04  Jakub Jelinek  <jakub@redhat.com>

	PR target/88188
	* config/rs6000/rs6000.c (ccr_bit): Return -1 instead of assertion
	failures.
	(print_operand): Use REG_P instead of GET_CODE == REG.
	<case 'D', case 't'>: Also check CR_REGNO_P (REGNO (x)).
	* config/rs6000/rs6000.md (scc patterns): Assert ccr_bit didn't
	return -1.

	* gcc.target/powerpc/pr88188-2.c: New test.


	Jakub

Comments

Segher Boessenkool Dec. 4, 2018, 3:32 p.m. UTC | #1 
Hi Jakub,

On Tue, Dec 04, 2018 at 09:56:45AM +0100, Jakub Jelinek wrote:
> This patch adds further tweaks discussed in the last PR88188 mail.

Thanks for doing this :-)

> ccr_bit now returns -1 instead of failing assertions, so that the caller
> can decide what to do with it (fail assertion or output_operand_lossage
> with some reasonable diagnostics.

(And all callers already test for that).

> For D and t it now requires CR_REGNO_P, so that it doesn't print completely
> meaningless numbers if used with other registers.
> And use REG_P macros a little bit.


> 2018-12-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/88188
> 	* config/rs6000/rs6000.c (ccr_bit): Return -1 instead of assertion
> 	failures.
> 	(print_operand): Use REG_P instead of GET_CODE == REG.
> 	<case 'D', case 't'>: Also check CR_REGNO_P (REGNO (x)).
> 	* config/rs6000/rs6000.md (scc patterns): Assert ccr_bit didn't
> 	return -1.
> 
> 	* gcc.target/powerpc/pr88188-2.c: New test.

> @@ -20754,7 +20765,7 @@ print_operand (FILE *file, rtx x, int co
>  
>      case 'D':
>        /* Like 'J' but get to the GT bit only.  */
> -      if (!REG_P (x))
> +      if (! REG_P (x) || ! CR_REGNO_P (REGNO (x)))

No space after ! please.  I know surrounding code does it, but how will
we ever get rid of that otherwise :-)

> @@ -21014,7 +21025,7 @@ print_operand (FILE *file, rtx x, int co
>  
>      case 't':
>        /* Like 'J' but get to the OVERFLOW/UNORDERED bit.  */
> -      if (!REG_P (x) || GET_MODE (x) != CCmode)
> +      if (! REG_P (x) || GET_MODE (x) != CCmode || ! CR_REGNO_P (REGNO (x)))

Strange that this one uses GET_MODE, all the rest do not.  Since you add
the CR_REGNO test now, the mode test is obviously superfluous, so you
can delete it?

Okay for trunk, with or without those extra tweaks.  Thanks!


Segher
diff mbox series

Patch

--- gcc/config/rs6000/rs6000.c.jj	2018-11-30 19:59:59.679789865 +0100
+++ gcc/config/rs6000/rs6000.c	2018-12-03 12:53:24.149582415 +0100
@@ -20624,7 +20624,8 @@  ccr_bit (rtx op, int scc_p)
 
   reg = XEXP (op, 0);
 
-  gcc_assert (GET_CODE (reg) == REG && CR_REGNO_P (REGNO (reg)));
+  if (!REG_P (reg) || !CR_REGNO_P (REGNO (reg)))
+    return -1;
 
   cc_mode = GET_MODE (reg);
   cc_regnum = REGNO (reg);
@@ -20634,9 +20635,19 @@  ccr_bit (rtx op, int scc_p)
 
   /* When generating a sCOND operation, only positive conditions are
      allowed.  */
-  gcc_assert (!scc_p
-	      || code == EQ || code == GT || code == LT || code == UNORDERED
-	      || code == GTU || code == LTU);
+  if (scc_p)
+    switch (code)
+      {
+      case EQ:
+      case GT:
+      case LT:
+      case UNORDERED:
+      case GTU:
+      case LTU:
+	break;
+      default:
+	return -1;
+      }
 
   switch (code)
     {
@@ -20661,7 +20672,7 @@  ccr_bit (rtx op, int scc_p)
       return scc_p ? base_bit + 3 : base_bit + 1;
 
     default:
-      gcc_unreachable ();
+      return -1;
     }
 }
 
@@ -20754,7 +20765,7 @@  print_operand (FILE *file, rtx x, int co
 
     case 'D':
       /* Like 'J' but get to the GT bit only.  */
-      if (!REG_P (x))
+      if (! REG_P (x) || ! CR_REGNO_P (REGNO (x)))
 	{
 	  output_operand_lossage ("invalid %%D value");
 	  return;
@@ -20782,7 +20793,7 @@  print_operand (FILE *file, rtx x, int co
 
     case 'E':
       /* X is a CR register.  Print the number of the EQ bit of the CR */
-      if (GET_CODE (x) != REG || ! CR_REGNO_P (REGNO (x)))
+      if (! REG_P (x) || ! CR_REGNO_P (REGNO (x)))
 	output_operand_lossage ("invalid %%E value");
       else
 	fprintf (file, "%d", 4 * (REGNO (x) - CR0_REGNO) + 2);
@@ -20791,7 +20802,7 @@  print_operand (FILE *file, rtx x, int co
     case 'f':
       /* X is a CR register.  Print the shift count needed to move it
 	 to the high-order four bits.  */
-      if (GET_CODE (x) != REG || ! CR_REGNO_P (REGNO (x)))
+      if (! REG_P (x) || ! CR_REGNO_P (REGNO (x)))
 	output_operand_lossage ("invalid %%f value");
       else
 	fprintf (file, "%d", 4 * (REGNO (x) - CR0_REGNO));
@@ -20800,7 +20811,7 @@  print_operand (FILE *file, rtx x, int co
     case 'F':
       /* Similar, but print the count for the rotate in the opposite
 	 direction.  */
-      if (GET_CODE (x) != REG || ! CR_REGNO_P (REGNO (x)))
+      if (! REG_P (x) || ! CR_REGNO_P (REGNO (x)))
 	output_operand_lossage ("invalid %%F value");
       else
 	fprintf (file, "%d", 32 - 4 * (REGNO (x) - CR0_REGNO));
@@ -20998,7 +21009,7 @@  print_operand (FILE *file, rtx x, int co
 
     case 'R':
       /* X is a CR register.  Print the mask for `mtcrf'.  */
-      if (GET_CODE (x) != REG || ! CR_REGNO_P (REGNO (x)))
+      if (! REG_P (x) || ! CR_REGNO_P (REGNO (x)))
 	output_operand_lossage ("invalid %%R value");
       else
 	fprintf (file, "%d", 128 >> (REGNO (x) - CR0_REGNO));
@@ -21014,7 +21025,7 @@  print_operand (FILE *file, rtx x, int co
 
     case 't':
       /* Like 'J' but get to the OVERFLOW/UNORDERED bit.  */
-      if (!REG_P (x) || GET_MODE (x) != CCmode)
+      if (! REG_P (x) || GET_MODE (x) != CCmode || ! CR_REGNO_P (REGNO (x)))
 	{
 	  output_operand_lossage ("invalid %%t value");
 	  return;
--- gcc/config/rs6000/rs6000.md.jj	2018-11-30 19:59:59.680789848 +0100
+++ gcc/config/rs6000/rs6000.md	2018-12-03 13:07:34.935747046 +0100
@@ -11676,6 +11676,7 @@  (define_insn ""
   int put_bit = 31 - (INTVAL (operands[3]) & 31);
   int count;
 
+  gcc_assert (is_bit != -1);
   if (is_bit >= put_bit)
     count = is_bit - put_bit;
   else
@@ -11710,6 +11711,7 @@  (define_insn ""
   int put_bit = 31 - (INTVAL (operands[3]) & 31);
   int count;
 
+  gcc_assert (is_bit != -1);
   /* Force split for non-cc0 compare.  */
   if (which_alternative == 1)
      return "#";
--- gcc/testsuite/gcc.target/powerpc/pr88188-2.c.jj	2018-12-03 13:30:22.734757070 +0100
+++ gcc/testsuite/gcc.target/powerpc/pr88188-2.c	2018-12-03 13:30:43.205432026 +0100
@@ -0,0 +1,8 @@ 
+/* PR target/88188 */
+/* { dg-do compile } */
+
+void
+foo (void)
+{
+  __asm volatile ("%D0" : : "r" (0));	/* { dg-error "invalid %D value" } */
+}