This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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: PR target/39911: The 'z' suffix doesn't work with 16bit integer insn


H.J. Lu wrote:

This doesn't solve the problem. You need to add the 'w' suffix for integer
instructions with memory operand.

This is how %z always worked and that is excatly the reason why I try to fix
this wrong behaviour.

I am testing a patch in

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39911

Since "%z' never really worked on integer instructions, I made "%Z" for
integer instructions only while providing backward compatibility for existing
asm statements.


This is the patch with a testcase. Tested on Linux/Intel64 with both 32/64 bits.
OK for trunk?

I have done a bit of research and found that:
- %z is _never_ used with any x87 insn outside gcc sources.
- there are usages of %z with integer insns [1] and a couple of bugreports of %z with integer ops.


Due to this, I don't think that we need to provide backward compatibility with _undocumented_ %z for x87 insns.

Attached patch now fixes the PR by introducing "%Z" for all x87 mnemonics, clearly separated from "%z" that is/was primarily intended for integer x86 mnemonics. However, to provide high level of backward compatibility, new %z handling falls through to %Z handling, so the majority (HImode and DImode conversions with x87 integer insns not included) of x87 operations are still handled the way it was before. A warning is emitted when %z is used with FP operand, so the code could eventually be fixed.

To fix the fallout from gcc sources itself, generation of x87 mnemonics now uses %Z instead of %z.

Instead of ICEing, the compiler now produces an informative message to tell what is wrong with %z and %Z modified operand. Since users are using them (well %z at least), I think that we can document these two modifiers and put them into general use.

2009-04-27 Uros Bizjak <ubizjak@gmail.com>

   PR target/39911
   * config/i386/i386.c (print_operand) ['Z']: Handle floating point
   and integer modes for x87 operands.  Do not ICE for unsupported size,
   generate error instead.  Generate error for unsupported operand types.
   ['z']: Do not handle HImode memory operands specially.  Warning
   for floating-point operands.  Fallthru to 'Z' for unsupported operand
   types.  Do not ICE for unsupported size, generate error instead.
   (output_387_binary_op): Use %Z to output operands.
   (output_fp_compare): Ditto.
   (output_387_reg_move): Ditto.

testsuite/ChangeLog:

2009-04-27  Uros Bizjak  <ubizjak@gmail.com>
       H.J. Lu  <hongjiu.lu@intel.com>

   PR target/39911
   * gcc.target/i386/pr39911.c: New test.

Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu {,-m32}.

[1] http://www.google.com/codesearch/p?hl=en#whSq6LR797E/src/arch/i386/include/libkir.h&q=mov%25z

Uros.

Index: testsuite/gcc.target/i386/pr39911.c
===================================================================
--- testsuite/gcc.target/i386/pr39911.c	(revision 0)
+++ testsuite/gcc.target/i386/pr39911.c	(revision 0)
@@ -0,0 +1,57 @@
+/* { dg-do assemble } */
+/* { dg-options "-O2" } */
+
+void 
+bar1 () 
+{
+  char foo;
+  asm volatile ("mov%z0 %1, %0": "=m" (foo): "iq" (-23));
+  asm volatile ("add%z0 %1, %0": "+m" (foo): "iq" (23));
+  asm volatile ("mov%z0 %1, %0": "=q" (foo): "iq" (-23));
+  asm volatile ("add%z0 %1, %0": "+q" (foo): "iq" (23));
+}
+
+void
+bar2 () 
+{
+  short foo;
+  asm volatile ("mov%z0 %1, %0": "=m" (foo): "ir" (-23));
+  asm volatile ("add%z0 %1, %0": "+m" (foo): "ir" (23));
+  asm volatile ("mov%z0 %1, %0": "=r" (foo): "ir" (-23));
+  asm volatile ("add%z0 %1, %0": "+r" (foo): "ir" (23));
+
+  asm volatile ("pop%z0 %0": "=m" (foo));
+  asm volatile ("pop%z0 %0": "=r" (foo));
+}
+
+void
+bar3 () 
+{
+  int foo;
+  asm volatile ("mov%z0 %1, %0": "=m" (foo): "ir" (-23));
+  asm volatile ("add%z0 %1, %0": "+m" (foo): "ir" (23));
+  asm volatile ("mov%z0 %1, %0": "=r" (foo): "ir" (-23));
+  asm volatile ("add%z0 %1, %0": "+r" (foo): "ir" (23));
+
+  if (sizeof (void *) == sizeof (int))
+    {
+      asm volatile ("pop%z0 %0": "=m" (foo));
+      asm volatile ("pop%z0 %0": "=r" (foo));
+    }
+}
+
+void
+bar4 () 
+{
+  if (sizeof (void *) == sizeof (long long))
+    {
+      long long foo;
+      asm volatile ("mov%z0 %1, %0": "=m" (foo): "er" (-23));
+      asm volatile ("add%z0 %1, %0": "+m" (foo): "er" (23));
+      asm volatile ("mov%z0 %1, %0": "=r" (foo): "er" (-23));
+      asm volatile ("add%z0 %1, %0": "+r" (foo): "er" (23));
+
+      asm volatile ("pop%z0 %0": "=m" (foo));
+      asm volatile ("pop%z0 %0": "=r" (foo));
+    }
+}
Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md	(revision 146825)
+++ config/i386/i386.md	(working copy)
@@ -36,7 +36,7 @@
 ;;      otherwise nothing
 ;; R -- print the prefix for register names.
 ;; z -- print the opcode suffix for the size of the current operand.
-;; Z -- likewise, with special suffixes for fild/fist instructions.
+;; Z -- likewise, with special suffixes for x87 instructions.
 ;; * -- print a star (in certain assembler syntax)
 ;; A -- print an absolute memory reference.
 ;; w -- print the operand as if it's a "word" (HImode) even if it isn't.
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 146825)
+++ config/i386/i386.c	(working copy)
@@ -10848,7 +10848,7 @@ get_some_local_dynamic_name (void)
         otherwise nothing
    R -- print the prefix for register names.
    z -- print the opcode suffix for the size of the current operand.
-   Z -- likewise, with special suffixes for fild/fist instructions.
+   Z -- likewise, with special suffixes for x87 instructions.
    * -- print a star (in certain assembler syntax)
    A -- print an absolute memory reference.
    w -- print the operand as if it's a "word" (HImode) even if it isn't.
@@ -10947,91 +10947,111 @@ print_operand (FILE *file, rtx x, int co
 	    putc ('t', file);
 	  return;
 
-	case 'Z':
-	  gcc_assert (MEM_P (x));
+	case 'z':
+	  if (GET_MODE_CLASS (GET_MODE (x)) == MODE_INT)
+	    {
+	      /* Opcodes don't get size suffixes if using Intel opcodes.  */
+	      if (ASSEMBLER_DIALECT == ASM_INTEL)
+		return;
+
+	      switch (GET_MODE_SIZE (GET_MODE (x)))
+		{
+		case 1:
+		  putc ('b', file);
+		  return;
+
+		case 2:
+		  putc ('w', file);
+		  return;
+
+		case 4:
+		  putc ('l', file);
+		  return;
+
+		case 8:
+		  putc ('q', file);
+		  return;
+
+		default:
+		  output_operand_lossage
+		    ("invalid operand size for operand code '%c'", code);
+		  return;
+		}
+	    }
 
-	  /* fild/fist don't get size suffixes if using Intel opcodes.  */
+	  if (GET_MODE_CLASS (GET_MODE (x)) == MODE_FLOAT)
+	    warning
+	      (0, "non-integer operand used with operand code '%c'", code);
+	  /* FALLTHRU */
+
+	case 'Z':
+	  /* 387 opcodes don't get size suffixes if using Intel opcodes.  */
 	  if (ASSEMBLER_DIALECT == ASM_INTEL)
 	    return;
 
-	  switch (GET_MODE_SIZE (GET_MODE (x)))
+	  if (GET_MODE_CLASS (GET_MODE (x)) == MODE_INT)
 	    {
-	    case 2:
+	      switch (GET_MODE_SIZE (GET_MODE (x)))
+		{
+		case 2:
 #ifdef HAVE_AS_IX86_FILDS
-	      putc ('s', file);
+		  putc ('s', file);
 #endif
-	      return;
+		  return;
 
-	    case 4:
-	      putc ('l', file);
-	      return;
+		case 4:
+		  putc ('l', file);
+		  return;
 
-	    case 8:
+		case 8:
 #ifdef HAVE_AS_IX86_FILDQ
-	      putc ('q', file);
+		  putc ('q', file);
 #else
-	      fputs ("ll", file);
+		  fputs ("ll", file);
 #endif
-	      return;
+		  return;
 
-	    default:
-	      gcc_unreachable ();
+		default:
+		  break;
+		}
 	    }
-	    
-	case 'z':
-	  /* 387 opcodes don't get size suffixes if the operands are
-	     registers.  */
-	  if (STACK_REG_P (x))
-	    return;
-
-	  /* Likewise if using Intel opcodes.  */
-	  if (ASSEMBLER_DIALECT == ASM_INTEL)
-	    return;
-
-	  /* This is the size of op from size of operand.  */
-	  switch (GET_MODE_SIZE (GET_MODE (x)))
+	  else if (GET_MODE_CLASS (GET_MODE (x)) == MODE_FLOAT)
 	    {
-	    case 1:
-	      putc ('b', file);
-	      return;
+	      /* 387 opcodes don't get size suffixes
+		 if the operands are registers.  */
+	      if (STACK_REG_P (x))
+		return;
 
-	    case 2:
-	      /* ??? This fails for HImode integer
-		 operator with memory operand.  */
-	      if (MEM_P (x))
+	      switch (GET_MODE_SIZE (GET_MODE (x)))
 		{
-#ifdef HAVE_AS_IX86_FILDS
+		case 4:
 		  putc ('s', file);
-#endif
 		  return;
-		}
-	      else
-		putc ('w', file);
-	      return;
 
-	    case 4:
-	      if (GET_MODE_CLASS (GET_MODE (x)) == MODE_INT)
-		putc ('l', file);
-	      else
-		putc ('s', file);
-	      return;
+		case 8:
+		  putc ('l', file);
+		  return;
 
-	    case 8:
-	      if (GET_MODE_CLASS (GET_MODE (x)) == MODE_INT)
-		putc ('q', file);
-	      else
-	        putc ('l', file);
-	      return;
+		case 12:
+		case 16:
+		  putc ('t', file);
+		  return;
 
-	    case 12:
-	    case 16:
-	      putc ('t', file);
+		default:
+		  break;
+		}
+	    }
+	  else
+	    {
+	      output_operand_lossage
+		("invalid operand type used with operand code '%c'", code);
 	      return;
-
-	    default:
-	      gcc_unreachable ();
 	    }
 
+	  output_operand_lossage
+	    ("invalid operand size for operand code '%c'", code);
+	  return;
+	    
 	case 'd':
 	case 'b':
 	case 'w':
@@ -11830,7 +11850,7 @@ output_387_binary_op (rtx insn, rtx *ope
 
       if (MEM_P (operands[2]))
 	{
-	  p = "%z2\t%2";
+	  p = "%Z2\t%2";
 	  break;
 	}
 
@@ -11860,13 +11880,13 @@ output_387_binary_op (rtx insn, rtx *ope
     case DIV:
       if (MEM_P (operands[1]))
 	{
-	  p = "r%z1\t%1";
+	  p = "r%Z1\t%1";
 	  break;
 	}
 
       if (MEM_P (operands[2]))
 	{
-	  p = "%z2\t%2";
+	  p = "%Z2\t%2";
 	  break;
 	}
 
@@ -12238,13 +12258,13 @@ output_fp_compare (rtx insn, rtx *operan
 
       static const char * const alt[16] =
       {
-	"fcom%z2\t%y2\n\tfnstsw\t%0",
-	"fcomp%z2\t%y2\n\tfnstsw\t%0",
-	"fucom%z2\t%y2\n\tfnstsw\t%0",
-	"fucomp%z2\t%y2\n\tfnstsw\t%0",
+	"fcom%Z2\t%y2\n\tfnstsw\t%0",
+	"fcomp%Z2\t%y2\n\tfnstsw\t%0",
+	"fucom%Z2\t%y2\n\tfnstsw\t%0",
+	"fucomp%Z2\t%y2\n\tfnstsw\t%0",
 
-	"ficom%z2\t%y2\n\tfnstsw\t%0",
-	"ficomp%z2\t%y2\n\tfnstsw\t%0",
+	"ficom%Z2\t%y2\n\tfnstsw\t%0",
+	"ficomp%Z2\t%y2\n\tfnstsw\t%0",
 	NULL,
 	NULL,
 
@@ -28779,22 +28799,22 @@ output_387_reg_move (rtx insn, rtx *oper
 	  return "fstp\t%y0";
 	}
       if (STACK_TOP_P (operands[0]))
-	return "fld%z1\t%y1";
+	return "fld%Z1\t%y1";
       return "fst\t%y0";
     }
   else if (MEM_P (operands[0]))
     {
       gcc_assert (REG_P (operands[1]));
       if (find_regno_note (insn, REG_DEAD, REGNO (operands[1])))
-	return "fstp%z0\t%y0";
+	return "fstp%Z0\t%y0";
       else
 	{
 	  /* There is no non-popping store to memory for XFmode.
 	     So if we need one, follow the store with a load.  */
 	  if (GET_MODE (operands[0]) == XFmode)
-	    return "fstp%z0\t%y0\n\tfld%z0\t%y0";
+	    return "fstp%Z0\t%y0\n\tfld%Z0\t%y0";
 	  else
-	    return "fst%z0\t%y0";
+	    return "fst%Z0\t%y0";
 	}
     }
   else

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