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: [PATCH] x86: Add .nop directive to assembler


I haven't read every post in this thread, nor do I pretend to understand all the
issues.

But it this change seems to have the effect that any instruction called "NOP" 
in the .s file are completely ignored :(

Many cpus have  a NOP instruction. It's often used in realtime systems to
implement short delays.

J'


On Fri, Feb 23, 2018 at 03:56:41PM -0800, H.J. Lu wrote:
     On Tue, Feb 20, 2018 at 5:24 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
     > On Mon, Feb 19, 2018 at 5:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
     >> On Mon, Feb 19, 2018 at 4:05 PM, Alan Modra <amodra@gmail.com> wrote:
     >>> On Mon, Feb 19, 2018 at 03:24:17PM -0800, H.J. Lu wrote:
     >>>> On Mon, Feb 19, 2018 at 3:19 PM, Alan Modra <amodra@gmail.com> wrote:
     >>>> > On Mon, Feb 19, 2018 at 10:08:38PM +0000, Maciej W. Rozycki wrote:
     >>>> >> On Mon, 19 Feb 2018, Alan Modra wrote:
     >>>> >>
     >>>> >> > > .nop is similar to .skip, except that it fills with NOPs.  Yes, it can be used
     >>>> >> > > wherever .skip can be used.
     >>>> >> >
     >>>> >> > Given https://sourceware.org/ml/binutils/2018-02/msg00322.html I'm
     >>>> >> > inclined to think the name of the directive should change.
     >>>> >> > .skipnops perhaps?
     >>>> >>
     >>>> >>  Just `.nops' maybe?  There doesn't appear to be any matching mnemonic in
     >>>> >> opcodes/.
     >>>> >
     >>>> > I'd be happy with that too.
     >>>>
     >>>> There is no guarantee that one of those NO_PSEUDO_DOT targets or the new
     >>>> NO_PSEUDO_DOT target won't have an instruction called nops or skipnops in
     >>>> the future.
     >>>
     >>> If a target adds an instruction like that, then the target will need
     >>> to deal with it.  For example, as the spu target deals with "set" and
     >>> "equ" which existed as directives before the spu defined them as
     >>> instructions.
     >>>
     >>> In this case the use of "nop" as an instruction existed before you
     >>> decided to define ".nop" as a directive, and lack of testing resulted
     >>> in not discovering the NO_PSEUDO_DOT clash.  I suspect we wouldn't be
     >>> having this conversation if you had run a full test suite regression,
     >>> rather than just testing x86.  You yourself would have chosen
     >>> something other than ".nop" as a directive!
     >>
     >> I would have chosen .nop and handled it for NO_PSEUDO_DOT targets.
     >>
     >> Here is a patch to rename .nop to .nops.  OK for master?
     >>
     >
     > Just for the record, I found it is extremely odd that .nop has to be
     > renamed to .nops just because of NO_PSEUDO_DOT targets.
     >
     
     How about this patch?
     
     -- 
     H.J.

     From 56fb57ef6b66d805df88b65a800f844cbbb59cbd Mon Sep 17 00:00:00 2001
     From: "H.J. Lu" <hjl.tools@gmail.com>
     Date: Fri, 23 Feb 2018 15:41:56 -0800
     Subject: [PATCH] Skip pseudo-op for instruction of the same name
     
     Pseudo-ops on NO_PSEUDO_DOT targets don't require a leading dot.  But
     there may be an instruction which has the same name as a pseudo-op.
     This patch adds md_pseudo_insn_table for NO_PSEUDO_DOT targets, which
     contains generic pseudo-ops which are also machine specific instructions,
     and it consults md_pseudo_insn_table to skip pseudo-op which matches
     a machine specific instruction.
     
     	* read.c (po_pseudo_insn_hash): New if md_pseudo_insn_table is
     	defined.
     	(pop_pseudo_insn_insert): New.
     	(pop_pseudo_insn_find): Likewise.
     	(pobegin): Call pop_pseudo_insn_insert.
     	(read_a_source_file): Skip pseudo-op without dot if there is an
     	instruction of the same name.
     	* config/tc-m68hc11.c (md_pseudo_insn_table): New.
     	* config/tc-m68hc11.h (md_pseudo_insn_table): Likewise.
     	* config/tc-m68k.c (md_pseudo_insn_table): Likewise.
     	* config/tc-m68k.h (md_pseudo_insn_table): Likewise.
     	* config/tc-spu.h (md_pseudo_insn_table): Likewise.
     	* config/tc-xgate.c (md_pseudo_insn_table): Likewise.
     	* config/tc-xgate.h (md_pseudo_insn_table): Likewise.
     	* config/tc-xtensa.c (md_pseudo_insn_table): Likewise.
     	* config/tc-xtensa.h (md_pseudo_insn_table): Likewise.
     	* config/tc-z80.h (md_pseudo_insn_table): Likewise.
     	* config/tc-spu.c (md_pseudo_table): Remove set, .set,
     	eqv and .eqv.
     	(md_pseudo_insn_table): New.
     	* config/tc-z80.c (md_pseudo_table): Remove set.
     	(md_pseudo_insn_table): New.
     ---
      gas/config/tc-m68hc11.c |  8 ++++++++
      gas/config/tc-m68hc11.h |  3 +++
      gas/config/tc-m68k.c    |  8 ++++++++
      gas/config/tc-m68k.h    |  3 +++
      gas/config/tc-spu.c     | 16 ++++++++++------
      gas/config/tc-spu.h     |  3 +++
      gas/config/tc-xgate.c   |  8 ++++++++
      gas/config/tc-xgate.h   |  3 +++
      gas/config/tc-xtensa.c  |  8 ++++++++
      gas/config/tc-xtensa.h  |  3 +++
      gas/config/tc-z80.c     | 10 +++++++++-
      gas/config/tc-z80.h     |  3 +++
      gas/read.c              | 36 ++++++++++++++++++++++++++++++++++--
      13 files changed, 103 insertions(+), 9 deletions(-)
     
     diff --git a/gas/config/tc-m68hc11.c b/gas/config/tc-m68hc11.c
     index 31ff9815b0..f93b2b3262 100644
     --- a/gas/config/tc-m68hc11.c
     +++ b/gas/config/tc-m68hc11.c
     @@ -320,6 +320,14 @@ const pseudo_typeS md_pseudo_table[] =
      
        {0, 0, 0}
      };
     +
     +/* This table describes all generic pseudo-ops which are also machine
     +   specific instructions.  */
     +const pseudo_typeS md_pseudo_insn_table[] =
     +{
     +  {"nop", NULL, 0},
     +  {NULL, NULL, 0}			/* End sentinel.  */
     +};
      
      /* Options and initialization.  */
      
     diff --git a/gas/config/tc-m68hc11.h b/gas/config/tc-m68hc11.h
     index e6bfd74e96..96764de5bd 100644
     --- a/gas/config/tc-m68hc11.h
     +++ b/gas/config/tc-m68hc11.h
     @@ -31,6 +31,9 @@ struct fix;
      /* Motorola assembler specs does not require '.' before pseudo-ops.  */
      #define NO_PSEUDO_DOT 1
      
     +extern const pseudo_typeS md_pseudo_insn_table[];
     +#define md_pseudo_insn_table md_pseudo_insn_table
     +
      /* The target BFD architecture.  */
      #define TARGET_ARCH (m68hc11_arch ())
      extern enum bfd_architecture m68hc11_arch (void);
     diff --git a/gas/config/tc-m68k.c b/gas/config/tc-m68k.c
     index 13fb897012..6ca01abd9c 100644
     --- a/gas/config/tc-m68k.c
     +++ b/gas/config/tc-m68k.c
     @@ -979,6 +979,14 @@ const pseudo_typeS mote_pseudo_table[] =
        {0, 0, 0}
      };
      
     +/* This table describes all generic pseudo-ops which are also machine
     +   specific instructions.  */
     +const pseudo_typeS md_pseudo_insn_table[] =
     +{
     +  {"nop", NULL, 0},
     +  {NULL, NULL, 0}			/* End sentinel.  */
     +};
     +
      /* Truncate and sign-extend at 32 bits, so that building on a 64-bit host
         gives identical results to a 32-bit host.  */
      #define TRUNC(X)	((valueT) (X) & 0xffffffff)
     diff --git a/gas/config/tc-m68k.h b/gas/config/tc-m68k.h
     index 42f69845e7..901c164f41 100644
     --- a/gas/config/tc-m68k.h
     +++ b/gas/config/tc-m68k.h
     @@ -93,6 +93,9 @@ extern const char *m68k_comment_chars;
      #define NO_PSEUDO_DOT 1
      #endif
      
     +extern const pseudo_typeS md_pseudo_insn_table[];
     +#define md_pseudo_insn_table md_pseudo_insn_table
     +
      extern void m68k_mri_mode_change (int);
      #define MRI_MODE_CHANGE(i) m68k_mri_mode_change (i)
      
     diff --git a/gas/config/tc-spu.c b/gas/config/tc-spu.c
     index 474805bc26..9388b9829f 100644
     --- a/gas/config/tc-spu.c
     +++ b/gas/config/tc-spu.c
     @@ -95,15 +95,19 @@ const pseudo_typeS md_pseudo_table[] =
        {"quad", spu_cons, 8},
        {"string", stringer, 8 + 1},
        {"word", spu_cons, 4},
     -  /* Force set to be treated as an instruction.  */
     -  {"set", NULL, 0},
     -  {".set", s_set, 0},
     -  /* Likewise for eqv.  */
     -  {"eqv", NULL, 0},
     -  {".eqv", s_set, -1},
        {0,0,0}
      };
      
     +/* This table describes all generic pseudo-ops which are also machine
     +   specific instructions.  */
     +const pseudo_typeS md_pseudo_insn_table[] =
     +{
     +  {"eqv", NULL, 0},
     +  {"nop", NULL, 0},
     +  {"set", NULL, 0},
     +  {NULL, NULL, 0}			/* End sentinel.  */
     +};
     +
      /* Bits plugged into branch instruction offset field.  */
      unsigned int brinfo;
      
     diff --git a/gas/config/tc-spu.h b/gas/config/tc-spu.h
     index b163cf0557..acfb3fa41c 100644
     --- a/gas/config/tc-spu.h
     +++ b/gas/config/tc-spu.h
     @@ -73,6 +73,9 @@ struct tc_fix_info {
      /* The spu uses pseudo-ops with no leading period.  */
      #define NO_PSEUDO_DOT 1
      
     +extern const pseudo_typeS md_pseudo_insn_table[];
     +#define md_pseudo_insn_table md_pseudo_insn_table
     +
      /* Don't warn on word overflow; it happens on %hi relocs.  */
      #undef WARN_SIGNED_OVERFLOW_WORD
      
     diff --git a/gas/config/tc-xgate.c b/gas/config/tc-xgate.c
     index 20dbbedcc7..dad00c634a 100644
     --- a/gas/config/tc-xgate.c
     +++ b/gas/config/tc-xgate.c
     @@ -163,6 +163,14 @@ const pseudo_typeS md_pseudo_table[] =
        {0, 0, 0}
      };
      
     +/* This table describes all generic pseudo-ops which are also machine
     +   specific instructions.  */
     +const pseudo_typeS md_pseudo_insn_table[] =
     +{
     +  {"nop", NULL, 0},
     +  {NULL, NULL, 0}			/* End sentinel.  */
     +};
     +
      const char *md_shortopts = "m:";
      
      struct option md_longopts[] =
     diff --git a/gas/config/tc-xgate.h b/gas/config/tc-xgate.h
     index 1b65f042d1..d86befe8bd 100644
     --- a/gas/config/tc-xgate.h
     +++ b/gas/config/tc-xgate.h
     @@ -31,6 +31,9 @@ struct fix;
      /* Motorola assembler specs does not require '.' before pseudo-ops.  */
      #define NO_PSEUDO_DOT 1
      
     +extern const pseudo_typeS md_pseudo_insn_table[];
     +#define md_pseudo_insn_table md_pseudo_insn_table
     +
      /* The target BFD architecture.  */
      #define TARGET_ARCH (xgate_arch ())
      extern enum bfd_architecture xgate_arch (void);
     diff --git a/gas/config/tc-xtensa.c b/gas/config/tc-xtensa.c
     index 4db7ef57e8..1f89ae61f9 100644
     --- a/gas/config/tc-xtensa.c
     +++ b/gas/config/tc-xtensa.c
     @@ -1136,6 +1136,14 @@ const pseudo_typeS md_pseudo_table[] =
        { NULL, 0, 0 },
      };
      
     +/* This table describes all generic pseudo-ops which are also machine
     +   specific instructions.  */
     +const pseudo_typeS md_pseudo_insn_table[] =
     +{
     +  {"nop", NULL, 0},
     +  {NULL, NULL, 0}			/* End sentinel.  */
     +};
     +
      
      static bfd_boolean
      use_transform (void)
     diff --git a/gas/config/tc-xtensa.h b/gas/config/tc-xtensa.h
     index d423776eb6..81dddfccfe 100644
     --- a/gas/config/tc-xtensa.h
     +++ b/gas/config/tc-xtensa.h
     @@ -381,6 +381,9 @@ extern void xtensa_init (int, char **);
      #define HANDLE_ALIGN(fragP)		xtensa_handle_align (fragP)
      #define MAX_MEM_FOR_RS_ALIGN_CODE	1
      
     +extern const pseudo_typeS md_pseudo_insn_table[];
     +#define md_pseudo_insn_table md_pseudo_insn_table
     +
      
      /* The renumber_section function must be mapped over all the sections
         after calling xtensa_post_relax_hook.  That function is static in
     diff --git a/gas/config/tc-z80.c b/gas/config/tc-z80.c
     index 5a4fd38fce..470fecd487 100644
     --- a/gas/config/tc-z80.c
     +++ b/gas/config/tc-z80.c
     @@ -1832,10 +1832,18 @@ const pseudo_typeS md_pseudo_table[] =
        { "ds",   s_space, 1}, /* Fill with bytes rather than words.  */
        { "dw", cons, 2},
        { "psect", obj_coff_section, 0}, /* TODO: Translate attributes.  */
     -  { "set", 0, 0}, 		/* Real instruction on z80.  */
        { NULL, 0, 0 }
      } ;
      
     +/* This table describes all generic pseudo-ops which are also machine
     +   specific instructions.  */
     +const pseudo_typeS md_pseudo_insn_table[] =
     +{
     +  {"nop", NULL, 0},
     +  {"set", NULL, 0}, 		/* Real instruction on z80.  */
     +  {NULL, NULL, 0}		/* End sentinel.  */
     +};
     +
      static table_t instab[] =
      {
        { "adc",  0x88, 0x4A, emit_adc },
     diff --git a/gas/config/tc-z80.h b/gas/config/tc-z80.h
     index 75f379a793..3f7d122477 100644
     --- a/gas/config/tc-z80.h
     +++ b/gas/config/tc-z80.h
     @@ -93,6 +93,9 @@ extern void z80_cons_fix_new (fragS *, int, int, expressionS *);
      #define SINGLE_QUOTE_STRINGS
      #define NO_STRING_ESCAPES
      
     +extern const pseudo_typeS md_pseudo_insn_table[];
     +#define md_pseudo_insn_table md_pseudo_insn_table
     +
      /* An `.lcomm' directive with no explicit alignment parameter will
         use this macro to set P2VAR to the alignment that a request for
         SIZE bytes will have.  The alignment is expressed as a power of
     diff --git a/gas/read.c b/gas/read.c
     index 9ab88f8962..06c82aeda2 100644
     --- a/gas/read.c
     +++ b/gas/read.c
     @@ -535,6 +535,31 @@ pop_insert (const pseudo_typeS *table)
      #define cfi_pop_insert()	pop_insert(cfi_pseudo_table)
      #endif
      
     +#ifdef md_pseudo_insn_table
     +static struct hash_control *po_pseudo_insn_hash;
     +
     +static void
     +pop_pseudo_insn_insert (void)
     +{
     +  const char *errtxt;
     +  const pseudo_typeS *pop;
     +  po_pseudo_insn_hash = hash_new ();
     +  for (pop = md_pseudo_insn_table; pop->poc_name; pop++)
     +    {
     +      errtxt = hash_insert (po_pseudo_insn_hash, pop->poc_name, (char *) pop);
     +      if (errtxt && (!pop_override_ok || strcmp (errtxt, "exists")))
     +	as_fatal (_("error constructing md insn-op table: %s"),
     +		  errtxt);
     +    }
     +}
     +
     +#define pop_pseudo_insn_find(s) \
     +  ((pseudo_typeS *) hash_find (po_pseudo_insn_hash, (s)))
     +#else
     +#define pop_pseudo_insn_insert()
     +#define pop_pseudo_insn_find(s) NULL
     +#endif
     +
      static void
      pobegin (void)
      {
     @@ -557,6 +582,8 @@ pobegin (void)
        pop_table_name = "cfi";
        pop_override_ok = 1;
        cfi_pop_insert ();
     +
     +  pop_pseudo_insn_insert ();
      }
      
      #define HANDLE_CONDITIONAL_ASSEMBLY(num_read)				\
     @@ -1067,8 +1094,13 @@ read_a_source_file (const char *name)
      		      /* The MRI assembler uses pseudo-ops without
      			 a period.  */
      		      pop = (pseudo_typeS *) hash_find (po_hash, s);
     -		      if (pop != NULL && pop->poc_handler == NULL)
     -			pop = NULL;
     +		      if (pop != NULL)
     +			{
     +			  pseudo_typeS *insn_pop
     +			    = pop_pseudo_insn_find (s);
     +			  if (insn_pop != NULL)
     +			    pop = NULL;
     +			}
      		    }
      
      		  if (pop != NULL
     -- 
     2.14.3
     


-- 
Avoid eavesdropping.  Send strong encrypted email.
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://sks-keyservers.net or any PGP keyserver for public key.

Attachment: signature.asc
Description: Digital signature


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