public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
@ 2009-04-26 20:47 H.J. Lu
  2009-04-27 12:16 ` Uros Bizjak
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2009-04-26 20:47 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2322 bytes --]

On Sun, Apr 26, 2009 at 11:46 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sun, Apr 26, 2009 at 10:00 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> H.J. Lu wrote:
>>>
>>> On Sat, Apr 25, 2009 at 1:15 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>
>>>>
>>>> Hello!
>>>>
>>>> Attached patch reverts %z handling of HImode operands back to previous
>>>> (wrong) state until better fix is found. Unfortunately, integer and
>>>> x87 operators don't agree which suffix to use for integer operands.
>>>>
>>>> 2009-04-25  Uros Bizjak  <ubizjak@gmail.com>
>>>>
>>>>       PR target/39897
>>>>       * config/i386/i386.c (print_operand) ['z']: Revert handling of
>>>>       HImode operands.
>>>>
>>>> Tested on x86_64-pc-linux-gnu {,-m32}, committed to mainline.
>>>>
>>>>
>>>
>>> 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?

Thanks.

-- 
H.J.
---
gcc/

2009-04-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/39911
	* config/i386/i386.c (ix86_file_end): Replace "%z0" with "%Z0".
	(output_set_got): Likewise.
	(output_set_got): Likewise.
	(x86_output_mi_thunk): Likewise.
	(print_operand): Restore 'z' handling.  Make 'Z' for integer
	insntructions only.
	(output_fix_trunc): Use '%z' to output suffix of fist{,p,tp} insn.

	* config/i386/i386.md (*floathi<mode>2_i387): Use '%z' to output
	suffix of fild insn.
	(*floatsi<mode>2_vector_mixed): Ditto.
	(*float<SSEMODEI24:mode><MODEF:mode>2_mixed_interunit): Ditto.
	(*float<SSEMODEI24:mode><MODEF:mode>2_mixed_nointerunit): Ditto.
	(*float<SSEMODEI24:mode><X87MODEF:mode>2_i387_with_temp): Ditto.
	(*float<SSEMODEI24:mode><X87MODEF:mode>2_i387): Ditto.

gcc/testsuite/

2009-04-26  H.J. Lu  <hongjiu.lu@intel.com>

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

[-- Attachment #2: gcc-pr39911-2.patch --]
[-- Type: text/plain, Size: 12315 bytes --]

gcc/

2009-04-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/39911
	* config/i386/i386.c (ix86_file_end): Replace "%z0" with "%Z0".
	(output_set_got): Likewise.
	(output_set_got): Likewise.
	(x86_output_mi_thunk): Likewise.
	(print_operand): Restore 'z' handling.  Make 'Z' for integer
	insntructions only.
	(output_fix_trunc): Use '%z' to output suffix of fist{,p,tp} insn.

	* config/i386/i386.md (*floathi<mode>2_i387): Use '%z' to output
	suffix of fild insn.
	(*floatsi<mode>2_vector_mixed): Ditto.
	(*float<SSEMODEI24:mode><MODEF:mode>2_mixed_interunit): Ditto.
	(*float<SSEMODEI24:mode><MODEF:mode>2_mixed_nointerunit): Ditto.
	(*float<SSEMODEI24:mode><X87MODEF:mode>2_i387_with_temp): Ditto.
	(*float<SSEMODEI24:mode><X87MODEF:mode>2_i387): Ditto.

gcc/testsuite/

2009-04-26  H.J. Lu  <hongjiu.lu@intel.com>

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

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,55 @@
+/* { dg-do assemble } */
+/* { dg-options "-O2" } */
+
+void 
+bar1 () 
+{
+  char 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" : "=q"(foo) : "ir"(23));
+  asm volatile ("add%z0 %1, %0": "+q" (foo): "ir" (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));
+}
+
+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));
+      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): "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));
+      asm volatile ("pop%z0 %0": : "r" (foo));
+    }
+}
Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md	(revision 146807)
+++ config/i386/i386.md	(working copy)
@@ -35,8 +35,9 @@
 ;; O -- if HAVE_AS_IX86_CMOV_SUN_SYNTAX, expand to "w.", "l." or "q.",
 ;;      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 -- print the integer instruction suffix for the size of the current
+;;	operand
+;; 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.
@@ -5141,7 +5142,7 @@
   "TARGET_80387
    && (!(SSE_FLOAT_MODE_P (<MODE>mode) && TARGET_SSE_MATH)
        || TARGET_MIX_SSE_I387)"
-  "fild%Z1\t%1"
+  "fild%z1\t%1"
   [(set_attr "type" "fmov")
    (set_attr "mode" "<MODE>")
    (set_attr "fp_int_src" "true")])
@@ -5251,7 +5252,7 @@
   "TARGET_SSE2 && TARGET_MIX_SSE_I387
    && TARGET_USE_VECTOR_CONVERTS && optimize_function_for_speed_p (cfun)"
   "@
-   fild%Z1\t%1
+   fild%z1\t%1
    #"
   [(set_attr "type" "fmov,sseicvt")
    (set_attr "mode" "<MODE>,<ssevecmode>")
@@ -5312,7 +5313,7 @@
    && SSE_FLOAT_MODE_P (<MODEF:MODE>mode) && TARGET_MIX_SSE_I387
    && (TARGET_INTER_UNIT_CONVERSIONS || optimize_function_for_size_p (cfun))"
   "@
-   fild%Z1\t%1
+   fild%z1\t%1
    %vcvtsi2s<MODEF:ssemodefsuffix><SSEMODEI24:rex64suffix>\t{%1, %d0|%d0, %1}
    %vcvtsi2s<MODEF:ssemodefsuffix><SSEMODEI24:rex64suffix>\t{%1, %d0|%d0, %1}"
   [(set_attr "type" "fmov,sseicvt,sseicvt")
@@ -5331,7 +5332,7 @@
    && SSE_FLOAT_MODE_P (<MODEF:MODE>mode) && TARGET_MIX_SSE_I387
    && !(TARGET_INTER_UNIT_CONVERSIONS || optimize_function_for_size_p (cfun))"
   "@
-   fild%Z1\t%1
+   fild%z1\t%1
    %vcvtsi2s<MODEF:ssemodefsuffix><SSEMODEI24:rex64suffix>\t{%1, %d0|%d0, %1}"
   [(set_attr "type" "fmov,sseicvt")
    (set_attr "prefix" "orig,maybe_vex")
@@ -5581,7 +5582,7 @@
   "TARGET_80387
    && X87_ENABLE_FLOAT (<X87MODEF:MODE>mode, <SSEMODEI24:MODE>mode)"
   "@
-   fild%Z1\t%1
+   fild%z1\t%1
    #"
   [(set_attr "type" "fmov,multi")
    (set_attr "mode" "<X87MODEF:MODE>")
@@ -5594,7 +5595,7 @@
 	  (match_operand:SSEMODEI24 1 "memory_operand" "m")))]
   "TARGET_80387
    && X87_ENABLE_FLOAT (<X87MODEF:MODE>mode, <SSEMODEI24:MODE>mode)"
-  "fild%Z1\t%1"
+  "fild%z1\t%1"
   [(set_attr "type" "fmov")
    (set_attr "mode" "<X87MODEF:MODE>")
    (set_attr "fp_int_src" "true")])
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 146807)
+++ config/i386/i386.c	(working copy)
@@ -7491,7 +7491,7 @@ ix86_file_end (void)
 
       xops[0] = gen_rtx_REG (Pmode, regno);
       xops[1] = gen_rtx_MEM (Pmode, stack_pointer_rtx);
-      output_asm_insn ("mov%z0\t{%1, %0|%0, %1}", xops);
+      output_asm_insn ("mov%Z0\t{%1, %0|%0, %1}", xops);
       output_asm_insn ("ret", xops);
     }
 
@@ -7531,7 +7531,7 @@ output_set_got (rtx dest, rtx label ATTR
       xops[2] = gen_rtx_LABEL_REF (Pmode, label ? label : gen_label_rtx ());
 
       if (!flag_pic)
-	output_asm_insn ("mov%z0\t{%2, %0|%0, %2}", xops);
+	output_asm_insn ("mov%Z0\t{%2, %0|%0, %2}", xops);
       else
 	output_asm_insn ("call\t%a2", xops);
 
@@ -7546,7 +7546,7 @@ output_set_got (rtx dest, rtx label ATTR
 				 CODE_LABEL_NUMBER (XEXP (xops[2], 0)));
 
       if (flag_pic)
-	output_asm_insn ("pop%z0\t%0", xops);
+	output_asm_insn ("pop%Z0\t%0", xops);
     }
   else
     {
@@ -7572,9 +7572,9 @@ output_set_got (rtx dest, rtx label ATTR
     return "";
 
   if (!flag_pic || TARGET_DEEP_BRANCH_PREDICTION)
-    output_asm_insn ("add%z0\t{%1, %0|%0, %1}", xops);
+    output_asm_insn ("add%Z0\t{%1, %0|%0, %1}", xops);
   else
-    output_asm_insn ("add%z0\t{%1+[.-%a2], %0|%0, %1+(.-%a2)}", xops);
+    output_asm_insn ("add%Z0\t{%1+[.-%a2], %0|%0, %1+(.-%a2)}", xops);
 
   return "";
 }
@@ -10847,8 +10847,9 @@ get_some_local_dynamic_name (void)
    O -- if HAVE_AS_IX86_CMOV_SUN_SYNTAX, expand to "w.", "l." or "q.",
         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 -- print the integer instruction suffix for the size of the current
+	operand
+   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,44 +10948,72 @@ print_operand (FILE *file, rtx x, int co
 	    putc ('t', file);
 	  return;
 
-	case 'Z':
-	  gcc_assert (MEM_P (x));
+	case 'z':
+	  /* 387 opcodes don't get size suffixes if the operands are
+	     registers.  */
+	  if (STACK_REG_P (x))
+	    return;
 
-	  /* fild/fist don't get size suffixes if using Intel opcodes.  */
+	  /* 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)))
 	    {
+	    case 1:
+	      /* Backward compability for asm statement.  */
+	      putc ('b', file);
+	      return;
+
 	    case 2:
-#ifdef HAVE_AS_IX86_FILDS
-	      putc ('s', file);
+	      /* Backward compability for asm statement.  */
+	      if (MEM_P (x))
+		{
+#ifdef HAVE_GAS_FILDS_FISTS
+		  putc ('s', file);
 #endif
+		  return;
+		}
+	      else
+		putc ('w', file);
 	      return;
 
 	    case 4:
-	      putc ('l', file);
+	      if (GET_MODE (x) == SFmode)
+		putc ('s', file);
+	      else
+		putc ('l', file);
+	      return;
+
+	    case 12:
+	    case 16:
+	      putc ('t', file);
 	      return;
 
 	    case 8:
-#ifdef HAVE_AS_IX86_FILDQ
-	      putc ('q', file);
-#else
-	      fputs ("ll", file);
-#endif
+	      /* Backward compability for asm statement.  */
+	      if (GET_MODE_CLASS (GET_MODE (x)) == MODE_INT)
+		{
+		  if (MEM_P (x))
+		    fputs ("ll", file);
+		  else
+		    putc ('q', file);
+		}
+	      else
+		putc ('l', file);
 	      return;
 
 	    default:
 	      gcc_unreachable ();
 	    }
 	    
-	case 'z':
-	  /* 387 opcodes don't get size suffixes if the operands are
-	     registers.  */
-	  if (STACK_REG_P (x))
-	    return;
+	case 'Z':
+	  /* 'Z' is for integer insntructions only.  */
+	  gcc_assert (!STACK_REG_P (x)
+		      && GET_MODE_CLASS (GET_MODE (x)) == MODE_INT);
 
-	  /* Likewise if using Intel opcodes.  */
+	  /* Intel syntax don't need size suffixes.  */
 	  if (ASSEMBLER_DIALECT == ASM_INTEL)
 	    return;
 
@@ -10996,36 +11025,15 @@ print_operand (FILE *file, rtx x, int co
 	      return;
 
 	    case 2:
-	      /* ??? This fails for HImode integer
-		 operator with memory operand.  */
-	      if (MEM_P (x))
-		{
-#ifdef HAVE_AS_IX86_FILDS
-		  putc ('s', file);
-#endif
-		  return;
-		}
-	      else
-		putc ('w', file);
+	      putc ('w', file);
 	      return;
 
 	    case 4:
-	      if (GET_MODE_CLASS (GET_MODE (x)) == MODE_INT)
-		putc ('l', file);
-	      else
-		putc ('s', file);
+	      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);
+	      putc ('q', file);
 	      return;
 
 	    default:
@@ -12108,15 +12116,15 @@ output_fix_trunc (rtx insn, rtx *operand
   gcc_assert (GET_MODE (operands[1]) != TFmode);
 
   if (fisttp)
-      output_asm_insn ("fisttp%Z0\t%0", operands);
+      output_asm_insn ("fisttp%z0\t%0", operands);
   else
     {
       if (round_mode != I387_CW_ANY)
 	output_asm_insn ("fldcw\t%3", operands);
       if (stack_top_dies || dimode_p)
-	output_asm_insn ("fistp%Z0\t%0", operands);
+	output_asm_insn ("fistp%z0\t%0", operands);
       else
-	output_asm_insn ("fist%Z0\t%0", operands);
+	output_asm_insn ("fist%z0\t%0", operands);
       if (round_mode != I387_CW_ANY)
 	output_asm_insn ("fldcw\t%2", operands);
     }
@@ -26989,7 +26997,7 @@ x86_output_mi_thunk (FILE *file ATTRIBUT
       /* Put the this parameter into %eax.  */
       xops[0] = this_param;
       xops[1] = this_reg = gen_rtx_REG (Pmode, AX_REG);
-      output_asm_insn ("mov%z1\t{%0, %1|%1, %0}", xops);
+      output_asm_insn ("mov%Z1\t{%0, %1|%1, %0}", xops);
     }
   else
     this_reg = NULL_RTX;
@@ -27031,7 +27039,7 @@ x86_output_mi_thunk (FILE *file ATTRIBUT
 
       xops[0] = gen_rtx_MEM (Pmode, this_reg);
       xops[1] = tmp;
-      output_asm_insn ("mov%z1\t{%0, %1|%1, %0}", xops);
+      output_asm_insn ("mov%Z1\t{%0, %1|%1, %0}", xops);
 
       /* Adjust the this parameter.  */
       xops[0] = gen_rtx_MEM (Pmode, plus_constant (tmp, vcall_offset));
@@ -27044,7 +27052,7 @@ x86_output_mi_thunk (FILE *file ATTRIBUT
 	  xops[0] = gen_rtx_MEM (Pmode, gen_rtx_PLUS (Pmode, tmp, tmp2));
 	}
       xops[1] = this_reg;
-      output_asm_insn ("add%z1\t{%0, %1|%1, %0}", xops);
+      output_asm_insn ("add%Z1\t{%0, %1|%1, %0}", xops);
     }
 
   /* If necessary, drop THIS back to its stack slot.  */
@@ -27052,7 +27060,7 @@ x86_output_mi_thunk (FILE *file ATTRIBUT
     {
       xops[0] = this_reg;
       xops[1] = this_param;
-      output_asm_insn ("mov%z1\t{%0, %1|%1, %0}", xops);
+      output_asm_insn ("mov%Z1\t{%0, %1|%1, %0}", xops);
     }
 
   xops[0] = XEXP (DECL_RTL (function), 0);

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit  integer insn
  2009-04-26 20:47 PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit integer insn H.J. Lu
@ 2009-04-27 12:16 ` Uros Bizjak
  2009-04-27 13:58   ` H.J. Lu
  2009-04-28 16:46   ` H.J. Lu
  0 siblings, 2 replies; 19+ messages in thread
From: Uros Bizjak @ 2009-04-27 12:16 UTC (permalink / raw)
  To: gcc-patches; +Cc: H.J. Lu, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 2815 bytes --]

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.


[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 8315 bytes --]

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 12:16 ` Uros Bizjak
@ 2009-04-27 13:58   ` H.J. Lu
  2009-04-27 14:11     ` Paolo Bonzini
  2009-04-28 16:46   ` H.J. Lu
  1 sibling, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2009-04-27 13:58 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches, Jakub Jelinek

On Mon, Apr 27, 2009 at 5:02 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> 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.

I disagree. There are many applications compiled by gcc, whose
source codes aren't disclosed.  We have to provide 100% compatibility
for %z on x87 instructions.

> - there are usages of %z with integer insns [1] and a couple of bugreports
> of %z with integer ops.

%z never fully worked on integer instructions. There are no
compatibility issues with %z on integer instructions. You can't
use %z on integer  instructions with gcc 4.1, 4.2, 4.3, 4.4, ... anyway.

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

The %z on x87 insns is widely used in gcc sources. It is one kind of
documentation.

> 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.
>

That is wrong to knowingly break existing working programs without
justification.  I will submit a testcase to check %z on x87 instructions.

-- 
H.J.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit    integer insn
  2009-04-27 13:58   ` H.J. Lu
@ 2009-04-27 14:11     ` Paolo Bonzini
  2009-04-27 14:17       ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2009-04-27 14:11 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Uros Bizjak, gcc-patches, Jakub Jelinek


>> Due to this, I don't think that we need to provide backward compatibility
>> with _undocumented_ %z for x87 insns.
> 
> The %z on x87 insns is widely used in gcc sources. It is one kind of
> documentation.

No, it is not.  Undocumented operand modifiers are, well, undocumented,
and if we decide to use a letter for something else there's no reason
not to do so.

In fact, operand modifiers are not documented at all except ia64 `%Pn'.

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 14:11     ` Paolo Bonzini
@ 2009-04-27 14:17       ` H.J. Lu
  2009-04-27 15:23         ` Paolo Bonzini
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2009-04-27 14:17 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Uros Bizjak, gcc-patches, Jakub Jelinek

On Mon, Apr 27, 2009 at 6:58 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>
>>> Due to this, I don't think that we need to provide backward compatibility
>>> with _undocumented_ %z for x87 insns.
>>
>> The %z on x87 insns is widely used in gcc sources. It is one kind of
>> documentation.
>
> No, it is not.  Undocumented operand modifiers are, well, undocumented,
> and if we decide to use a letter for something else there's no reason
> not to do so.
>

Are you suggesting we are free to change asm statement behaviors
as long as they aren't mentioned in gcc.info? What is difference
between a feature which we failed to document properly and a
feature purely internal to gcc?

> In fact, operand modifiers are not documented at all except ia64 `%Pn'.

I saw

  `m'
          Memory operand.  Remember that `m' allows postincrement and
          postdecrement which require printing with `%Pn' on IA-64.
          Use `S' to disallow postincrement and postdecrement.

Can you tell how to use it from above? I have to find an example for it.
Guess where I can find such examples?

I don't think you are telling me that it is OK to change those modifiers
used by asm statement, except for ia64 `%Pn'.

-- 
H.J.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit  integer insn
  2009-04-27 14:17       ` H.J. Lu
@ 2009-04-27 15:23         ` Paolo Bonzini
  2009-04-27 16:24           ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: Paolo Bonzini @ 2009-04-27 15:23 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Uros Bizjak, gcc-patches, Jakub Jelinek


> Are you suggesting we are free to change asm statement behaviors
> as long as they aren't mentioned in gcc.info? What is difference
> between a feature which we failed to document properly and a
> feature purely internal to gcc?

In any case we have to use our judgement about what code is going to
break when we change undocumented details.  In some case, we may like to
be more free about how to make such a feature can be most useful to the
users in preparation to it being documented.  This way, _when and if_ we
document it, it will behave as it has existed for a while.

In this particular case, there are two possible definitions.  For one of
it, we do not know about uses in the wild, but of course there may be
proprietary uses.  The other is what is documented in the GCC sources,
despite they actually implement the first, and we have bug reports
suggesting that users had taken the wrong suggestion for the GCC sources.

>> In fact, operand modifiers are not documented at all except ia64 `%Pn'.
> 
> I saw
> 
>   `m'
>           Memory operand.  Remember that `m' allows postincrement and
>           postdecrement which require printing with `%Pn' on IA-64.
>           Use `S' to disallow postincrement and postdecrement.
> 
> Can you tell how to use it from above? I have to find an example for it.
> Guess where I can find such examples?

I see your point, but if one looks for %z in config/i386/i386.md he gets

;; 'z' mov%z1 would be movl, movw, or movb depending on the mode of
;;     operands[1].

which is exactly the definition that Uros is pushing.

> I don't think you are telling me that it is OK to change those modifiers
> used by asm statement, except for ia64 `%Pn'.

I do think that operand modifiers are a feature that, right now, can be
considered "almost" internal to gcc.

It does not have to stay this way forever, of course.  To fully solve
the issue, we should document operand modifiers at least in a restricted
form (for example i386 %b could be documented too, it is useful with
setCC) and add the change to the GCC 4.5 changes page.

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 15:23         ` Paolo Bonzini
@ 2009-04-27 16:24           ` H.J. Lu
  2009-04-27 16:36             ` Paolo Bonzini
  2009-04-27 19:04             ` Uros Bizjak
  0 siblings, 2 replies; 19+ messages in thread
From: H.J. Lu @ 2009-04-27 16:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Uros Bizjak, gcc-patches, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 4269 bytes --]

On Mon, Apr 27, 2009 at 8:18 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>
>> Are you suggesting we are free to change asm statement behaviors
>> as long as they aren't mentioned in gcc.info? What is difference
>> between a feature which we failed to document properly and a
>> feature purely internal to gcc?
>
> In any case we have to use our judgement about what code is going to
> break when we change undocumented details.  In some case, we may like to
> be more free about how to make such a feature can be most useful to the
> users in preparation to it being documented.  This way, _when and if_ we
> document it, it will behave as it has existed for a while.
>
> In this particular case, there are two possible definitions.  For one of
> it, we do not know about uses in the wild, but of course there may be
> proprietary uses.  The other is what is documented in the GCC sources,
> despite they actually implement the first, and we have bug reports
> suggesting that users had taken the wrong suggestion for the GCC sources.

>>> In fact, operand modifiers are not documented at all except ia64 `%Pn'.
>>
>> I saw
>>
>>   `m'
>>           Memory operand.  Remember that `m' allows postincrement and
>>           postdecrement which require printing with `%Pn' on IA-64.
>>           Use `S' to disallow postincrement and postdecrement.
>>
>> Can you tell how to use it from above? I have to find an example for it.
>> Guess where I can find such examples?
>
> I see your point, but if one looks for %z in config/i386/i386.md he gets
>
> ;; 'z' mov%z1 would be movl, movw, or movb depending on the mode of
> ;;     operands[1].
>
> which is exactly the definition that Uros is pushing.

There are several problems:

1. It is still the part of gcc source. It isn't the user documentation.
Given some out-of-date comments and actual usages in gcc source,
I will take actual usages over some out-of-date comments.
2. %z is fully implemented and used for x87 insns.
3. "movq" isn't mentioned.
4. "movw/movq" never worked on memory operand.

What do breaking %z on x87 insns and fixing %z on integer
insns in gcc 4.5 give to gcc users? What do you tell gcc
users who are using %z on x87 insns and %z on integer
insns?

>> I don't think you are telling me that it is OK to change those modifiers
>> used by asm statement, except for ia64 `%Pn'.
>
> I do think that operand modifiers are a feature that, right now, can be
> considered "almost" internal to gcc.

What do you tell to those gcc users who have been using %z
on x87 insns since as back as gcc 2.95.2.1?

> It does not have to stay this way forever, of course.  To fully solve
> the issue, we should document operand modifiers at least in a restricted
> form (for example i386 %b could be documented too, it is useful with
> setCC) and add the change to the GCC 4.5 changes page.
>

Here is the updated patch to make sure that %z works on x87 insns.
There is also a regression where we no longer check in-tree binutils
version for filds. There is no need to test fildq.


-- 
H.J.
---
gcc/

2009-04-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/39911
	* configure.ac (HAVE_AS_IX86_FILDS): Rename to ...
	(HAVE_AS_IX86_FILDS_FISTS): This.  Restore in-tree binutils.
	(HAVE_AS_IX86_FILDQ): Removed.
	* configure: Regenerated.
	* config.in: Likewise.

	* config/i386/i386.c (ix86_file_end): Replace "%z0" with "%Z0".
	(output_set_got): Likewise.
	(output_set_got): Likewise.
	(x86_output_mi_thunk): Likewise.
	(print_operand): Restore 'z' handling.  Make 'Z' for integer
	insntructions only.
	(output_fix_trunc): Use '%z' to output suffix of fist{,p,tp} insn.

	* config/i386/i386.md (*floathi<mode>2_i387): Use '%z' to output
	suffix of fild insn.
	(*floatsi<mode>2_vector_mixed): Ditto.
	(*float<SSEMODEI24:mode><MODEF:mode>2_mixed_interunit): Ditto.
	(*float<SSEMODEI24:mode><MODEF:mode>2_mixed_nointerunit): Ditto.
	(*float<SSEMODEI24:mode><X87MODEF:mode>2_i387_with_temp): Ditto.
	(*float<SSEMODEI24:mode><X87MODEF:mode>2_i387): Ditto.

gcc/testsuite/

2009-04-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/39911
	* gcc.target/i386/pr39911-1.c: New.
	* gcc.target/i386/pr39911-2.c: Likewise.

[-- Attachment #2: gcc-pr39911-3.patch --]
[-- Type: text/plain, Size: 18323 bytes --]

gcc/

2009-04-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/39911
	* configure.ac (HAVE_AS_IX86_FILDS): Rename to ...
	(HAVE_AS_IX86_FILDS_FISTS): This.  Restore in-tree binutils.
	(HAVE_AS_IX86_FILDQ): Removed.
	* configure: Regenerated.
	* config.in: Likewise.

	* config/i386/i386.c (ix86_file_end): Replace "%z0" with "%Z0".
	(output_set_got): Likewise.
	(output_set_got): Likewise.
	(x86_output_mi_thunk): Likewise.
	(print_operand): Restore 'z' handling.  Make 'Z' for integer
	insntructions only.
	(output_fix_trunc): Use '%z' to output suffix of fist{,p,tp} insn.

	* config/i386/i386.md (*floathi<mode>2_i387): Use '%z' to output
	suffix of fild insn.
	(*floatsi<mode>2_vector_mixed): Ditto.
	(*float<SSEMODEI24:mode><MODEF:mode>2_mixed_interunit): Ditto.
	(*float<SSEMODEI24:mode><MODEF:mode>2_mixed_nointerunit): Ditto.
	(*float<SSEMODEI24:mode><X87MODEF:mode>2_i387_with_temp): Ditto.
	(*float<SSEMODEI24:mode><X87MODEF:mode>2_i387): Ditto.

gcc/testsuite/

2009-04-26  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/39911
	* gcc.target/i386/pr39911-1.c: New.
	* gcc.target/i386/pr39911-2.c: Likewise.

Index: gcc/configure
===================================================================
--- gcc/configure	(revision 146841)
+++ gcc/configure	(working copy)
@@ -22638,45 +22638,16 @@ fi
 
     echo "$as_me:$LINENO: checking assembler for filds and fists mnemonics" >&5
 echo $ECHO_N "checking assembler for filds and fists mnemonics... $ECHO_C" >&6
-if test "${gcc_cv_as_ix86_filds+set}" = set; then
+if test "${gcc_cv_as_ix86_filds_fists+set}" = set; then
   echo $ECHO_N "(cached) $ECHO_C" >&6
 else
-  gcc_cv_as_ix86_filds=no
-  if test x$gcc_cv_as != x; then
-    echo 'filds mem; fists mem' > conftest.s
-    if { ac_try='$gcc_cv_as  -o conftest.o conftest.s >&5'
-  { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5
-  (eval $ac_try) 2>&5
-  ac_status=$?
-  echo "$as_me:$LINENO: \$? = $ac_status" >&5
-  (exit $ac_status); }; }
-    then
-	gcc_cv_as_ix86_filds=yes
-    else
-      echo "configure: failed program was" >&5
-      cat conftest.s >&5
-    fi
-    rm -f conftest.o conftest.s
-  fi
-fi
-echo "$as_me:$LINENO: result: $gcc_cv_as_ix86_filds" >&5
-echo "${ECHO_T}$gcc_cv_as_ix86_filds" >&6
-if test $gcc_cv_as_ix86_filds = yes; then
-
-cat >>confdefs.h <<\_ACEOF
-#define HAVE_AS_IX86_FILDS 1
-_ACEOF
-
+  gcc_cv_as_ix86_filds_fists=no
+    if test $in_tree_gas = yes; then
+    if test $gcc_cv_gas_vers -ge `expr \( \( 2 \* 1000 \) + 9 \) \* 1000 + 0`
+  then gcc_cv_as_ix86_filds_fists=yes
 fi
-
-    echo "$as_me:$LINENO: checking assembler for fildq and fistpq mnemonics" >&5
-echo $ECHO_N "checking assembler for fildq and fistpq mnemonics... $ECHO_C" >&6
-if test "${gcc_cv_as_ix86_fildq+set}" = set; then
-  echo $ECHO_N "(cached) $ECHO_C" >&6
-else
-  gcc_cv_as_ix86_fildq=no
-  if test x$gcc_cv_as != x; then
-    echo 'fildq mem; fistpq mem' > conftest.s
+  elif test x$gcc_cv_as != x; then
+    echo 'filds mem; fists mem' > conftest.s
     if { ac_try='$gcc_cv_as  -o conftest.o conftest.s >&5'
   { (eval echo "$as_me:$LINENO: \"$ac_try\"") >&5
   (eval $ac_try) 2>&5
@@ -22684,7 +22655,7 @@ else
   echo "$as_me:$LINENO: \$? = $ac_status" >&5
   (exit $ac_status); }; }
     then
-	gcc_cv_as_ix86_fildq=yes
+	gcc_cv_as_ix86_filds_fists=yes
     else
       echo "configure: failed program was" >&5
       cat conftest.s >&5
@@ -22692,12 +22663,12 @@ else
     rm -f conftest.o conftest.s
   fi
 fi
-echo "$as_me:$LINENO: result: $gcc_cv_as_ix86_fildq" >&5
-echo "${ECHO_T}$gcc_cv_as_ix86_fildq" >&6
-if test $gcc_cv_as_ix86_fildq = yes; then
+echo "$as_me:$LINENO: result: $gcc_cv_as_ix86_filds_fists" >&5
+echo "${ECHO_T}$gcc_cv_as_ix86_filds_fists" >&6
+if test $gcc_cv_as_ix86_filds_fists = yes; then
 
 cat >>confdefs.h <<\_ACEOF
-#define HAVE_AS_IX86_FILDQ 1
+#define HAVE_AS_IX86_FILDS_FISTS 1
 _ACEOF
 
 fi
Index: gcc/testsuite/gcc.target/i386/pr39911-1.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr39911-1.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/pr39911-1.c	(revision 0)
@@ -0,0 +1,63 @@
+/* { dg-do assemble } */
+/* { dg-options "-O2" } */
+
+void 
+bar1 () 
+{
+  char 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" : "=q"(foo) : "ir"(23));
+  asm volatile ("add%Z0 %1, %0": "+q" (foo): "ir" (23));
+  asm volatile ("mov%z0 %1, %0" : "=q"(foo) : "ir"(23));
+  asm volatile ("add%z0 %1, %0": "+q" (foo): "ir" (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 ("mov%z0 %1, %0" : "=r"(foo) : "ir"(23));
+  asm volatile ("add%z0 %1, %0": "+r" (foo): "ir" (23));
+}
+
+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));
+  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));
+      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): "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 ("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));
+      asm volatile ("pop%z0 %0": : "r" (foo));
+    }
+}

Property changes on: gcc/testsuite/gcc.target/i386/pr39911-1.c
___________________________________________________________________
Added: svn:mergeinfo

Index: gcc/testsuite/gcc.target/i386/pr39911-2.c
===================================================================
--- gcc/testsuite/gcc.target/i386/pr39911-2.c	(revision 0)
+++ gcc/testsuite/gcc.target/i386/pr39911-2.c	(revision 0)
@@ -0,0 +1,44 @@
+/* { dg-do assemble } */
+/* { dg-options "-O2" } */
+
+void 
+bar1 () 
+{
+  short foo;
+  asm volatile ("fild%z0 %0": : "m" (foo));
+}
+
+void
+bar2 () 
+{
+  int foo;
+  asm volatile ("fild%z0 %0": : "m" (foo));
+}
+
+void
+bar3 () 
+{
+  long long foo;
+  asm volatile ("fild%z0 %0": : "m" (foo));
+}
+
+void 
+bar4 () 
+{
+  float foo;
+  asm volatile ("fld%z0 %0": : "m" (foo));
+}
+
+void 
+bar5 () 
+{
+  double foo;
+  asm volatile ("fld%z0 %0": : "m" (foo));
+}
+
+void 
+bar6 () 
+{
+  long double foo;
+  asm volatile ("fld%z0 %0": : "m" (foo));
+}
Index: gcc/config.in
===================================================================
--- gcc/config.in	(revision 146841)
+++ gcc/config.in	(working copy)
@@ -273,15 +273,9 @@
 #endif
 
 
-/* Define if your assembler uses fildq and fistq mnemonics. */
+/* Define if your assembler uses the new HImode fild and fist notation. */
 #ifndef USED_FOR_TARGET
-#undef HAVE_AS_IX86_FILDQ
-#endif
-
-
-/* Define if your assembler uses filds and fists mnemonics. */
-#ifndef USED_FOR_TARGET
-#undef HAVE_AS_IX86_FILDS
+#undef HAVE_AS_IX86_FILDS_FISTS
 #endif
 
 
Index: gcc/configure.ac
===================================================================
--- gcc/configure.ac	(revision 146841)
+++ gcc/configure.ac	(working copy)
@@ -2931,16 +2931,10 @@ foo:	nop
     esac
 
     gcc_GAS_CHECK_FEATURE([filds and fists mnemonics],
-       gcc_cv_as_ix86_filds,,,
-       [filds mem; fists mem],,
-       [AC_DEFINE(HAVE_AS_IX86_FILDS, 1,
-         [Define if your assembler uses filds and fists mnemonics.])])
-
-    gcc_GAS_CHECK_FEATURE([fildq and fistpq mnemonics],
-       gcc_cv_as_ix86_fildq,,,
-       [fildq mem; fistpq mem],,
-       [AC_DEFINE(HAVE_AS_IX86_FILDQ, 1,
-         [Define if your assembler uses fildq and fistq mnemonics.])])
+       gcc_cv_as_ix86_filds_fists,
+      [2,9,0],, [filds mem; fists mem],,
+      [AC_DEFINE(HAVE_AS_IX86_FILDS_FISTS, 1,
+        [Define if your assembler uses the new HImode fild and fist notation.])])
 
     gcc_GAS_CHECK_FEATURE([cmov syntax],
       gcc_cv_as_ix86_cmov_sun_syntax,,,
Index: gcc/config/i386/i386.md
===================================================================
--- gcc/config/i386/i386.md	(revision 146841)
+++ gcc/config/i386/i386.md	(working copy)
@@ -35,8 +35,9 @@
 ;; O -- if HAVE_AS_IX86_CMOV_SUN_SYNTAX, expand to "w.", "l." or "q.",
 ;;      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 -- print the integer instruction suffix for the size of the current
+;;	operand
+;; 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.
@@ -5141,7 +5142,7 @@
   "TARGET_80387
    && (!(SSE_FLOAT_MODE_P (<MODE>mode) && TARGET_SSE_MATH)
        || TARGET_MIX_SSE_I387)"
-  "fild%Z1\t%1"
+  "fild%z1\t%1"
   [(set_attr "type" "fmov")
    (set_attr "mode" "<MODE>")
    (set_attr "fp_int_src" "true")])
@@ -5251,7 +5252,7 @@
   "TARGET_SSE2 && TARGET_MIX_SSE_I387
    && TARGET_USE_VECTOR_CONVERTS && optimize_function_for_speed_p (cfun)"
   "@
-   fild%Z1\t%1
+   fild%z1\t%1
    #"
   [(set_attr "type" "fmov,sseicvt")
    (set_attr "mode" "<MODE>,<ssevecmode>")
@@ -5312,7 +5313,7 @@
    && SSE_FLOAT_MODE_P (<MODEF:MODE>mode) && TARGET_MIX_SSE_I387
    && (TARGET_INTER_UNIT_CONVERSIONS || optimize_function_for_size_p (cfun))"
   "@
-   fild%Z1\t%1
+   fild%z1\t%1
    %vcvtsi2s<MODEF:ssemodefsuffix><SSEMODEI24:rex64suffix>\t{%1, %d0|%d0, %1}
    %vcvtsi2s<MODEF:ssemodefsuffix><SSEMODEI24:rex64suffix>\t{%1, %d0|%d0, %1}"
   [(set_attr "type" "fmov,sseicvt,sseicvt")
@@ -5331,7 +5332,7 @@
    && SSE_FLOAT_MODE_P (<MODEF:MODE>mode) && TARGET_MIX_SSE_I387
    && !(TARGET_INTER_UNIT_CONVERSIONS || optimize_function_for_size_p (cfun))"
   "@
-   fild%Z1\t%1
+   fild%z1\t%1
    %vcvtsi2s<MODEF:ssemodefsuffix><SSEMODEI24:rex64suffix>\t{%1, %d0|%d0, %1}"
   [(set_attr "type" "fmov,sseicvt")
    (set_attr "prefix" "orig,maybe_vex")
@@ -5581,7 +5582,7 @@
   "TARGET_80387
    && X87_ENABLE_FLOAT (<X87MODEF:MODE>mode, <SSEMODEI24:MODE>mode)"
   "@
-   fild%Z1\t%1
+   fild%z1\t%1
    #"
   [(set_attr "type" "fmov,multi")
    (set_attr "mode" "<X87MODEF:MODE>")
@@ -5594,7 +5595,7 @@
 	  (match_operand:SSEMODEI24 1 "memory_operand" "m")))]
   "TARGET_80387
    && X87_ENABLE_FLOAT (<X87MODEF:MODE>mode, <SSEMODEI24:MODE>mode)"
-  "fild%Z1\t%1"
+  "fild%z1\t%1"
   [(set_attr "type" "fmov")
    (set_attr "mode" "<X87MODEF:MODE>")
    (set_attr "fp_int_src" "true")])
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	(revision 146841)
+++ gcc/config/i386/i386.c	(working copy)
@@ -7491,7 +7491,7 @@ ix86_file_end (void)
 
       xops[0] = gen_rtx_REG (Pmode, regno);
       xops[1] = gen_rtx_MEM (Pmode, stack_pointer_rtx);
-      output_asm_insn ("mov%z0\t{%1, %0|%0, %1}", xops);
+      output_asm_insn ("mov%Z0\t{%1, %0|%0, %1}", xops);
       output_asm_insn ("ret", xops);
     }
 
@@ -7531,7 +7531,7 @@ output_set_got (rtx dest, rtx label ATTR
       xops[2] = gen_rtx_LABEL_REF (Pmode, label ? label : gen_label_rtx ());
 
       if (!flag_pic)
-	output_asm_insn ("mov%z0\t{%2, %0|%0, %2}", xops);
+	output_asm_insn ("mov%Z0\t{%2, %0|%0, %2}", xops);
       else
 	output_asm_insn ("call\t%a2", xops);
 
@@ -7546,7 +7546,7 @@ output_set_got (rtx dest, rtx label ATTR
 				 CODE_LABEL_NUMBER (XEXP (xops[2], 0)));
 
       if (flag_pic)
-	output_asm_insn ("pop%z0\t%0", xops);
+	output_asm_insn ("pop%Z0\t%0", xops);
     }
   else
     {
@@ -7572,9 +7572,9 @@ output_set_got (rtx dest, rtx label ATTR
     return "";
 
   if (!flag_pic || TARGET_DEEP_BRANCH_PREDICTION)
-    output_asm_insn ("add%z0\t{%1, %0|%0, %1}", xops);
+    output_asm_insn ("add%Z0\t{%1, %0|%0, %1}", xops);
   else
-    output_asm_insn ("add%z0\t{%1+[.-%a2], %0|%0, %1+(.-%a2)}", xops);
+    output_asm_insn ("add%Z0\t{%1+[.-%a2], %0|%0, %1+(.-%a2)}", xops);
 
   return "";
 }
@@ -10847,8 +10847,9 @@ get_some_local_dynamic_name (void)
    O -- if HAVE_AS_IX86_CMOV_SUN_SYNTAX, expand to "w.", "l." or "q.",
         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 -- print the integer instruction suffix for the size of the current
+	operand
+   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,44 +10948,72 @@ print_operand (FILE *file, rtx x, int co
 	    putc ('t', file);
 	  return;
 
-	case 'Z':
-	  gcc_assert (MEM_P (x));
+	case 'z':
+	  /* 387 opcodes don't get size suffixes if the operands are
+	     registers.  */
+	  if (STACK_REG_P (x))
+	    return;
 
-	  /* fild/fist don't get size suffixes if using Intel opcodes.  */
+	  /* 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)))
 	    {
+	    case 1:
+	      /* Backward compability for asm statement.  */
+	      putc ('b', file);
+	      return;
+
 	    case 2:
-#ifdef HAVE_AS_IX86_FILDS
-	      putc ('s', file);
+	      /* Backward compability for asm statement.  */
+	      if (MEM_P (x))
+		{
+#ifdef HAVE_AS_IX86_FILDS_FISTS
+		  putc ('s', file);
 #endif
+		  return;
+		}
+	      else
+		putc ('w', file);
 	      return;
 
 	    case 4:
-	      putc ('l', file);
+	      if (GET_MODE (x) == SFmode)
+		putc ('s', file);
+	      else
+		putc ('l', file);
+	      return;
+
+	    case 12:
+	    case 16:
+	      putc ('t', file);
 	      return;
 
 	    case 8:
-#ifdef HAVE_AS_IX86_FILDQ
-	      putc ('q', file);
-#else
-	      fputs ("ll", file);
-#endif
+	      /* Backward compability for asm statement.  */
+	      if (GET_MODE_CLASS (GET_MODE (x)) == MODE_INT)
+		{
+		  if (MEM_P (x))
+		    fputs ("ll", file);
+		  else
+		    putc ('q', file);
+		}
+	      else
+		putc ('l', file);
 	      return;
 
 	    default:
 	      gcc_unreachable ();
 	    }
 	    
-	case 'z':
-	  /* 387 opcodes don't get size suffixes if the operands are
-	     registers.  */
-	  if (STACK_REG_P (x))
-	    return;
+	case 'Z':
+	  /* 'Z' is for integer insntructions only.  */
+	  gcc_assert (!STACK_REG_P (x)
+		      && GET_MODE_CLASS (GET_MODE (x)) == MODE_INT);
 
-	  /* Likewise if using Intel opcodes.  */
+	  /* Intel syntax don't need size suffixes.  */
 	  if (ASSEMBLER_DIALECT == ASM_INTEL)
 	    return;
 
@@ -10996,36 +11025,15 @@ print_operand (FILE *file, rtx x, int co
 	      return;
 
 	    case 2:
-	      /* ??? This fails for HImode integer
-		 operator with memory operand.  */
-	      if (MEM_P (x))
-		{
-#ifdef HAVE_AS_IX86_FILDS
-		  putc ('s', file);
-#endif
-		  return;
-		}
-	      else
-		putc ('w', file);
+	      putc ('w', file);
 	      return;
 
 	    case 4:
-	      if (GET_MODE_CLASS (GET_MODE (x)) == MODE_INT)
-		putc ('l', file);
-	      else
-		putc ('s', file);
+	      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);
+	      putc ('q', file);
 	      return;
 
 	    default:
@@ -12108,15 +12116,15 @@ output_fix_trunc (rtx insn, rtx *operand
   gcc_assert (GET_MODE (operands[1]) != TFmode);
 
   if (fisttp)
-      output_asm_insn ("fisttp%Z0\t%0", operands);
+      output_asm_insn ("fisttp%z0\t%0", operands);
   else
     {
       if (round_mode != I387_CW_ANY)
 	output_asm_insn ("fldcw\t%3", operands);
       if (stack_top_dies || dimode_p)
-	output_asm_insn ("fistp%Z0\t%0", operands);
+	output_asm_insn ("fistp%z0\t%0", operands);
       else
-	output_asm_insn ("fist%Z0\t%0", operands);
+	output_asm_insn ("fist%z0\t%0", operands);
       if (round_mode != I387_CW_ANY)
 	output_asm_insn ("fldcw\t%2", operands);
     }
@@ -26989,7 +26997,7 @@ x86_output_mi_thunk (FILE *file ATTRIBUT
       /* Put the this parameter into %eax.  */
       xops[0] = this_param;
       xops[1] = this_reg = gen_rtx_REG (Pmode, AX_REG);
-      output_asm_insn ("mov%z1\t{%0, %1|%1, %0}", xops);
+      output_asm_insn ("mov%Z1\t{%0, %1|%1, %0}", xops);
     }
   else
     this_reg = NULL_RTX;
@@ -27031,7 +27039,7 @@ x86_output_mi_thunk (FILE *file ATTRIBUT
 
       xops[0] = gen_rtx_MEM (Pmode, this_reg);
       xops[1] = tmp;
-      output_asm_insn ("mov%z1\t{%0, %1|%1, %0}", xops);
+      output_asm_insn ("mov%Z1\t{%0, %1|%1, %0}", xops);
 
       /* Adjust the this parameter.  */
       xops[0] = gen_rtx_MEM (Pmode, plus_constant (tmp, vcall_offset));
@@ -27044,7 +27052,7 @@ x86_output_mi_thunk (FILE *file ATTRIBUT
 	  xops[0] = gen_rtx_MEM (Pmode, gen_rtx_PLUS (Pmode, tmp, tmp2));
 	}
       xops[1] = this_reg;
-      output_asm_insn ("add%z1\t{%0, %1|%1, %0}", xops);
+      output_asm_insn ("add%Z1\t{%0, %1|%1, %0}", xops);
     }
 
   /* If necessary, drop THIS back to its stack slot.  */
@@ -27052,7 +27060,7 @@ x86_output_mi_thunk (FILE *file ATTRIBUT
     {
       xops[0] = this_reg;
       xops[1] = this_param;
-      output_asm_insn ("mov%z1\t{%0, %1|%1, %0}", xops);
+      output_asm_insn ("mov%Z1\t{%0, %1|%1, %0}", xops);
     }
 
   xops[0] = XEXP (DECL_RTL (function), 0);

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit    integer insn
  2009-04-27 16:24           ` H.J. Lu
@ 2009-04-27 16:36             ` Paolo Bonzini
  2009-04-27 16:37               ` Paolo Bonzini
  2009-04-27 16:49               ` H.J. Lu
  2009-04-27 19:04             ` Uros Bizjak
  1 sibling, 2 replies; 19+ messages in thread
From: Paolo Bonzini @ 2009-04-27 16:36 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Uros Bizjak, gcc-patches, Jakub Jelinek


> 1. It is still the part of gcc source. It isn't the user documentation.
> Given some out-of-date comments and actual usages in gcc source,
> I will take actual usages over some out-of-date comments.
> 2. %z is fully implemented and used for x87 insns.

I don't think most users would look at the actual %z implementation.
Those that look at the usages, likely did not know the story about
fildll and guessed that using %z for x87 insns only is a quirk of GCC's
machine description.

> 3. "movq" isn't mentioned.
> 4. "movw/movq" never worked on memory operand.
> 
> What do breaking %z on x87 insns and fixing %z on integer
> insns in gcc 4.5 give to gcc users?

Since no one is apparently using it %z on x87 insns, the advantage is
that some users will have a fixed bug rather than one closed as invalid.
Their 32-bit code which worked under GCC < 4.4, in addition, will work
without any modification under GCC 4.5 too (and for both 32-bit and 64-bit).

>>> I don't think you are telling me that it is OK to change those modifiers
>>> used by asm statement, except for ia64 `%Pn'.
>> I do think that operand modifiers are a feature that, right now, can be
>> considered "almost" internal to gcc.
> 
> What do you tell to those gcc users who have been using %z
> on x87 insns since as back as gcc 2.95.2.1?

Are there any?

I'm not going to argue anymore.  I think we all expressed our point and
the maintainers will make up their mind.

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit    integer insn
  2009-04-27 16:36             ` Paolo Bonzini
@ 2009-04-27 16:37               ` Paolo Bonzini
  2009-04-27 16:49               ` H.J. Lu
  1 sibling, 0 replies; 19+ messages in thread
From: Paolo Bonzini @ 2009-04-27 16:37 UTC (permalink / raw)
  To: gcc-patches; +Cc: Uros Bizjak, gcc-patches, Jakub Jelinek


> 1. It is still the part of gcc source. It isn't the user documentation.
> Given some out-of-date comments and actual usages in gcc source,
> I will take actual usages over some out-of-date comments.
> 2. %z is fully implemented and used for x87 insns.

I don't think most users would look at the actual %z implementation.
Those that look at the usages, likely did not know the story about
fildll and guessed that using %z for x87 insns only is a quirk of GCC's
machine description.

> 3. "movq" isn't mentioned.
> 4. "movw/movq" never worked on memory operand.
> 
> What do breaking %z on x87 insns and fixing %z on integer
> insns in gcc 4.5 give to gcc users?

Since no one is apparently using it %z on x87 insns, the advantage is
that some users will have a fixed bug rather than one closed as invalid.
Their 32-bit code which worked under GCC < 4.4, in addition, will work
without any modification under GCC 4.5 too (and for both 32-bit and 64-bit).

>>> I don't think you are telling me that it is OK to change those modifiers
>>> used by asm statement, except for ia64 `%Pn'.
>> I do think that operand modifiers are a feature that, right now, can be
>> considered "almost" internal to gcc.
> 
> What do you tell to those gcc users who have been using %z
> on x87 insns since as back as gcc 2.95.2.1?

Are there any?

I'm not going to argue anymore.  I think we all expressed our point and
the maintainers will make up their mind.

Paolo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 16:36             ` Paolo Bonzini
  2009-04-27 16:37               ` Paolo Bonzini
@ 2009-04-27 16:49               ` H.J. Lu
  1 sibling, 0 replies; 19+ messages in thread
From: H.J. Lu @ 2009-04-27 16:49 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Uros Bizjak, gcc-patches, Jakub Jelinek

On Mon, Apr 27, 2009 at 9:35 AM, Paolo Bonzini <bonzini@gnu.org> wrote:
>
>> 1. It is still the part of gcc source. It isn't the user documentation.
>> Given some out-of-date comments and actual usages in gcc source,
>> I will take actual usages over some out-of-date comments.
>> 2. %z is fully implemented and used for x87 insns.
>
> I don't think most users would look at the actual %z implementation.
> Those that look at the usages, likely did not know the story about
> fildll and guessed that using %z for x87 insns only is a quirk of GCC's
> machine description.
>
>> 3. "movq" isn't mentioned.
>> 4. "movw/movq" never worked on memory operand.
>>
>> What do breaking %z on x87 insns and fixing %z on integer
>> insns in gcc 4.5 give to gcc users?
>
> Since no one is apparently using it %z on x87 insns, the advantage is

I have no idea how to come to this conclusion. You didn't see any
usages and no one has reported any issues don't mean it isn't used.

> that some users will have a fixed bug rather than one closed as invalid.
> Their 32-bit code which worked under GCC < 4.4, in addition, will work

That is wrong again. Try

  int foo;
  asm volatile ("mov%z0 %1, %0" : "=r"(foo) : "ir"(23));
  asm volatile ("add%z0 %1, %0": "+r" (foo): "ir" (23));

and tell me what you get.

> without any modification under GCC 4.5 too (and for both 32-bit and 64-bit).

My proposal provides 100% backward compatibility. The one you
advocated does nothing for gcc 4.4/older and doesn't provide
100% backward compatibility.

>>>> I don't think you are telling me that it is OK to change those modifiers
>>>> used by asm statement, except for ia64 `%Pn'.
>>> I do think that operand modifiers are a feature that, right now, can be
>>> considered "almost" internal to gcc.
>>
>> What do you tell to those gcc users who have been using %z
>> on x87 insns since as back as gcc 2.95.2.1?
>
> Are there any?

You can count me. I wrote one in my last patch.


-- 
H.J.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 16:24           ` H.J. Lu
  2009-04-27 16:36             ` Paolo Bonzini
@ 2009-04-27 19:04             ` Uros Bizjak
  2009-04-27 19:59               ` H.J. Lu
  1 sibling, 1 reply; 19+ messages in thread
From: Uros Bizjak @ 2009-04-27 19:04 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Paolo Bonzini, gcc-patches, Jakub Jelinek

H.J. Lu wrote:

>>>> In fact, operand modifiers are not documented at all except ia64 `%Pn'.
>>>>         
>>> I saw
>>>
>>>   `m'
>>>           Memory operand.  Remember that `m' allows postincrement and
>>>           postdecrement which require printing with `%Pn' on IA-64.
>>>           Use `S' to disallow postincrement and postdecrement.
>>>
>>> Can you tell how to use it from above? I have to find an example for it.
>>> Guess where I can find such examples?
>>>       
>> I see your point, but if one looks for %z in config/i386/i386.md he gets
>>
>> ;; 'z' mov%z1 would be movl, movw, or movb depending on the mode of
>> ;;     operands[1].
>>
>> which is exactly the definition that Uros is pushing.
>>     
>
> There are several problems:
>
> 1. It is still the part of gcc source. It isn't the user documentation.
> Given some out-of-date comments and actual usages in gcc source,
> I will take actual usages over some out-of-date comments.
> 2. %z is fully implemented and used for x87 insns.
> 3. "movq" isn't mentioned.
> 4. "movw/movq" never worked on memory operand.
>   

My reasoning for fixing "%z" are:
- users found %z in the comment from i386.md and used it exactly in the 
way as shown there.
- %z with x87 doesn not give the suffix as mentioned in the comment above.
- there were separate bugreports for ICEs in print_operand with QImode 
operands and DImode operands. There were reports for wrong HImode suffix 
for memory operands as well as DImode memory operands. Actually, 
everything that could go wrong went wrong and these bugreports shows the 
pattern of uses for %z - also for DImode, even if "movq" is not mentioned.

Users are expecting %z to give "q,l,w,b" consistently for register and 
memory operands. Regarding x87, not even glibc took the opportunity to 
use fistp%z in mathinline.h [1].

The decision to "break" (?) existing code for x87 was done on these 
usage patterns. %z modifier was always PITA, so let's give users what 
they want and let's document the correct usage. And that is %z for all 
integer and %Z for FP insns.

[1] 
http://sourceware.org/cgi-bin/cvsweb.cgi/libc/sysdeps/i386/fpu/bits/mathinline.h?rev=1.62&content-type=text/x-cvsweb-markup&cvsroot=glibc

Uros.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 19:04             ` Uros Bizjak
@ 2009-04-27 19:59               ` H.J. Lu
  2009-04-27 20:28                 ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2009-04-27 19:59 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Paolo Bonzini, gcc-patches, Jakub Jelinek

On Mon, Apr 27, 2009 at 11:03 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> H.J. Lu wrote:
>
>>>>> In fact, operand modifiers are not documented at all except ia64 `%Pn'.
>>>>>
>>>>
>>>> I saw
>>>>
>>>>  `m'
>>>>          Memory operand.  Remember that `m' allows postincrement and
>>>>          postdecrement which require printing with `%Pn' on IA-64.
>>>>          Use `S' to disallow postincrement and postdecrement.
>>>>
>>>> Can you tell how to use it from above? I have to find an example for it.
>>>> Guess where I can find such examples?
>>>>
>>>
>>> I see your point, but if one looks for %z in config/i386/i386.md he gets
>>>
>>> ;; 'z' mov%z1 would be movl, movw, or movb depending on the mode of
>>> ;;     operands[1].
>>>
>>> which is exactly the definition that Uros is pushing.
>>>
>>
>> There are several problems:
>>
>> 1. It is still the part of gcc source. It isn't the user documentation.
>> Given some out-of-date comments and actual usages in gcc source,
>> I will take actual usages over some out-of-date comments.
>> 2. %z is fully implemented and used for x87 insns.
>> 3. "movq" isn't mentioned.
>> 4. "movw/movq" never worked on memory operand.
>>
>
> My reasoning for fixing "%z" are:
> - users found %z in the comment from i386.md and used it exactly in the way
> as shown there.

I would say actual codes override any out-of-date comments. Whoever
made the change forgot to update the comments.

> - %z with x87 doesn not give the suffix as mentioned in the comment above.

%z with x87 insns works for all operands as far as back to gcc 2.95.2.1.

> - there were separate bugreports for ICEs in print_operand with QImode
> operands and DImode operands. There were reports for wrong HImode suffix for
> memory operands as well as DImode memory operands. Actually, everything that
> could go wrong went wrong and these bugreports shows the pattern of uses for
> %z - also for DImode, even if "movq" is not mentioned.
>
> Users are expecting %z to give "q,l,w,b" consistently for register and

There are no working codes which use %z on memory operand with
integer insns.  Otherwise, we won't have this problem.

> memory operands. Regarding x87, not even glibc took the opportunity to use
> fistp%z in mathinline.h [1].

That shows %z may be useful. We really don't know there are no source
codes which use %z on x87 insns. Since %z has been working with
x87 insns for a long time, you won't see any gcc bug reports on this.

> The decision to "break" (?) existing code for x87 was done on these usage
> patterns. %z modifier was always PITA, so let's give users what they want
> and let's document the correct usage. And that is %z for all integer and %Z

I don't think the "correct" usage should be determined by some out-of-date
comments in gcc source codes. There are much more %z usages on x87
than %z on integer insns in gcc source.

> for FP insns.
>

When I am using gcc, I simply can't use %z on memory operand with
integer insns since it doesn't work with gcc ...., 4.1, 4,2, 4,3, 4.4. Breaking
%z on x87 insns in gcc 4.5 will provide very little benefit for gcc users and
may cause compatibility issues.


-- 
H.J.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 19:59               ` H.J. Lu
@ 2009-04-27 20:28                 ` H.J. Lu
  2009-04-27 20:37                   ` Uros Bizjak
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2009-04-27 20:28 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Paolo Bonzini, gcc-patches, Jakub Jelinek

On Mon, Apr 27, 2009 at 12:49 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Mon, Apr 27, 2009 at 11:03 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> H.J. Lu wrote:
>>
>>>>>> In fact, operand modifiers are not documented at all except ia64 `%Pn'.
>>>>>>
>>>>>
>>>>> I saw
>>>>>
>>>>>  `m'
>>>>>          Memory operand.  Remember that `m' allows postincrement and
>>>>>          postdecrement which require printing with `%Pn' on IA-64.
>>>>>          Use `S' to disallow postincrement and postdecrement.
>>>>>
>>>>> Can you tell how to use it from above? I have to find an example for it.
>>>>> Guess where I can find such examples?
>>>>>
>>>>
>>>> I see your point, but if one looks for %z in config/i386/i386.md he gets
>>>>
>>>> ;; 'z' mov%z1 would be movl, movw, or movb depending on the mode of
>>>> ;;     operands[1].
>>>>
>>>> which is exactly the definition that Uros is pushing.
>>>>
>>>
>>> There are several problems:
>>>
>>> 1. It is still the part of gcc source. It isn't the user documentation.
>>> Given some out-of-date comments and actual usages in gcc source,
>>> I will take actual usages over some out-of-date comments.
>>> 2. %z is fully implemented and used for x87 insns.
>>> 3. "movq" isn't mentioned.
>>> 4. "movw/movq" never worked on memory operand.
>>>
>>
>> My reasoning for fixing "%z" are:
>> - users found %z in the comment from i386.md and used it exactly in the way
>> as shown there.
>
> I would say actual codes override any out-of-date comments. Whoever
> made the change forgot to update the comments.
>
>> - %z with x87 doesn not give the suffix as mentioned in the comment above.
>
> %z with x87 insns works for all operands as far as back to gcc 2.95.2.1.
>
>> - there were separate bugreports for ICEs in print_operand with QImode
>> operands and DImode operands. There were reports for wrong HImode suffix for
>> memory operands as well as DImode memory operands. Actually, everything that
>> could go wrong went wrong and these bugreports shows the pattern of uses for
>> %z - also for DImode, even if "movq" is not mentioned.
>>
>> Users are expecting %z to give "q,l,w,b" consistently for register and
>
> There are no working codes which use %z on memory operand with
> integer insns.  Otherwise, we won't have this problem.
>
>> memory operands. Regarding x87, not even glibc took the opportunity to use
>> fistp%z in mathinline.h [1].
>
> That shows %z may be useful. We really don't know there are no source
> codes which use %z on x87 insns. Since %z has been working with
> x87 insns for a long time, you won't see any gcc bug reports on this.
>
>> The decision to "break" (?) existing code for x87 was done on these usage
>> patterns. %z modifier was always PITA, so let's give users what they want
>> and let's document the correct usage. And that is %z for all integer and %Z
>
> I don't think the "correct" usage should be determined by some out-of-date
> comments in gcc source codes. There are much more %z usages on x87
> than %z on integer insns in gcc source.
>
>> for FP insns.
>>
>
> When I am using gcc, I simply can't use %z on memory operand with
> integer insns since it doesn't work with gcc ...., 4.1, 4,2, 4,3, 4.4. Breaking
> %z on x87 insns in gcc 4.5 will provide very little benefit for gcc users and
> may cause compatibility issues.
>

I checked the history on i386.c. %z works on x87 insns as far as back
to the initial version on gcc.gnu.org in 1992 and %z never worked on
memory operand with integer insns. I don't believe we should break
%z now. We should use %Z to support integer insns.

-- 
H.J.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 20:28                 ` H.J. Lu
@ 2009-04-27 20:37                   ` Uros Bizjak
  2009-04-27 20:56                     ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: Uros Bizjak @ 2009-04-27 20:37 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Paolo Bonzini, gcc-patches, Jakub Jelinek

H.J. Lu wrote:

> I checked the history on i386.c. %z works on x87 insns as far as back
> to the initial version on gcc.gnu.org in 1992 and %z never worked on
> memory operand with integer insns. I don't believe we should break
> %z now. We should use %Z to support integer insns.
>   
The problem is, that users want "%z" due to the comment in i386.md. 
Whether gcc uses "%Z" or "%z" internally does not matter at all. I 
propose that we go ahead with my patch to see if/what breaks. It is just 
a matter of changing the letter throughout a couple of source files to 
switch the modifiers.

Uros.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 20:37                   ` Uros Bizjak
@ 2009-04-27 20:56                     ` H.J. Lu
  2009-04-27 21:12                       ` Uros Bizjak
  0 siblings, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2009-04-27 20:56 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Paolo Bonzini, gcc-patches, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

On Mon, Apr 27, 2009 at 1:26 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> H.J. Lu wrote:
>
>> I checked the history on i386.c. %z works on x87 insns as far as back
>> to the initial version on gcc.gnu.org in 1992 and %z never worked on
>> memory operand with integer insns. I don't believe we should break
>> %z now. We should use %Z to support integer insns.
>>
>
> The problem is, that users want "%z" due to the comment in i386.md. Whether
> gcc uses "%Z" or "%z" internally does not matter at all. I propose that we
> go ahead with my patch to see if/what breaks. It is just a matter of
> changing the letter throughout a couple of source files to switch the
> modifiers.
>

Since we won't change 4.3/4.4 branches, we should update comments
on 4.3/4.4 branches to reflect how 'z' is actually used in gcc. It should have
zero impact.. OK for 4.3/4.4?

Thanks.

-- 
H.J.
---
2009-04-27  H.J. Lu  <hongjiu.lu@intel.com>

	* config/i386/i386.c (print_operand): Update comments for 'z'.
	* config/i386/i386.md: Likewise.

[-- Attachment #2: gcc-z-1.patch --]
[-- Type: text/plain, Size: 1732 bytes --]

2009-04-27  H.J. Lu  <hongjiu.lu@intel.com>

	* config/i386/i386.c (print_operand): Update comments for 'z'.
	* config/i386/i386.md: Likewise.

Index: gcc/config/i386/i386.md
===================================================================
--- gcc/config/i386/i386.md	(revision 145983)
+++ gcc/config/i386/i386.md	(working copy)
@@ -27,8 +27,9 @@
 ;; See file "rtl.def" for documentation on define_insn, match_*, et. al.
 ;;
 ;; The special asm out single letter directives following a '%' are:
-;; 'z' mov%z1 would be movl, movw, or movb depending on the mode of
-;;     operands[1].
+;; 'z' print the instruction suffix for the size of the current
+;;     register/memory operand on x87 instructions and register
+;;     operand on integer instructions.
 ;; 'L' Print the opcode suffix for a 32-bit integer opcode.
 ;; 'W' Print the opcode suffix for a 16-bit integer opcode.
 ;; 'B' Print the opcode suffix for an 8-bit integer opcode.
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	(revision 145983)
+++ gcc/config/i386/i386.c	(working copy)
@@ -10725,7 +10725,9 @@ get_some_local_dynamic_name (void)
    O -- if HAVE_AS_IX86_CMOV_SUN_SYNTAX, expand to "w.", "l." or "q.",
         otherwise nothing
    R -- print the prefix for register names.
-   z -- print the opcode suffix for the size of the current operand.
+   z -- print the instruction suffix for the size of the current
+	register/memory operand on x87 instructions and register
+	operand on integer 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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 20:56                     ` H.J. Lu
@ 2009-04-27 21:12                       ` Uros Bizjak
  2009-04-27 21:19                         ` H.J. Lu
  0 siblings, 1 reply; 19+ messages in thread
From: Uros Bizjak @ 2009-04-27 21:12 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Paolo Bonzini, gcc-patches, Jakub Jelinek

H.J. Lu wrote:
> On Mon, Apr 27, 2009 at 1:26 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>   
>> H.J. Lu wrote:
>>
>>     
>>> I checked the history on i386.c. %z works on x87 insns as far as back
>>> to the initial version on gcc.gnu.org in 1992 and %z never worked on
>>> memory operand with integer insns. I don't believe we should break
>>> %z now. We should use %Z to support integer insns.
>>>
>>>       
>> The problem is, that users want "%z" due to the comment in i386.md. Whether
>> gcc uses "%Z" or "%z" internally does not matter at all. I propose that we
>> go ahead with my patch to see if/what breaks. It is just a matter of
>> changing the letter throughout a couple of source files to switch the
>> modifiers.
>>
>>     
>
> Since we won't change 4.3/4.4 branches, we should update comments
> on 4.3/4.4 branches to reflect how 'z' is actually used in gcc. It should have
> zero impact.. OK for 4.3/4.4?
>   

I suggest that we leave this mess on 4.3 and 4.4 branches as is. x87 
insns with register operands don't get suffixes, and suffixes for FP and 
integer memory operands of the same size are different (i.e. "s" and "l" 
for 32bit and 64bit FP ops vs. "s", "l" and "ll" for 16bit, 32bit and 
64bit integer ops). Having "b", "w", "l" and "q" suffixes for integer 
insns doesn't fit into this at all.

Uros.
> Thanks.
>
>   

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 21:12                       ` Uros Bizjak
@ 2009-04-27 21:19                         ` H.J. Lu
  0 siblings, 0 replies; 19+ messages in thread
From: H.J. Lu @ 2009-04-27 21:19 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: Paolo Bonzini, gcc-patches, Jakub Jelinek

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]

On Mon, Apr 27, 2009 at 1:55 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> H.J. Lu wrote:
>>
>> On Mon, Apr 27, 2009 at 1:26 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>
>>>
>>> H.J. Lu wrote:
>>>
>>>
>>>>
>>>> I checked the history on i386.c. %z works on x87 insns as far as back
>>>> to the initial version on gcc.gnu.org in 1992 and %z never worked on
>>>> memory operand with integer insns. I don't believe we should break
>>>> %z now. We should use %Z to support integer insns.
>>>>
>>>>
>>>
>>> The problem is, that users want "%z" due to the comment in i386.md.
>>> Whether
>>> gcc uses "%Z" or "%z" internally does not matter at all. I propose that
>>> we
>>> go ahead with my patch to see if/what breaks. It is just a matter of
>>> changing the letter throughout a couple of source files to switch the
>>> modifiers.
>>>
>>>
>>
>> Since we won't change 4.3/4.4 branches, we should update comments
>> on 4.3/4.4 branches to reflect how 'z' is actually used in gcc. It should
>> have
>> zero impact.. OK for 4.3/4.4?
>>
>
> I suggest that we leave this mess on 4.3 and 4.4 branches as is. x87 insns
> with register operands don't get suffixes, and suffixes for FP and integer
> memory operands of the same size are different (i.e. "s" and "l" for 32bit
> and 64bit FP ops vs. "s", "l" and "ll" for 16bit, 32bit and 64bit integer
> ops). Having "b", "w", "l" and "q" suffixes for integer insns doesn't fit
> into this at all.
>

Here is the updated patch for 4.3/4.4. OK for 4.3/4.4? Otherwise, people
get wrong information if they are reading gcc 4.3/4,4 sources.

Thanks.

-- 
H.J.

[-- Attachment #2: gcc-z-1.patch --]
[-- Type: text/plain, Size: 1714 bytes --]

2009-04-27  H.J. Lu  <hongjiu.lu@intel.com>

	* config/i386/i386.c (print_operand): Update comments for 'z'.
	* config/i386/i386.md: Likewise.

Index: gcc/config/i386/i386.md
===================================================================
--- gcc/config/i386/i386.md	(revision 145983)
+++ gcc/config/i386/i386.md	(working copy)
@@ -27,8 +27,9 @@
 ;; See file "rtl.def" for documentation on define_insn, match_*, et. al.
 ;;
 ;; The special asm out single letter directives following a '%' are:
-;; 'z' mov%z1 would be movl, movw, or movb depending on the mode of
-;;     operands[1].
+;; 'z' print the instruction suffix for the size of the current
+;;     memory operand on x87 instructions and register operand on
+;;     integer instructions.
 ;; 'L' Print the opcode suffix for a 32-bit integer opcode.
 ;; 'W' Print the opcode suffix for a 16-bit integer opcode.
 ;; 'B' Print the opcode suffix for an 8-bit integer opcode.
Index: gcc/config/i386/i386.c
===================================================================
--- gcc/config/i386/i386.c	(revision 145983)
+++ gcc/config/i386/i386.c	(working copy)
@@ -10725,7 +10725,9 @@ get_some_local_dynamic_name (void)
    O -- if HAVE_AS_IX86_CMOV_SUN_SYNTAX, expand to "w.", "l." or "q.",
         otherwise nothing
    R -- print the prefix for register names.
-   z -- print the opcode suffix for the size of the current operand.
+   z -- print the instruction suffix for the size of the current
+	memory operand on x87 instructions and register operand on
+	integer 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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-27 12:16 ` Uros Bizjak
  2009-04-27 13:58   ` H.J. Lu
@ 2009-04-28 16:46   ` H.J. Lu
  2009-04-28 17:25     ` Uros Bizjak
  1 sibling, 1 reply; 19+ messages in thread
From: H.J. Lu @ 2009-04-28 16:46 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches, Jakub Jelinek

On Mon, Apr 27, 2009 at 5:02 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> 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}.
>

This caused:

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

-- 
H.J.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit   integer insn
  2009-04-28 16:46   ` H.J. Lu
@ 2009-04-28 17:25     ` Uros Bizjak
  0 siblings, 0 replies; 19+ messages in thread
From: Uros Bizjak @ 2009-04-28 17:25 UTC (permalink / raw)
  To: H.J. Lu; +Cc: gcc-patches, Jakub Jelinek

H.J. Lu wrote:

>> 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}.
>>
>>     
>
> This caused:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39949
>   

This is by design.

I think I have said everything I have to say about this matter. I have 
presented the rationale for the decision, and fixed the source according 
to the decision.  If you don't agree with this decision, you can bring 
this matter to Release Managers, so someone else (i.e. not me) will take 
the responsibility for the change of %z to %Z for integer opcodes.

Uros.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2009-04-28 17:16 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-26 20:47 PATCH: PR target/39911: The 'z' suffix doesn't work with 16bit integer insn H.J. Lu
2009-04-27 12:16 ` Uros Bizjak
2009-04-27 13:58   ` H.J. Lu
2009-04-27 14:11     ` Paolo Bonzini
2009-04-27 14:17       ` H.J. Lu
2009-04-27 15:23         ` Paolo Bonzini
2009-04-27 16:24           ` H.J. Lu
2009-04-27 16:36             ` Paolo Bonzini
2009-04-27 16:37               ` Paolo Bonzini
2009-04-27 16:49               ` H.J. Lu
2009-04-27 19:04             ` Uros Bizjak
2009-04-27 19:59               ` H.J. Lu
2009-04-27 20:28                 ` H.J. Lu
2009-04-27 20:37                   ` Uros Bizjak
2009-04-27 20:56                     ` H.J. Lu
2009-04-27 21:12                       ` Uros Bizjak
2009-04-27 21:19                         ` H.J. Lu
2009-04-28 16:46   ` H.J. Lu
2009-04-28 17:25     ` Uros Bizjak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).