This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: S390: Allowing a NOP instruction without operands


Hi Nick,

On Wed, 17 Jun 2009 17:09:11 +0100
Nick Clifton <nickc@redhat.com> wrote:

>   A Fedora user recently filed a binutils bug report about the NOP
>   instruction for the s390:
> 
>     https://bugzilla.redhat.com/show_bug.cgi?id=506417
> 
>   In it they report that the s390's NOP instruction has to have some
>   operands specified, which seems rather counter-intuitive given that
>   it does not do anything.
> 
>   I have proposed a patch (attached here and also available via that
>   bugzilla page) to allow the assembler to ignore any missing operands
>   when a NOP instruction is found.  What do you think ?  Would this be
>   OK for the s390 port or is there a reason why the NOP instruction
>   should take some parameters ?

First of all: yes I think it makes sense to have a nop instruction that
can be used without operands. I don't like the implementation though,
to check for particular opcodes with a strcmp feels wrong. My counter
proposal would be to introduce new variantes for the RR_0R and RX_0RRD
instruction formats. Make all operands optional and replace the strcmp
check with a check against S390_OPERAND_OPTIONAL. And someday I have to
rewrite the whole optional operands logic ..

--
diff -urpN src/gas/config/tc-s390.c src-s390/gas/config/tc-s390.c
--- src/gas/config/tc-s390.c	2009-04-20 12:51:45.000000000 +0200
+++ src-s390/gas/config/tc-s390.c	2009-06-18 10:49:10.000000000 +0200
@@ -1188,7 +1188,24 @@ md_gather_operands (char *str,
       if (ex.X_op == O_illegal)
 	as_bad (_("illegal operand"));
       else if (ex.X_op == O_absent)
-	as_bad (_("missing operand"));
+	{
+	  /* No operands, check if all operands can be skipped.  */
+	  while (*opindex_ptr != 0 && operand->flags & S390_OPERAND_OPTIONAL)
+	    {
+	      if (operand->flags & S390_OPERAND_DISP)
+		{
+		  /* An optional displacement makes the whole D(X,B)
+		     D(L,B) or D(B) block optional.  */
+		  do {
+		    operand = s390_operands + *(++opindex_ptr);
+		  } while (!(operand->flags & S390_OPERAND_BASE));
+		}
+	      operand = s390_operands + *(++opindex_ptr);
+	    }
+	  if (opindex_ptr[0] == '\0')
+	    break;
+	  as_bad (_("missing operand"));
+	}
       else if (ex.X_op == O_register || ex.X_op == O_constant)
 	{
 	  s390_lit_suffix (&str, &ex, ELF_SUFFIX_NONE);
diff -urpN src/opcodes/s390-opc.c src-s390/opcodes/s390-opc.c
--- src/opcodes/s390-opc.c	2008-11-17 15:59:12.000000000 +0100
+++ src-s390/opcodes/s390-opc.c	2009-06-18 10:25:09.000000000 +0200
@@ -48,132 +48,136 @@ const struct s390_operand s390_operands[
   { 4, 8, S390_OPERAND_GPR },
 #define R_12   2                  /* GPR starting at position 12 */
   { 4, 12, S390_OPERAND_GPR },
-#define R_16   3                  /* GPR starting at position 16 */
+#define RO_12  3                 /* optional GPR starting at position 12 */
+  { 4, 12, S390_OPERAND_GPR|S390_OPERAND_OPTIONAL },
+#define R_16   4                  /* GPR starting at position 16 */
   { 4, 16, S390_OPERAND_GPR },
-#define R_20   4                  /* GPR starting at position 20 */
+#define R_20   5                  /* GPR starting at position 20 */
   { 4, 20, S390_OPERAND_GPR },
-#define R_24   5                  /* GPR starting at position 24 */
+#define R_24   6                  /* GPR starting at position 24 */
   { 4, 24, S390_OPERAND_GPR },
-#define R_28   6                  /* GPR starting at position 28 */
+#define R_28   7                  /* GPR starting at position 28 */
   { 4, 28, S390_OPERAND_GPR },
-#define RO_28  7                  /* optional GPR starting at position 28 */
+#define RO_28  8                  /* optional GPR starting at position 28 */
   { 4, 28, (S390_OPERAND_GPR | S390_OPERAND_OPTIONAL) },
-#define R_32   8                  /* GPR starting at position 32 */
+#define R_32   9                  /* GPR starting at position 32 */
   { 4, 32, S390_OPERAND_GPR },
 
 /* Floating point register operands.  */
 
-#define F_8    9                  /* FPR starting at position 8 */
+#define F_8    10                 /* FPR starting at position 8 */
   { 4, 8, S390_OPERAND_FPR },
-#define F_12   10                 /* FPR starting at position 12 */
+#define F_12   11                 /* FPR starting at position 12 */
   { 4, 12, S390_OPERAND_FPR },
-#define F_16   11                 /* FPR starting at position 16 */
+#define F_16   12                 /* FPR starting at position 16 */
   { 4, 16, S390_OPERAND_FPR },
-#define F_20   12                 /* FPR starting at position 16 */
+#define F_20   13                 /* FPR starting at position 16 */
   { 4, 16, S390_OPERAND_FPR },
-#define F_24   13                 /* FPR starting at position 24 */
+#define F_24   14                 /* FPR starting at position 24 */
   { 4, 24, S390_OPERAND_FPR },
-#define F_28   14                 /* FPR starting at position 28 */
+#define F_28   15                 /* FPR starting at position 28 */
   { 4, 28, S390_OPERAND_FPR },
-#define F_32   15                 /* FPR starting at position 32 */
+#define F_32   16                 /* FPR starting at position 32 */
   { 4, 32, S390_OPERAND_FPR },
 
 /* Access register operands.  */
 
-#define A_8    16                 /* Access reg. starting at position 8 */
+#define A_8    17                 /* Access reg. starting at position 8 */
   { 4, 8, S390_OPERAND_AR },
-#define A_12   17                 /* Access reg. starting at position 12 */
+#define A_12   18                 /* Access reg. starting at position 12 */
   { 4, 12, S390_OPERAND_AR },
-#define A_24   18                 /* Access reg. starting at position 24 */
+#define A_24   19                 /* Access reg. starting at position 24 */
   { 4, 24, S390_OPERAND_AR },
-#define A_28   19                 /* Access reg. starting at position 28 */
+#define A_28   20                 /* Access reg. starting at position 28 */
   { 4, 28, S390_OPERAND_AR },
 
 /* Control register operands.  */
 
-#define C_8    20                 /* Control reg. starting at position 8 */
+#define C_8    21                 /* Control reg. starting at position 8 */
   { 4, 8, S390_OPERAND_CR },
-#define C_12   21                 /* Control reg. starting at position 12 */
+#define C_12   22                 /* Control reg. starting at position 12 */
   { 4, 12, S390_OPERAND_CR },
 
 /* Base register operands.  */
 
-#define B_16   22                 /* Base register starting at position 16 */
+#define B_16   23                 /* Base register starting at position 16 */
   { 4, 16, S390_OPERAND_BASE|S390_OPERAND_GPR },
-#define B_32   23                 /* Base register starting at position 32 */
+#define B_32   24                 /* Base register starting at position 32 */
   { 4, 32, S390_OPERAND_BASE|S390_OPERAND_GPR },
 
-#define X_12   24                 /* Index register starting at position 12 */
+#define X_12   25                 /* Index register starting at position 12 */
   { 4, 12, S390_OPERAND_INDEX|S390_OPERAND_GPR },
 
 /* Address displacement operands.  */
 
-#define D_20   25                 /* Displacement starting at position 20 */
+#define D_20   26                 /* Displacement starting at position 20 */
   { 12, 20, S390_OPERAND_DISP },
-#define D_36   26                 /* Displacement starting at position 36 */
+#define DO_20  27                 /* optional Displ. starting at position 20 */
+  { 12, 20, S390_OPERAND_DISP|S390_OPERAND_OPTIONAL },
+#define D_36   28                 /* Displacement starting at position 36 */
   { 12, 36, S390_OPERAND_DISP },
-#define D20_20 27		  /* 20 bit displacement starting at 20 */
+#define D20_20 29		  /* 20 bit displacement starting at 20 */
   { 20, 20, S390_OPERAND_DISP|S390_OPERAND_SIGNED },
 
 /* Length operands.  */
 
-#define L4_8   28                 /* 4 bit length starting at position 8 */
+#define L4_8   30                 /* 4 bit length starting at position 8 */
   { 4, 8, S390_OPERAND_LENGTH },
-#define L4_12  29                 /* 4 bit length starting at position 12 */
+#define L4_12  31                 /* 4 bit length starting at position 12 */
   { 4, 12, S390_OPERAND_LENGTH },
-#define L8_8   30                 /* 8 bit length starting at position 8 */
+#define L8_8   32                 /* 8 bit length starting at position 8 */
   { 8, 8, S390_OPERAND_LENGTH },
 
 /* Signed immediate operands.  */
 
-#define I8_8   31		  /* 8 bit signed value starting at 8 */
+#define I8_8   33		  /* 8 bit signed value starting at 8 */
   { 8, 8, S390_OPERAND_SIGNED },
-#define I8_32  32		  /* 8 bit signed value starting at 32 */
+#define I8_32  34		  /* 8 bit signed value starting at 32 */
   { 8, 32, S390_OPERAND_SIGNED },
-#define I16_16 33                 /* 16 bit signed value starting at 16 */
+#define I16_16 35                 /* 16 bit signed value starting at 16 */
   { 16, 16, S390_OPERAND_SIGNED },
-#define I16_32 34                 /* 16 bit signed value starting at 32 */
+#define I16_32 36                 /* 16 bit signed value starting at 32 */
   { 16, 32, S390_OPERAND_SIGNED },
-#define I32_16 35		  /* 32 bit signed value starting at 16 */
+#define I32_16 37		  /* 32 bit signed value starting at 16 */
   { 32, 16, S390_OPERAND_SIGNED },
 
 /* Unsigned immediate operands.  */
 
-#define U4_8   36                 /* 4 bit unsigned value starting at 8 */
+#define U4_8   38                 /* 4 bit unsigned value starting at 8 */
   { 4, 8, 0 },
-#define U4_12  37                 /* 4 bit unsigned value starting at 12 */
+#define U4_12  39                 /* 4 bit unsigned value starting at 12 */
   { 4, 12, 0 },
-#define U4_16  38                 /* 4 bit unsigned value starting at 16 */
+#define U4_16  40                 /* 4 bit unsigned value starting at 16 */
   { 4, 16, 0 },
-#define U4_20  39                 /* 4 bit unsigned value starting at 20 */
+#define U4_20  41                 /* 4 bit unsigned value starting at 20 */
   { 4, 20, 0 },
-#define U4_32  40                 /* 4 bit unsigned value starting at 32 */
+#define U4_32  42                 /* 4 bit unsigned value starting at 32 */
   { 4, 32, 0 },
-#define U8_8   41                 /* 8 bit unsigned value starting at 8 */
+#define U8_8   43                 /* 8 bit unsigned value starting at 8 */
   { 8, 8, 0 },
-#define U8_16  42                 /* 8 bit unsigned value starting at 16 */
+#define U8_16  44                 /* 8 bit unsigned value starting at 16 */
   { 8, 16, 0 },
-#define U8_24  43                 /* 8 bit unsigned value starting at 24 */
+#define U8_24  45                 /* 8 bit unsigned value starting at 24 */
   { 8, 24, 0 },
-#define U8_32  44                 /* 8 bit unsigned value starting at 32 */
+#define U8_32  46                 /* 8 bit unsigned value starting at 32 */
   { 8, 32, 0 },
-#define U16_16 45                 /* 16 bit unsigned value starting at 16 */
+#define U16_16 47                 /* 16 bit unsigned value starting at 16 */
   { 16, 16, 0 },
-#define U16_32 46		  /* 16 bit unsigned value starting at 32 */
+#define U16_32 48		  /* 16 bit unsigned value starting at 32 */
   { 16, 32, 0 },
-#define U32_16 47		  /* 32 bit unsigned value starting at 16 */
+#define U32_16 49		  /* 32 bit unsigned value starting at 16 */
   { 32, 16, 0 },
 
 /* PC-relative address operands.  */
 
-#define J16_16 48                 /* PC relative jump offset at 16 */
+#define J16_16 50                 /* PC relative jump offset at 16 */
   { 16, 16, S390_OPERAND_PCREL },
-#define J32_16 49                 /* PC relative long offset at 16 */
+#define J32_16 51                 /* PC relative long offset at 16 */
   { 32, 16, S390_OPERAND_PCREL },
 
 /* Conditional mask operands.  */
 
-#define M_16   50                 /* 4 bit optional mask starting at 16 */
+#define M_16   52                 /* 4 bit optional mask starting at 16 */
   { 4, 16, S390_OPERAND_OPTIONAL },
 
 };
@@ -278,6 +282,7 @@ const struct s390_operand s390_operands[
 #define INSTR_RRF_U0RR   4, { R_24,R_28,U4_16,0,0,0 }          /* e.g. clrt  */
 #define INSTR_RRF_00RR   4, { R_24,R_28,0,0,0,0 }              /* e.g. clrtne */
 #define INSTR_RR_0R      2, { R_12, 0,0,0,0,0 }                /* e.g. br    */
+#define INSTR_RR_0R_OPT  2, { RO_12, 0,0,0,0,0 }               /* e.g. nopr  */
 #define INSTR_RR_FF      2, { F_8,F_12,0,0,0,0 }               /* e.g. adr   */
 #define INSTR_RR_R0      2, { R_8, 0,0,0,0,0 }                 /* e.g. spm   */
 #define INSTR_RR_RR      2, { R_8,R_12,0,0,0,0 }               /* e.g. lr    */
@@ -308,6 +313,7 @@ const struct s390_operand s390_operands[
 #define INSTR_RXY_FRRD   6, { F_8,D20_20,X_12,B_16,0,0 }       /* e.g. ley   */
 #define INSTR_RXY_URRD   6, { U4_8,D20_20,X_12,B_16,0,0 }      /* e.g. pfd   */
 #define INSTR_RX_0RRD    4, { D_20,X_12,B_16,0,0,0 }           /* e.g. be    */
+#define INSTR_RX_0RRD_OPT 4, { DO_20,X_12,B_16,0,0,0 }         /* e.g. nop   */
 #define INSTR_RX_FRRD    4, { F_8,D_20,X_12,B_16,0,0 }         /* e.g. ae    */
 #define INSTR_RX_RRRD    4, { R_8,D_20,X_12,B_16,0,0 }         /* e.g. l     */
 #define INSTR_RX_URRD    4, { U4_8,D_20,X_12,B_16,0,0 }        /* e.g. bc    */
@@ -382,6 +388,7 @@ const struct s390_operand s390_operands[
 #define MASK_RRF_U0RR    { 0xff, 0xff, 0x0f, 0x00, 0x00, 0x00 }
 #define MASK_RRF_00RR    { 0xff, 0xff, 0xff, 0x00, 0x00, 0x00 }
 #define MASK_RR_0R       { 0xff, 0xf0, 0x00, 0x00, 0x00, 0x00 }
+#define MASK_RR_0R_OPT   { 0xff, 0xf0, 0x00, 0x00, 0x00, 0x00 }
 #define MASK_RR_FF       { 0xff, 0x00, 0x00, 0x00, 0x00, 0x00 }
 #define MASK_RR_R0       { 0xff, 0x0f, 0x00, 0x00, 0x00, 0x00 }
 #define MASK_RR_RR       { 0xff, 0x00, 0x00, 0x00, 0x00, 0x00 }
@@ -412,6 +419,7 @@ const struct s390_operand s390_operands[
 #define MASK_RXY_FRRD    { 0xff, 0x00, 0x00, 0x00, 0x00, 0xff }
 #define MASK_RXY_URRD    { 0xff, 0x00, 0x00, 0x00, 0x00, 0xff }
 #define MASK_RX_0RRD     { 0xff, 0xf0, 0x00, 0x00, 0x00, 0x00 }
+#define MASK_RX_0RRD_OPT { 0xff, 0xf0, 0x00, 0x00, 0x00, 0x00 }
 #define MASK_RX_FRRD     { 0xff, 0x00, 0x00, 0x00, 0x00, 0x00 }
 #define MASK_RX_RRRD     { 0xff, 0x00, 0x00, 0x00, 0x00, 0x00 }
 #define MASK_RX_URRD     { 0xff, 0x00, 0x00, 0x00, 0x00, 0x00 }
diff -urpN src/opcodes/s390-opc.txt src-s390/opcodes/s390-opc.txt
--- src/opcodes/s390-opc.txt	2008-11-19 11:27:07.000000000 +0100
+++ src-s390/opcodes/s390-opc.txt	2009-06-18 09:51:16.000000000 +0200
@@ -260,10 +260,10 @@ b252 msr RRE_RR "multiply single" g5 esa
 71 ms RX_RRRD "multiply single" g5 esa,zarch
 a700 tmh RI_RU "test under mask high" g5 esa,zarch
 a701 tml RI_RU "test under mask low" g5 esa,zarch
-0700 nopr RR_0R "no operation" g5 esa,zarch
+0700 nopr RR_0R_OPT "no operation" g5 esa,zarch
 0700 b*8r RR_0R "conditional branch" g5 esa,zarch
 07f0 br RR_0R "unconditional branch" g5 esa,zarch
-4700 nop RX_0RRD "no operation" g5 esa,zarch
+4700 nop RX_0RRD_OPT "no operation" g5 esa,zarch
 4700 b*8 RX_0RRD "conditional branch" g5 esa,zarch
 47f0 b RX_0RRD "unconditional branch" g5 esa,zarch
 a704 j*8 RI_0P "conditional jump" g5 esa,zarch


-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]