This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: S390: Allowing a NOP instruction without operands
- From: Martin Schwidefsky <schwidefsky at de dot ibm dot com>
- To: Nick Clifton <nickc at redhat dot com>
- Cc: binutils at sourceware dot org, Andreas Krebbel <Andreas dot Krebbel at de dot ibm dot com>
- Date: Thu, 18 Jun 2009 11:19:14 +0200
- Subject: Re: S390: Allowing a NOP instruction without operands
- References: <m3d492kfko.fsf@redhat.com>
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.