Patchwork x86: emit tzcnt unconditionally

login
register
mail settings
Submitter Uros Bizjak
Date May 7, 2012, 7:36 a.m.
Message ID <CAFULd4a1jRMjQG30jP+VkCYqFxZjRsk-tNX-VOox7UND1X+s1Q@mail.gmail.com>
Download mbox | patch
Permalink /patch/157246/
State New
Headers show

Comments

Uros Bizjak - May 7, 2012, 7:36 a.m.
On Mon, May 7, 2012 at 7:43 AM, Jakub Jelinek <jakub@redhat.com> wrote:

>> Index: i386.md
>> ===================================================================
>> --- i386.md   (revision 187217)
>> +++ i386.md   (working copy)
>> @@ -12112,9 +12112,22 @@
>>     (set (match_operand:SWI48 0 "register_operand" "=r")
>>       (ctz:SWI48 (match_dup 1)))]
>>    ""
>> -  "bsf{<imodesuffix>}\t{%1, %0|%0, %1}"
>> +{
>> +  if (optimize_function_for_size_p (cfun))
>> +    return "bsf{<imodesuffix>}\t{%1, %0|%0, %1}";
>> +  else if (TARGET_BMI)
>> +    return "tzcnt{<imodesuffix>}\t{%1, %0|%0, %1}";
>> +  else
>> +    /* tzcnt expands to rep;bsf and we can use it even if !TARGET_BMI.  */
>> +    return "rep; bsf{<imodesuffix>}\t{%1, %0|%0, %1}";
>> +}
>
> Shouldn't that be done only for generic tuning?  If somebody uses
> -mtune=native, then emitting rep; bsf is overkill, the code is intended
> to be run on a CPU without (or with TARGET_BMI with) tzcnt insn support.

Yes, this is a good idea.

Something like attached patch?

Thanks,
Uros.
Jakub Jelinek - May 7, 2012, 8 a.m.
On Mon, May 07, 2012 at 09:36:38AM +0200, Uros Bizjak wrote:
> On Mon, May 7, 2012 at 7:43 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> >> Index: i386.md
> >> ===================================================================
> >> --- i386.md   (revision 187217)
> >> +++ i386.md   (working copy)
> >> @@ -12112,9 +12112,22 @@
> >>     (set (match_operand:SWI48 0 "register_operand" "=r")
> >>       (ctz:SWI48 (match_dup 1)))]
> >>    ""
> >> -  "bsf{<imodesuffix>}\t{%1, %0|%0, %1}"
> >> +{
> >> +  if (optimize_function_for_size_p (cfun))
> >> +    return "bsf{<imodesuffix>}\t{%1, %0|%0, %1}";
> >> +  else if (TARGET_BMI)
> >> +    return "tzcnt{<imodesuffix>}\t{%1, %0|%0, %1}";
> >> +  else
> >> +    /* tzcnt expands to rep;bsf and we can use it even if !TARGET_BMI.  */
> >> +    return "rep; bsf{<imodesuffix>}\t{%1, %0|%0, %1}";
> >> +}
> >
> > Shouldn't that be done only for generic tuning?  If somebody uses
> > -mtune=native, then emitting rep; bsf is overkill, the code is intended
> > to be run on a CPU without (or with TARGET_BMI with) tzcnt insn support.
> 
> Yes, this is a good idea.
> 
> Something like attached patch?

Yeah, but probably with the TARGET_BMI and optimize_function_for_size_p
lines swapped.

Because i386.h defines
CTZ_DEFINED_VALUE_AT_ZERO to 1 for TARGET_BMI, therefore it isn't
undefined for TARGET_BMI, so even for -Os you can't just use bsf.

	Jakub

Patch

Index: i386.md
===================================================================
--- i386.md	(revision 187223)
+++ i386.md	(working copy)
@@ -12114,18 +12114,22 @@ 
   ""
 {
   if (optimize_function_for_size_p (cfun))
-    return "bsf{<imodesuffix>}\t{%1, %0|%0, %1}";
+    ;
   else if (TARGET_BMI)
     return "tzcnt{<imodesuffix>}\t{%1, %0|%0, %1}";
-  else 
+  else if (TARGET_GENERIC)
     /* tzcnt expands to rep;bsf and we can use it even if !TARGET_BMI.  */
     return "rep; bsf{<imodesuffix>}\t{%1, %0|%0, %1}";
+
+  return "bsf{<imodesuffix>}\t{%1, %0|%0, %1}";
 }
   [(set_attr "type" "alu1")
    (set_attr "prefix_0f" "1")
    (set (attr "prefix_rep")
      (if_then_else
-       (match_test "optimize_function_for_size_p (cfun)")
+       (and (match_test "optimize_function_for_size_p (cfun)")
+	    (not (ior (match_test "TARGET_BMI")
+		      (match_test "TARGET_GENERIC"))))
        (const_string "0")
        (const_string "1")))
    (set_attr "mode" "<MODE>")])
@@ -12137,18 +12141,22 @@ 
   ""
 {
   if (optimize_function_for_size_p (cfun))
-    return "bsf{<imodesuffix>}\t{%1, %0|%0, %1}";
+    ;
   else if (TARGET_BMI)
     return "tzcnt{<imodesuffix>}\t{%1, %0|%0, %1}";
-  else 
+  else if (TARGET_GENERIC)
     /* tzcnt expands to rep;bsf and we can use it even if !TARGET_BMI.  */
     return "rep; bsf{<imodesuffix>}\t{%1, %0|%0, %1}";
+
+  return "bsf{<imodesuffix>}\t{%1, %0|%0, %1}";
 }
   [(set_attr "type" "alu1")
    (set_attr "prefix_0f" "1")
    (set (attr "prefix_rep")
      (if_then_else
-       (match_test "optimize_function_for_size_p (cfun)")
+       (and (match_test "optimize_function_for_size_p (cfun)")
+	    (not (ior (match_test "TARGET_BMI")
+		      (match_test "TARGET_GENERIC"))))
        (const_string "0")
        (const_string "1")))
    (set_attr "mode" "<MODE>")])