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]

[PATCH] Replace IgnoreSize/DefaultSize with MnemonicSize


On Tue, Mar 3, 2020 at 6:50 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 03.03.2020 15:09, H.J. Lu wrote:
> > I am testing this patch with GCC 8.  I will check it in if it fixes
> > regressions in GCC 8 testsuits:
> >
> > https://gcc.gnu.org/ml/gcc-regression/2020-03/msg00008.html
> >
> > H.J.
> > ---
> > According to gas manual, suffix in instruction mnemonics isn't always
> > required:
> >
> > When there is no sizing suffix and no (suitable) register operands to
> > deduce the size of memory operands, with a few exceptions and where long
> > operand size is possible in the first place, operand size will default
> > to long in 32- and 64-bit modes.
>
> Nothing there says that this defaulting is to happen silently. Yet
> _that's_ what my earlier changes altered. The defaulting is still
> the same. And no - SUCH CASES SHOULD NOT GO SILENTLY, neither here
> nor in the MOVSX/MOVZX case. Ambiguities should _always_ be
> pointed out by the assembler. (There may be [and there is] a mode
> in which this goes silently, to be enabled at the programmer's
> risk.)

It is not going to happen in AT&T syntax.   Gas has to support older GCC
without any warnings.

> > This includes cvtsi2sd, cvtsi2ss, vcvtsi2sd, vcvtsi2ss, vcvtusi2sd and
> > vcvtusi2ss.  Since they are used in GCC 8 and older GCC releases, they
> > must be allowed without suffix in AT&T syntax.
> >
> > gas/
> >
> >       PR gas/25622
> >       * testsuite/gas/i386/i386.exp: Run x86-64-default-suffix and
> >       x86-64-default-suffix-avx.
> >       * testsuite/gas/i386/noreg64.s: Remove cvtsi2sd, cvtsi2ss,
> >       vcvtsi2sd, vcvtsi2ss, vcvtusi2sd and vcvtusi2ss entries.
> >       * testsuite/gas/i386/noreg64.d: Updated.
> >       * testsuite/gas/i386/noreg64.l: Likewise.
> >       * testsuite/gas/i386/x86-64-default-suffix-avx.d: New file.
> >       * testsuite/gas/i386/x86-64-default-suffix.d: Likewise.
> >       * testsuite/gas/i386/x86-64-default-suffix.s: Likewise.
> >
> > opcodes/
> >
> >       PR gas/25622
> >       * i386-opc.tbl: Add IgnoreSize to cvtsi2sd, cvtsi2ss, vcvtsi2sd,
> >       vcvtsi2ss, vcvtusi2sd and vcvtusi2ss for AT&T syntax.
>
> Oh no. I'm trying to clean up the IgnoreSize mess and you want to
> add new instances for no good reason (yes, there are cases where
> this is actually missing; hopefully I'll get to send out the
> series later this week).

Since an instruction template can't have both IgnoreSize and DefaultSize,
I am testing this patch and will check it if there are no regressions.  Then
we can add one value to MnemonicSize.

> I know I can't prevent this going in, but I'm heavily opposed.
> You don't "fix" anything here, you break things.
>

I disagree.


-- 
H.J.
From e29697adabb46380ccee5f8c65560836fbebe449 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 3 Mar 2020 09:10:36 -0800
Subject: [PATCH] Replace IgnoreSize/DefaultSize with MnemonicSize

---
 gas/config/tc-i386.c |    28 +-
 opcodes/i386-gen.c   |     3 +-
 opcodes/i386-opc.h   |     8 +-
 opcodes/i386-opc.tbl |     3 +
 opcodes/i386-tbl.h   | 21702 ++++++++++++++++++++---------------------
 5 files changed, 10874 insertions(+), 10870 deletions(-)

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index dc745aa7d2..af0a1cb960 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -5876,7 +5876,7 @@ match_template (char mnem_suffix)
       if (i.suffix == QWORD_MNEM_SUFFIX
 	  && flag_code != CODE_64BIT
 	  && (intel_syntax
-	      ? (!t->opcode_modifier.ignoresize
+	      ? (t->opcode_modifier.mnemonicsize != IGNORESIZE
 	         && !t->opcode_modifier.broadcast
 		 && !intel_float_operand (t->name))
 	      : intel_float_operand (t->name) != 2)
@@ -5892,7 +5892,7 @@ match_template (char mnem_suffix)
       else if (i.suffix == LONG_MNEM_SUFFIX
 	       && !cpu_arch_flags.bitfield.cpui386
 	       && (intel_syntax
-		   ? (!t->opcode_modifier.ignoresize
+		   ? (t->opcode_modifier.mnemonicsize != IGNORESIZE
 		      && !intel_float_operand (t->name))
 		   : intel_float_operand (t->name) != 2)
 	       && ((operand_types[0].bitfield.class != RegMMX
@@ -6247,7 +6247,7 @@ match_template (char mnem_suffix)
 	as_warn (_("indirect %s without `*'"), t->name);
 
       if (t->opcode_modifier.isprefix
-	  && t->opcode_modifier.ignoresize)
+	  && t->opcode_modifier.mnemonicsize == IGNORESIZE)
 	{
 	  /* Warn them that a data or address size prefix doesn't
 	     affect assembly of the next line of code.  */
@@ -6371,7 +6371,7 @@ process_suffix (void)
       else if (i.suffix == BYTE_MNEM_SUFFIX)
 	{
 	  if (intel_syntax
-	      && i.tm.opcode_modifier.ignoresize
+	      && i.tm.opcode_modifier.mnemonicsize == IGNORESIZE
 	      && i.tm.opcode_modifier.no_bsuf)
 	    i.suffix = 0;
 	  else if (!check_byte_reg ())
@@ -6380,7 +6380,7 @@ process_suffix (void)
       else if (i.suffix == LONG_MNEM_SUFFIX)
 	{
 	  if (intel_syntax
-	      && i.tm.opcode_modifier.ignoresize
+	      && i.tm.opcode_modifier.mnemonicsize == IGNORESIZE
 	      && i.tm.opcode_modifier.no_lsuf
 	      && !i.tm.opcode_modifier.todword
 	      && !i.tm.opcode_modifier.toqword)
@@ -6391,7 +6391,7 @@ process_suffix (void)
       else if (i.suffix == QWORD_MNEM_SUFFIX)
 	{
 	  if (intel_syntax
-	      && i.tm.opcode_modifier.ignoresize
+	      && i.tm.opcode_modifier.mnemonicsize == IGNORESIZE
 	      && i.tm.opcode_modifier.no_qsuf
 	      && !i.tm.opcode_modifier.todword
 	      && !i.tm.opcode_modifier.toqword)
@@ -6402,13 +6402,14 @@ process_suffix (void)
       else if (i.suffix == WORD_MNEM_SUFFIX)
 	{
 	  if (intel_syntax
-	      && i.tm.opcode_modifier.ignoresize
+	      && i.tm.opcode_modifier.mnemonicsize == IGNORESIZE
 	      && i.tm.opcode_modifier.no_wsuf)
 	    i.suffix = 0;
 	  else if (!check_word_reg ())
 	    return 0;
 	}
-      else if (intel_syntax && i.tm.opcode_modifier.ignoresize)
+      else if (intel_syntax
+	       && i.tm.opcode_modifier.mnemonicsize == IGNORESIZE)
 	/* Do nothing if the instruction is going to ignore the prefix.  */
 	;
       else
@@ -6417,7 +6418,8 @@ process_suffix (void)
       /* Undo the movsx/movzx change done above.  */
       i.operands = numop;
     }
-  else if (i.tm.opcode_modifier.defaultsize && !i.suffix)
+  else if (i.tm.opcode_modifier.mnemonicsize == DEFAULTSIZE
+	   && !i.suffix)
     {
       i.suffix = stackop_size;
       if (stackop_size == LONG_MNEM_SUFFIX)
@@ -6466,12 +6468,12 @@ process_suffix (void)
     }
 
   if (!i.suffix
-      && (!i.tm.opcode_modifier.defaultsize
+      && (i.tm.opcode_modifier.mnemonicsize != DEFAULTSIZE
 	  /* Also cover lret/retf/iret in 64-bit mode.  */
 	  || (flag_code == CODE_64BIT
 	      && !i.tm.opcode_modifier.no_lsuf
 	      && !i.tm.opcode_modifier.no_qsuf))
-      && !i.tm.opcode_modifier.ignoresize
+      && i.tm.opcode_modifier.mnemonicsize != IGNORESIZE
       /* Accept FLDENV et al without suffix.  */
       && (i.tm.opcode_modifier.no_ssuf || i.tm.opcode_modifier.floatmf))
     {
@@ -6544,7 +6546,7 @@ process_suffix (void)
       if (suffixes & (suffixes - 1))
 	{
 	  if (intel_syntax
-	      && (!i.tm.opcode_modifier.defaultsize
+	      && (i.tm.opcode_modifier.mnemonicsize != DEFAULTSIZE
 		  || operand_check == check_error))
 	    {
 	      as_bad (_("ambiguous operand size for `%s'"), i.tm.name);
@@ -6638,7 +6640,7 @@ process_suffix (void)
 	 size prefix, except for instructions that will ignore this
 	 prefix anyway.  */
       if (i.suffix != QWORD_MNEM_SUFFIX
-	  && !i.tm.opcode_modifier.ignoresize
+	  && i.tm.opcode_modifier.mnemonicsize != IGNORESIZE
 	  && !i.tm.opcode_modifier.floatmf
 	  && !is_any_vex_encoding (&i.tm)
 	  && ((i.suffix == LONG_MNEM_SUFFIX) == (flag_code == CODE_16BIT)
diff --git a/opcodes/i386-gen.c b/opcodes/i386-gen.c
index 52e6b3e21a..ac7852ee19 100644
--- a/opcodes/i386-gen.c
+++ b/opcodes/i386-gen.c
@@ -622,8 +622,7 @@ static bitfield opcode_modifiers[] =
   BITFIELD (FloatR),
   BITFIELD (Size),
   BITFIELD (CheckRegSize),
-  BITFIELD (IgnoreSize),
-  BITFIELD (DefaultSize),
+  BITFIELD (MnemonicSize),
   BITFIELD (Anysize),
   BITFIELD (No_bSuf),
   BITFIELD (No_wSuf),
diff --git a/opcodes/i386-opc.h b/opcodes/i386-opc.h
index fc69d4d0fb..1718ca2751 100644
--- a/opcodes/i386-opc.h
+++ b/opcodes/i386-opc.h
@@ -427,9 +427,10 @@ enum
   CheckRegSize,
   /* instruction ignores operand size prefix and in Intel mode ignores
      mnemonic size suffix check.  */
-  IgnoreSize,
+#define IGNORESIZE	1
   /* default insn size depends on mode */
-  DefaultSize,
+#define DEFAULTSIZE	2
+  MnemonicSize,
   /* any memory size */
   Anysize,
   /* b suffix on instruction illegal */
@@ -661,8 +662,7 @@ typedef struct i386_opcode_modifier
   unsigned int floatr:1;
   unsigned int size:2;
   unsigned int checkregsize:1;
-  unsigned int ignoresize:1;
-  unsigned int defaultsize:1;
+  unsigned int mnemonicsize:2;
   unsigned int anysize:1;
   unsigned int no_bsuf:1;
   unsigned int no_wsuf:1;
diff --git a/opcodes/i386-opc.tbl b/opcodes/i386-opc.tbl
index 69b9cf5235..2c8c0f8f3c 100644
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -66,6 +66,9 @@
 #define Size32 Size=SIZE32
 #define Size64 Size=SIZE64
 
+#define IgnoreSize	MnemonicSize=IGNORESIZE
+#define DefaultSize	MnemonicSize=DEFAULTSIZE
+
 // RegMem implies a ModR/M byte
 #define RegMem Modrm|RegMem
 

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