diff mbox

[02/10] addr32: Only handle zero-extended DImode addresses

Message ID CAFULd4byQpY50zhpjiXf_NY5mrzq-eHmyJzts8-KDQpSxzFDtA@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak March 9, 2012, 6:15 p.m. UTC 
On Fri, Mar 9, 2012 at 4:26 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Mar 8, 2012 at 7:20 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Sun, Mar 4, 2012 at 9:13 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>
>>>> We only need to handle zero-extended addresses in DImode.
>>>> OK for trunk?
>>>
>>>> 2012-03-02  H.J. Lu  <hongjiu.lu@intel.com>
>>>>
>>>>        * config/i386/i386.c (ix86_print_operand_address): Only handle
>>>>        zero-extended DImode addresses.
>>>
>>> OK.
>>
>> The patch was reverted due to PR target/52530.
>>
>
> Revert breaks Pmode == SImode for x32.  Here is a different patch.
> It checks Pmode == DImode instead of TARGET_64BIT.  Tested on
> Linux/x32.  OK for trunk?

This will still emit i.e. "leal 1(%rSImode), %rSImode" on Pmode ==
SImode targets, so you win nothing really.

Attached patch finally decouples LEA operand handling from generic
address handling, and by introducing %E operand modifier, we are able
to always emit DImode registers for LEAs (which is good anyway to
avoid unnecessary addr32 prefixes). Luckily, the "leal 1(%rSImode),
%rSImode" triggered some unknown problem with Sun assembler, so we
were able to detect the problem.

I would like to point out that the patched compiler now also emits
address registers in their natural mode (modulo zero-extended RTXes)
and fixes following failure on Pmode == SImode targets:

--cut here--
struct foo
{
 int *f;
 int i;
};

void
__attribute__ ((noinline))
bar (struct foo x)
{
 *(x.f) = 1;
}
--cut here--

For Pmode == SImode, the compiler emitted (%rdi) address, which was
wrong, since "i" was passed in the high part of (%rdi) register.

2012-03-09  Uros Bizjak  <ubizjak@gmail.com>

       PR target/52530
       * config/i386/i386.c (ix86_print_operand): Handle 'E' operand modifier.
       (ix86_print_operand_address): Handle UNSPEC_LEA_ADDR. Do not fallback
       to set code to 'q'.
       * config/i386/i386.md (UNSPEC_LEA_ADDR): New unspec.
       (*movdi_internal_rex64): Use %E operand modifier for lea.
       (*movsi_internal): Ditto.
       (*lea_1): Ditto.
       (*lea<mode>_2): Ditto.
       (*lea_{3,4,5,6}_zext): Ditto.
       (*tls_global_dynamic_32_gnu): Ditto.
       (*tls_global_dynamic_64): Ditto.
       (*tls_dynamic_gnu2_lea_32): Ditto.
       (*tls_dynamic_gnu2_lea_64): Ditto.
       (pro_epilogue_adjust_stack_<mode>_add): Ditto.

Patch was tested on x86_64-pc-linux-gnu {,-m32}. I have also eyeballed
x32 code (Pmode == SImode) and found no problems.

Committed to mainline SVN.

H.J., can you please construct a runtime test from the above example code?

Uros.

Comments

H.J. Lu March 11, 2012, 3:46 p.m. UTC | #1 
On Fri, Mar 9, 2012 at 10:15 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Fri, Mar 9, 2012 at 4:26 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, Mar 8, 2012 at 7:20 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Sun, Mar 4, 2012 at 9:13 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>>
>>>>> We only need to handle zero-extended addresses in DImode.
>>>>> OK for trunk?
>>>>
>>>>> 2012-03-02  H.J. Lu  <hongjiu.lu@intel.com>
>>>>>
>>>>>        * config/i386/i386.c (ix86_print_operand_address): Only handle
>>>>>        zero-extended DImode addresses.
>>>>
>>>> OK.
>>>
>>> The patch was reverted due to PR target/52530.
>>>
>>
>> Revert breaks Pmode == SImode for x32.  Here is a different patch.
>> It checks Pmode == DImode instead of TARGET_64BIT.  Tested on
>> Linux/x32.  OK for trunk?
>
> This will still emit i.e. "leal 1(%rSImode), %rSImode" on Pmode ==
> SImode targets, so you win nothing really.
>
> Attached patch finally decouples LEA operand handling from generic
> address handling, and by introducing %E operand modifier, we are able
> to always emit DImode registers for LEAs (which is good anyway to
> avoid unnecessary addr32 prefixes). Luckily, the "leal 1(%rSImode),
> %rSImode" triggered some unknown problem with Sun assembler, so we
> were able to detect the problem.
>
> I would like to point out that the patched compiler now also emits
> address registers in their natural mode (modulo zero-extended RTXes)
> and fixes following failure on Pmode == SImode targets:
>
> --cut here--
> struct foo
> {
>  int *f;
>  int i;
> };
>
> void
> __attribute__ ((noinline))
> bar (struct foo x)
> {
>  *(x.f) = 1;
> }
> --cut here--
>
> For Pmode == SImode, the compiler emitted (%rdi) address, which was
> wrong, since "i" was passed in the high part of (%rdi) register.
>
> 2012-03-09  Uros Bizjak  <ubizjak@gmail.com>
>
>        PR target/52530
>        * config/i386/i386.c (ix86_print_operand): Handle 'E' operand modifier.
>        (ix86_print_operand_address): Handle UNSPEC_LEA_ADDR. Do not fallback
>        to set code to 'q'.
>        * config/i386/i386.md (UNSPEC_LEA_ADDR): New unspec.
>        (*movdi_internal_rex64): Use %E operand modifier for lea.
>        (*movsi_internal): Ditto.
>        (*lea_1): Ditto.
>        (*lea<mode>_2): Ditto.
>        (*lea_{3,4,5,6}_zext): Ditto.
>        (*tls_global_dynamic_32_gnu): Ditto.
>        (*tls_global_dynamic_64): Ditto.
>        (*tls_dynamic_gnu2_lea_32): Ditto.
>        (*tls_dynamic_gnu2_lea_64): Ditto.
>        (pro_epilogue_adjust_stack_<mode>_add): Ditto.
>
> Patch was tested on x86_64-pc-linux-gnu {,-m32}. I have also eyeballed
> x32 code (Pmode == SImode) and found no problems.
>
> Committed to mainline SVN.
>
> H.J., can you please construct a runtime test from the above example code?
>
> Uros.

It passed all my x32 tests.

Thanks.
diff mbox

Patch

Index: i386.md
===================================================================
--- i386.md	(revision 185139)
+++ i386.md	(working copy)
@@ -38,6 +38,7 @@ 
 ;; Z -- likewise, with special suffixes for x87 instructions.
 ;; * -- print a star (in certain assembler syntax)
 ;; A -- print an absolute memory reference.
+;; E -- print address with DImode register names if TARGET_64BIT.
 ;; w -- print the operand as if it's a "word" (HImode) even if it isn't.
 ;; s -- print a shift double count, followed by the assemblers argument
 ;;	delimiter.
@@ -111,6 +112,7 @@ 
   UNSPEC_MS_TO_SYSV_CALL
   UNSPEC_CALL_NEEDS_VZEROUPPER
   UNSPEC_PAUSE
+  UNSPEC_LEA_ADDR
 
   ;; For SSE/MMX support:
   UNSPEC_FIX_NOTRUNC
@@ -1965,7 +1967,7 @@ 
       return "#";
 
     case TYPE_LEA:
-      return "lea{q}\t{%a1, %0|%0, %a1}";
+      return "lea{q}\t{%E1, %0|%0, %E1}";
 
     default:
       gcc_assert (!flag_pic || LEGITIMATE_PIC_OPERAND_P (operands[1]));
@@ -1974,7 +1976,7 @@ 
       else if (which_alternative == 2)
 	return "movabs{q}\t{%1, %0|%0, %1}";
       else if (ix86_use_lea_for_mov (insn, operands))
-	return "lea{q}\t{%a1, %0|%0, %a1}";
+	return "lea{q}\t{%E1, %0|%0, %E1}";
       else
 	return "mov{q}\t{%1, %0|%0, %1}";
     }
@@ -2206,12 +2208,12 @@ 
       return "movd\t{%1, %0|%0, %1}";
 
     case TYPE_LEA:
-      return "lea{l}\t{%a1, %0|%0, %a1}";
+      return "lea{l}\t{%E1, %0|%0, %E1}";
 
     default:
       gcc_assert (!flag_pic || LEGITIMATE_PIC_OPERAND_P (operands[1]));
       if (ix86_use_lea_for_mov (insn, operands))
-	return "lea{l}\t{%a1, %0|%0, %a1}";
+	return "lea{l}\t{%E1, %0|%0, %E1}";
       else
 	return "mov{l}\t{%1, %0|%0, %1}";
     }
@@ -5426,7 +5428,7 @@ 
   [(set (match_operand:SI 0 "register_operand" "=r")
 	(subreg:SI (match_operand:DI 1 "lea_address_operand" "p") 0))]
   "TARGET_64BIT"
-  "lea{l}\t{%a1, %0|%0, %a1}"
+  "lea{l}\t{%E1, %0|%0, %E1}"
   "&& reload_completed && ix86_avoid_lea_for_addr (insn, operands)"
   [(const_int 0)]
 {
@@ -5440,7 +5442,7 @@ 
   [(set (match_operand:SWI48 0 "register_operand" "=r")
 	(match_operand:SWI48 1 "lea_address_operand" "p"))]
   ""
-  "lea{<imodesuffix>}\t{%a1, %0|%0, %a1}"
+  "lea{<imodesuffix>}\t{%E1, %0|%0, %E1}"
   "reload_completed && ix86_avoid_lea_for_addr (insn, operands)"
   [(const_int 0)]
 {
@@ -5455,7 +5457,7 @@ 
 	(zero_extend:DI
 	  (subreg:SI (match_operand:DI 1 "lea_address_operand" "j") 0)))]
   "TARGET_64BIT"
-  "lea{l}\t{%a1, %k0|%k0, %a1}"
+  "lea{l}\t{%E1, %k0|%k0, %E1}"
   [(set_attr "type" "lea")
    (set_attr "mode" "SI")])
 
@@ -5464,7 +5466,7 @@ 
 	(zero_extend:DI
 	  (match_operand:SI 1 "lea_address_operand" "j")))]
   "TARGET_64BIT"
-  "lea{l}\t{%a1, %k0|%k0, %a1}"
+  "lea{l}\t{%E1, %k0|%k0, %E1}"
   [(set_attr "type" "lea")
    (set_attr "mode" "SI")])
 
@@ -5474,7 +5476,7 @@ 
 	  (subreg:DI (match_operand:SI 1 "lea_address_operand" "p") 0)
 	  (match_operand:DI 2 "const_32bit_mask" "n")))]
   "TARGET_64BIT"
-  "lea{l}\t{%a1, %k0|%k0, %a1}"
+  "lea{l}\t{%E1, %k0|%k0, %E1}"
   [(set_attr "type" "lea")
    (set_attr "mode" "SI")])
 
@@ -5484,7 +5486,7 @@ 
 	  (match_operand:DI 1 "lea_address_operand" "p")
 	  (match_operand:DI 2 "const_32bit_mask" "n")))]
   "TARGET_64BIT"
-  "lea{l}\t{%a1, %k0|%k0, %a1}"
+  "lea{l}\t{%E1, %k0|%k0, %E1}"
   [(set_attr "type" "lea")
    (set_attr "mode" "SI")])
 
@@ -12541,7 +12543,7 @@ 
   "!TARGET_64BIT && TARGET_GNU_TLS"
 {
   output_asm_insn
-    ("lea{l}\t{%a2@tlsgd(,%1,1), %0|%0, %a2@tlsgd[%1*1]}", operands);
+    ("lea{l}\t{%E2@tlsgd(,%1,1), %0|%0, %E2@tlsgd[%1*1]}", operands);
   if (TARGET_SUN_TLS)
 #ifdef HAVE_AS_IX86_TLSGDPLT
     return "call\t%a2@tlsgdplt";
@@ -12576,7 +12578,7 @@ 
   if (!TARGET_X32)
     fputs (ASM_BYTE "0x66\n", asm_out_file);
   output_asm_insn
-    ("lea{q}\t{%a1@tlsgd(%%rip), %%rdi|rdi, %a1@tlsgd[rip]}", operands);
+    ("lea{q}\t{%E1@tlsgd(%%rip), %%rdi|rdi, %E1@tlsgd[rip]}", operands);
   fputs (ASM_SHORT "0x6666\n", asm_out_file);
   fputs ("\trex64\n", asm_out_file);
   if (TARGET_SUN_TLS)
@@ -12802,7 +12804,7 @@ 
 		  (unspec:SI [(match_operand:SI 2 "tls_symbolic_operand" "")]
 			      UNSPEC_TLSDESC))))]
   "!TARGET_64BIT && TARGET_GNU2_TLS"
-  "lea{l}\t{%a2@TLSDESC(%1), %0|%0, %a2@TLSDESC[%1]}"
+  "lea{l}\t{%E2@TLSDESC(%1), %0|%0, %E2@TLSDESC[%1]}"
   [(set_attr "type" "lea")
    (set_attr "mode" "SI")
    (set_attr "length" "6")
@@ -12864,7 +12866,7 @@ 
 	(unspec:DI [(match_operand 1 "tls_symbolic_operand" "")]
 		   UNSPEC_TLSDESC))]
   "TARGET_64BIT && TARGET_GNU2_TLS"
-  "lea{q}\t{%a1@TLSDESC(%%rip), %0|%0, %a1@TLSDESC[rip]}"
+  "lea{q}\t{%E1@TLSDESC(%%rip), %0|%0, %E1@TLSDESC[rip]}"
   [(set_attr "type" "lea")
    (set_attr "mode" "DI")
    (set_attr "length" "7")
@@ -16614,7 +16616,7 @@ 
 
     default:
       operands[2] = SET_SRC (XVECEXP (PATTERN (insn), 0, 0));
-      return "lea{<imodesuffix>}\t{%a2, %0|%0, %a2}";
+      return "lea{<imodesuffix>}\t{%E2, %0|%0, %E2}";
     }
 }
   [(set (attr "type")
Index: i386.c
===================================================================
--- i386.c	(revision 185139)
+++ i386.c	(working copy)
@@ -13730,6 +13730,7 @@  get_some_local_dynamic_name (void)
    Z -- likewise, with special suffixes for x87 instructions.
    * -- print a star (in certain assembler syntax)
    A -- print an absolute memory reference.
+   E -- print address with DImode register names if TARGET_64BIT.
    w -- print the operand as if it's a "word" (HImode) even if it isn't.
    s -- print a shift double count, followed by the assemblers argument
 	delimiter.
@@ -13806,7 +13807,14 @@  ix86_print_operand (FILE *file, rtx x, int code)
 	  ix86_print_operand (file, x, 0);
 	  return;
 
+	case 'E':
+	  /* Wrap address in an UNSPEC to declare special handling.  */
+	  if (TARGET_64BIT)
+	    x = gen_rtx_UNSPEC (DImode, gen_rtvec (1, x), UNSPEC_LEA_ADDR);
 
+	  output_address (x);
+	  return;
+	    
 	case 'L':
 	  if (ASSEMBLER_DIALECT == ASM_ATT)
 	    putc ('l', file);
@@ -14418,6 +14426,7 @@  ix86_print_operand_address (FILE *file, rtx addr)
   int scale;
   int ok;
   bool vsib = false;
+  int code = 0;
 
   if (GET_CODE (addr) == UNSPEC && XINT (addr, 1) == UNSPEC_VSIBADDR)
     {
@@ -14428,6 +14437,12 @@  ix86_print_operand_address (FILE *file, rtx addr)
       addr = XVECEXP (addr, 0, 0);
       vsib = true;
     }
+  else if (GET_CODE (addr) == UNSPEC && XINT (addr, 1) == UNSPEC_LEA_ADDR)
+    {
+      gcc_assert (TARGET_64BIT);
+      ok = ix86_decompose_address (XVECEXP (addr, 0, 0), &parts);
+      code = 'q';
+    }
   else
     ok = ix86_decompose_address (addr, &parts);
 
@@ -14498,16 +14513,16 @@  ix86_print_operand_address (FILE *file, rtx addr)
     }
   else
     {
-      int code = 0;
+      /* Print SImode register names for zero-extended
+	 addresses to force addr32 prefix.  */
+      if (TARGET_64BIT
+	  && (GET_CODE (addr) == ZERO_EXTEND
+	      || GET_CODE (addr) == AND))
+	{
+	  gcc_assert (!code);
+	  code = 'l';
+	}
 
-      /* Print SImode registers for zero-extended addresses to force
-	 addr32 prefix.  Otherwise print DImode registers to avoid it.  */
-      if (TARGET_64BIT)
-	code = ((GET_CODE (addr) == ZERO_EXTEND
-		 || GET_CODE (addr) == AND)
-		? 'l'
-		: 'q');
-
       if (ASSEMBLER_DIALECT == ASM_ATT)
 	{
 	  if (disp)