From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22932 invoked by alias); 18 Jun 2009 09:19:30 -0000 Received: (qmail 22911 invoked by uid 22791); 18 Jun 2009 09:19:28 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_35,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mtagate8.de.ibm.com (HELO mtagate8.de.ibm.com) (195.212.29.157) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 18 Jun 2009 09:19:20 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate8.de.ibm.com (8.14.3/8.13.8) with ESMTP id n5I9JH2d314232 for ; Thu, 18 Jun 2009 09:19:17 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n5I9JHNM3383454 for ; Thu, 18 Jun 2009 11:19:17 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n5I9JGlc006247 for ; Thu, 18 Jun 2009 11:19:17 +0200 Received: from skybase (dyn-9-152-212-53.boeblingen.de.ibm.com [9.152.212.53]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id n5I9JGSQ006239; Thu, 18 Jun 2009 11:19:16 +0200 Date: Thu, 18 Jun 2009 09:38:00 -0000 From: Martin Schwidefsky To: Nick Clifton Cc: binutils@sourceware.org, Andreas Krebbel Subject: Re: S390: Allowing a NOP instruction without operands Message-ID: <20090618111914.496fb4d1@skybase> In-Reply-To: References: Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org X-SW-Source: 2009-06/txt/msg00298.txt.bz2 Hi Nick, On Wed, 17 Jun 2009 17:09:11 +0100 Nick Clifton 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.