This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] x86: Add .nop directive to assembler
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