diff mbox

[RFC] : Handle Pmode == SImode in stringop patterns

Message ID CAFULd4a7wsiga_MHLzaa7hqfB3NSGid_YyRQd4ShLyFBKG7B5w@mail.gmail.com
State New
Headers show

Commit Message

Uros Bizjak March 5, 2012, 9:33 p.m. UTC 
Hello!

Attached RFC patch enhances stringop patterns to emit addr32 prefix
when Pmode == SImode. I have introduced %^ operand modifier that
conditionally emits "addr32" to all stringop insn templates.

H.J., can you please test the patch if it works OK on SImode X32
target? I have tested it on x86_64-pc-linux-gnu {,-m32}, but I can't
test anything else than that it doesn't break compilation and
regression tests.

Thanks,
Uros.

Comments

Jakub Jelinek March 5, 2012, 9:42 p.m. UTC | #1 
On Mon, Mar 05, 2012 at 10:33:19PM +0100, Uros Bizjak wrote:
> +	case '^':
> +	  if (TARGET_64BIT && Pmode == SImode)
> +	    {
> +	      fputs ("addr32", file);
> +#ifndef HAVE_AS_IX86_REP_LOCK_PREFIX
> +	      if (ASSEMBLER_DIALECT == ASM_ATT)
> +		fputs ("addr32; ", file);
> +	      else
> +#endif
> +		fputs ("addr32 ", file);
> +	    }

Why do you print addr32 twice? "addr32addr32; " or "addr32addr32 ".

	Jakub
Uros Bizjak March 5, 2012, 10:04 p.m. UTC | #2 
On Mon, Mar 5, 2012 at 10:42 PM, Jakub Jelinek <jakub@redhat.com> wrote:

> On Mon, Mar 05, 2012 at 10:33:19PM +0100, Uros Bizjak wrote:
>> +     case '^':
>> +       if (TARGET_64BIT && Pmode == SImode)
>> +         {
>> +           fputs ("addr32", file);
>> +#ifndef HAVE_AS_IX86_REP_LOCK_PREFIX
>> +           if (ASSEMBLER_DIALECT == ASM_ATT)
>> +             fputs ("addr32; ", file);
>> +           else
>> +#endif
>> +             fputs ("addr32 ", file);
>> +         }
>
> Why do you print addr32 twice? "addr32addr32; " or "addr32addr32 ".

Oops, please remove the first one.

Thanks,
Uros.
H.J. Lu March 5, 2012, 10:42 p.m. UTC | #3 
On Mon, Mar 5, 2012 at 2:04 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Mar 5, 2012 at 10:42 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>
>> On Mon, Mar 05, 2012 at 10:33:19PM +0100, Uros Bizjak wrote:
>>> +     case '^':
>>> +       if (TARGET_64BIT && Pmode == SImode)
>>> +         {
>>> +           fputs ("addr32", file);
>>> +#ifndef HAVE_AS_IX86_REP_LOCK_PREFIX
>>> +           if (ASSEMBLER_DIALECT == ASM_ATT)
>>> +             fputs ("addr32; ", file);
>>> +           else
>>> +#endif
>>> +             fputs ("addr32 ", file);
>>> +         }
>>
>> Why do you print addr32 twice? "addr32addr32; " or "addr32addr32 ".
>
> Oops, please remove the first one.
>

It looks OK to me.  I will test after I fix indirect jmp/call.
Uros Bizjak March 6, 2012, 9:37 a.m. UTC | #4 
On Mon, Mar 5, 2012 at 11:42 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> +     case '^':
>>>> +       if (TARGET_64BIT && Pmode == SImode)
>>>> +         {
>>>> +           fputs ("addr32", file);
>>>> +#ifndef HAVE_AS_IX86_REP_LOCK_PREFIX
>>>> +           if (ASSEMBLER_DIALECT == ASM_ATT)
>>>> +             fputs ("addr32; ", file);
>>>> +           else
>>>> +#endif
>>>> +             fputs ("addr32 ", file);
>>>> +         }
>>>
>>> Why do you print addr32 twice? "addr32addr32; " or "addr32addr32 ".
>>
>> Oops, please remove the first one.
>>
>
> It looks OK to me.  I will test after I fix indirect jmp/call.

FYI, addr32 prefix can't stand alone (but "addr32 rep; insn" is OK),
so #ifndefed part is bogus.

Uros.
>
>
>
> --
> H.J.
H.J. Lu March 6, 2012, 9:27 p.m. UTC | #5 
On Tue, Mar 6, 2012 at 1:37 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Mar 5, 2012 at 11:42 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> +     case '^':
>>>>> +       if (TARGET_64BIT && Pmode == SImode)
>>>>> +         {
>>>>> +           fputs ("addr32", file);
>>>>> +#ifndef HAVE_AS_IX86_REP_LOCK_PREFIX
>>>>> +           if (ASSEMBLER_DIALECT == ASM_ATT)
>>>>> +             fputs ("addr32; ", file);
>>>>> +           else
>>>>> +#endif
>>>>> +             fputs ("addr32 ", file);
>>>>> +         }
>>>>
>>>> Why do you print addr32 twice? "addr32addr32; " or "addr32addr32 ".
>>>
>>> Oops, please remove the first one.
>>>
>>
>> It looks OK to me.  I will test after I fix indirect jmp/call.
>
> FYI, addr32 prefix can't stand alone (but "addr32 rep; insn" is OK),
> so #ifndefed part is bogus.
>

I changed it to

+  case '^':
+    if (TARGET_64BIT && Pmode == SImode)
+      fputs ("addr32 ", file);
+    return;

and it seems to work.

Thanks.
Uros Bizjak March 7, 2012, 3:47 p.m. UTC | #6 
On Tue, Mar 6, 2012 at 10:27 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> +     case '^':
>>>>>> +       if (TARGET_64BIT && Pmode == SImode)
>>>>>> +         {
>>>>>> +           fputs ("addr32", file);
>>>>>> +#ifndef HAVE_AS_IX86_REP_LOCK_PREFIX
>>>>>> +           if (ASSEMBLER_DIALECT == ASM_ATT)
>>>>>> +             fputs ("addr32; ", file);
>>>>>> +           else
>>>>>> +#endif
>>>>>> +             fputs ("addr32 ", file);
>>>>>> +         }
>>>>>
>>>>> Why do you print addr32 twice? "addr32addr32; " or "addr32addr32 ".
>>>>
>>>> Oops, please remove the first one.
>>>>
>>>
>>> It looks OK to me.  I will test after I fix indirect jmp/call.
>>
>> FYI, addr32 prefix can't stand alone (but "addr32 rep; insn" is OK),
>> so #ifndefed part is bogus.
>>
>
> I changed it to
>
> +  case '^':
> +    if (TARGET_64BIT && Pmode == SImode)
> +      fputs ("addr32 ", file);
> +    return;
>
> and it seems to work.

Committed with slight adjustment to above code

+	case '^':
+	  if (TARGET_64BIT && Pmode != word_mode)
+	    fputs ("addr32 ", file);
+	  return;
+

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

	* config/i386/i386.c (ix86_print_operand_punct_valid_p): Add '^'.
	(ix86_print_operand): Handle '^'.
	* config/i386/i386.md (*strmovdi_rex_1): Macroize memory operands
	using P mode iterator.  Add %^ to asm template to conditionally emit
	addr32 prefix.
	(*rep_movdi_rex64): Ditto.
	(*strsetdi_rex_1): Ditto.
	(*rep_stosdi_rex64): Ditto.
	(*strmov{si,hi,qi}_1): Add %^ to asm template to
	conditionally emit addr32 prefix.
	(*rep_mov{si,qi}): Ditto.
	(*strset{si,hi,qi}): Ditto.
	(*rep_stos{si,qi}): Ditto.
	(*cmpstrnqi_nz_1): Ditto.
	(*cmpstrnqi_1): Ditto.
	(*strlenqi_1): Ditto.

Re-tested on x86_64-pc-linux-gnu.

Uros.
diff mbox

Patch

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 184959)
+++ config/i386/i386.c	(working copy)
@@ -13742,6 +13748,7 @@  get_some_local_dynamic_name (void)
    ; -- print a semicolon (after prefixes due to bug in older gas).
    ~ -- print "i" if TARGET_AVX2, "f" otherwise.
    @ -- print a segment register of thread base pointer load
+   ^ -- print addr32 address prefix if TARGET_64BIT and Pmode == SImode
  */
 
 void
@@ -14247,6 +14254,19 @@  ix86_print_operand (FILE *file, rtx x, int code)
 	  putc (TARGET_AVX2 ? 'i' : 'f', file);
 	  return;
 
+	case '^':
+	  if (TARGET_64BIT && Pmode == SImode)
+	    {
+	      fputs ("addr32", file);
+#ifndef HAVE_AS_IX86_REP_LOCK_PREFIX
+	      if (ASSEMBLER_DIALECT == ASM_ATT)
+		fputs ("addr32; ", file);
+	      else
+#endif
+		fputs ("addr32 ", file);
+	    }
+	  return;
+
 	default:
 	    output_operand_lossage ("invalid operand code '%c'", code);
 	}
@@ -14386,8 +14406,8 @@  ix86_print_operand (FILE *file, rtx x, int code)
 static bool
 ix86_print_operand_punct_valid_p (unsigned char code)
 {
-  return (code == '@' || code == '*' || code == '+'
-	  || code == '&' || code == ';' || code == '~');
+  return (code == '@' || code == '*' || code == '+' || code == '&'
+	  || code == ';' || code == '~' || code == '^');
 }
 
 /* Print a memory operand whose address is ADDR.  */
Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md	(revision 184959)
+++ config/i386/i386.md	(working copy)
@@ -60,7 +60,9 @@ 
 ;; Y -- print condition for XOP pcom* instruction.
 ;; + -- print a branch hint as 'cs' or 'ds' prefix
 ;; ; -- print a semicolon (after prefixes due to bug in older gas).
+;; ~ -- print "i" if TARGET_AVX2, "f" otherwise.
 ;; @ -- print a segment register of thread base pointer load
+;; ^ -- print addr32 address prefix if TARGET_64BIT and Pmode == SImode
 
 (define_c_enum "unspec" [
   ;; Relocation specifiers
@@ -15678,17 +15687,17 @@ 
   "ix86_current_function_needs_cld = 1;")
 
 (define_insn "*strmovdi_rex_1"
-  [(set (mem:DI (match_operand:DI 2 "register_operand" "0"))
-	(mem:DI (match_operand:DI 3 "register_operand" "1")))
-   (set (match_operand:DI 0 "register_operand" "=D")
-	(plus:DI (match_dup 2)
-		 (const_int 8)))
-   (set (match_operand:DI 1 "register_operand" "=S")
-	(plus:DI (match_dup 3)
-		 (const_int 8)))]
+  [(set (mem:DI (match_operand:P 2 "register_operand" "0"))
+	(mem:DI (match_operand:P 3 "register_operand" "1")))
+   (set (match_operand:P 0 "register_operand" "=D")
+	(plus:P (match_dup 2)
+		(const_int 8)))
+   (set (match_operand:P 1 "register_operand" "=S")
+	(plus:P (match_dup 3)
+		(const_int 8)))]
   "TARGET_64BIT
    && !(fixed_regs[SI_REG] || fixed_regs[DI_REG])"
-  "movsq"
+  "%^movsq"
   [(set_attr "type" "str")
    (set_attr "memory" "both")
    (set_attr "mode" "DI")])
@@ -15703,7 +15712,7 @@ 
 	(plus:P (match_dup 3)
 		(const_int 4)))]
   "!(fixed_regs[SI_REG] || fixed_regs[DI_REG])"
-  "movs{l|d}"
+  "%^movs{l|d}"
   [(set_attr "type" "str")
    (set_attr "memory" "both")
    (set_attr "mode" "SI")])
@@ -15718,7 +15727,7 @@ 
 	(plus:P (match_dup 3)
 		(const_int 2)))]
   "!(fixed_regs[SI_REG] || fixed_regs[DI_REG])"
-  "movsw"
+  "%^movsw"
   [(set_attr "type" "str")
    (set_attr "memory" "both")
    (set_attr "mode" "HI")])
@@ -15733,7 +15742,7 @@ 
 	(plus:P (match_dup 3)
 		(const_int 1)))]
   "!(fixed_regs[SI_REG] || fixed_regs[DI_REG])"
-  "movsb"
+  "%^movsb"
   [(set_attr "type" "str")
    (set_attr "memory" "both")
    (set (attr "prefix_rex")
@@ -15756,20 +15765,20 @@ 
   "ix86_current_function_needs_cld = 1;")
 
 (define_insn "*rep_movdi_rex64"
-  [(set (match_operand:DI 2 "register_operand" "=c") (const_int 0))
-   (set (match_operand:DI 0 "register_operand" "=D")
-        (plus:DI (ashift:DI (match_operand:DI 5 "register_operand" "2")
-			    (const_int 3))
-		 (match_operand:DI 3 "register_operand" "0")))
-   (set (match_operand:DI 1 "register_operand" "=S")
-        (plus:DI (ashift:DI (match_dup 5) (const_int 3))
-		 (match_operand:DI 4 "register_operand" "1")))
+  [(set (match_operand:P 2 "register_operand" "=c") (const_int 0))
+   (set (match_operand:P 0 "register_operand" "=D")
+        (plus:P (ashift:P (match_operand:P 5 "register_operand" "2")
+			  (const_int 3))
+		(match_operand:P 3 "register_operand" "0")))
+   (set (match_operand:P 1 "register_operand" "=S")
+        (plus:P (ashift:P (match_dup 5) (const_int 3))
+		(match_operand:P 4 "register_operand" "1")))
    (set (mem:BLK (match_dup 3))
 	(mem:BLK (match_dup 4)))
    (use (match_dup 5))]
   "TARGET_64BIT
    && !(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])"
-  "rep{%;} movsq"
+  "%^rep{%;} movsq"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
    (set_attr "memory" "both")
@@ -15788,7 +15797,7 @@ 
 	(mem:BLK (match_dup 4)))
    (use (match_dup 5))]
   "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])"
-  "rep{%;} movs{l|d}"
+  "%^rep{%;} movs{l|d}"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
    (set_attr "memory" "both")
@@ -15805,7 +15814,7 @@ 
 	(mem:BLK (match_dup 4)))
    (use (match_dup 5))]
   "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])"
-  "rep{%;} movsb"
+  "%^rep{%;} movsb"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
    (set_attr "memory" "both")
@@ -15866,14 +15875,14 @@ 
   "ix86_current_function_needs_cld = 1;")
 
 (define_insn "*strsetdi_rex_1"
-  [(set (mem:DI (match_operand:DI 1 "register_operand" "0"))
+  [(set (mem:DI (match_operand:P 1 "register_operand" "0"))
 	(match_operand:DI 2 "register_operand" "a"))
-   (set (match_operand:DI 0 "register_operand" "=D")
-	(plus:DI (match_dup 1)
-		 (const_int 8)))]
+   (set (match_operand:P 0 "register_operand" "=D")
+	(plus:P (match_dup 1)
+		(const_int 8)))]
   "TARGET_64BIT
    && !(fixed_regs[AX_REG] || fixed_regs[DI_REG])"
-  "stosq"
+  "%^stosq"
   [(set_attr "type" "str")
    (set_attr "memory" "store")
    (set_attr "mode" "DI")])
@@ -15885,7 +15894,7 @@ 
 	(plus:P (match_dup 1)
 		(const_int 4)))]
   "!(fixed_regs[AX_REG] || fixed_regs[DI_REG])"
-  "stos{l|d}"
+  "%^stos{l|d}"
   [(set_attr "type" "str")
    (set_attr "memory" "store")
    (set_attr "mode" "SI")])
@@ -15897,7 +15906,7 @@ 
 	(plus:P (match_dup 1)
 		(const_int 2)))]
   "!(fixed_regs[AX_REG] || fixed_regs[DI_REG])"
-  "stosw"
+  "%^stosw"
   [(set_attr "type" "str")
    (set_attr "memory" "store")
    (set_attr "mode" "HI")])
@@ -15909,7 +15918,7 @@ 
 	(plus:P (match_dup 1)
 		(const_int 1)))]
   "!(fixed_regs[AX_REG] || fixed_regs[DI_REG])"
-  "stosb"
+  "%^stosb"
   [(set_attr "type" "str")
    (set_attr "memory" "store")
    (set (attr "prefix_rex")
@@ -15930,18 +15939,18 @@ 
   "ix86_current_function_needs_cld = 1;")
 
 (define_insn "*rep_stosdi_rex64"
-  [(set (match_operand:DI 1 "register_operand" "=c") (const_int 0))
-   (set (match_operand:DI 0 "register_operand" "=D")
-        (plus:DI (ashift:DI (match_operand:DI 4 "register_operand" "1")
-			    (const_int 3))
-		 (match_operand:DI 3 "register_operand" "0")))
+  [(set (match_operand:P 1 "register_operand" "=c") (const_int 0))
+   (set (match_operand:P 0 "register_operand" "=D")
+        (plus:P (ashift:P (match_operand:P 4 "register_operand" "1")
+			  (const_int 3))
+		 (match_operand:P 3 "register_operand" "0")))
    (set (mem:BLK (match_dup 3))
 	(const_int 0))
    (use (match_operand:DI 2 "register_operand" "a"))
    (use (match_dup 4))]
   "TARGET_64BIT
    && !(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])"
-  "rep{%;} stosq"
+  "%^rep{%;} stosq"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
    (set_attr "memory" "store")
@@ -15958,7 +15967,7 @@ 
    (use (match_operand:SI 2 "register_operand" "a"))
    (use (match_dup 4))]
   "!(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])"
-  "rep{%;} stos{l|d}"
+  "%^rep{%;} stos{l|d}"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
    (set_attr "memory" "store")
@@ -15974,7 +15983,7 @@ 
    (use (match_operand:QI 2 "register_operand" "a"))
    (use (match_dup 4))]
   "!(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])"
-  "rep{%;} stosb"
+  "%^rep{%;} stosb"
   [(set_attr "type" "str")
    (set_attr "prefix_rep" "1")
    (set_attr "memory" "store")
@@ -16095,7 +16104,7 @@ 
    (clobber (match_operand:P 1 "register_operand" "=D"))
    (clobber (match_operand:P 2 "register_operand" "=c"))]
   "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])"
-  "repz{%;} cmpsb"
+  "%^repz{%;} cmpsb"
   [(set_attr "type" "str")
    (set_attr "mode" "QI")
    (set (attr "prefix_rex")
@@ -16135,7 +16144,7 @@ 
    (clobber (match_operand:P 1 "register_operand" "=D"))
    (clobber (match_operand:P 2 "register_operand" "=c"))]
   "!(fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])"
-  "repz{%;} cmpsb"
+  "%^repz{%;} cmpsb"
   [(set_attr "type" "str")
    (set_attr "mode" "QI")
    (set (attr "prefix_rex")
@@ -16176,7 +16185,7 @@ 
    (clobber (match_operand:P 1 "register_operand" "=D"))
    (clobber (reg:CC FLAGS_REG))]
   "!(fixed_regs[AX_REG] || fixed_regs[CX_REG] || fixed_regs[DI_REG])"
-  "repnz{%;} scasb"
+  "%^repnz{%;} scasb"
   [(set_attr "type" "str")
    (set_attr "mode" "QI")
    (set (attr "prefix_rex")