Patchwork [off,list] Re: [PATCH] x86: use 'rep bsf' syntax when assembler supports it

login
register
mail settings
Submitter Roland McGrath
Date June 22, 2012, 10 p.m.
Message ID <CAB=4xhpXQQn-wJV5GVbjDSRqyfYAJ19QMYJWOLOLXercJPhgxQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/166679/
State New
Headers show

Comments

Roland McGrath - June 22, 2012, 10 p.m.
Here is an alternative patch that just changes the configure test
controlling %; so it will elide the ; only for an assembler that
also accepts 'rep bsf', 'rep bsr', and 'rep ret', and just uses
%; for these cases too.  You'll need to have built binutils from its trunk
within the last five minutes or so to have an assembler that passes the
test now.


Thanks,
Roland


gcc/
2012-06-22  Roland McGrath  <mcgrathr@google.com>

	* configure.ac (HAVE_AS_IX86_REP_LOCK_PREFIX): Also require that the
	assembler accept 'rep bsf ...', 'rep bsr ...', and 'rep ret'.
	* configure: Regenerated.
	* config/i386/i386.md (simple_return_internal_long): Use %;
	(ctz<mode>2): Likewise.
Uros Bizjak - July 1, 2012, 8:08 a.m.
On Sat, Jun 23, 2012 at 12:00 AM, Roland McGrath <mcgrathr@google.com> wrote:
> Here is an alternative patch that just changes the configure test
> controlling %; so it will elide the ; only for an assembler that
> also accepts 'rep bsf', 'rep bsr', and 'rep ret', and just uses
> %; for these cases too.  You'll need to have built binutils from its trunk
> within the last five minutes or so to have an assembler that passes the
> test now.

We found another one ... "rep nop" that implements "pause". It looks
that this is really the last rep prefixed insn.

With existing binutils, the use of ";" was mostly cosmetic, assembler
checked that rep was indeed followed by a string instruction. Other
than that, older binutils didn't care about the placement of ";".

> gcc/
> 2012-06-22  Roland McGrath  <mcgrathr@google.com>
>
>         * configure.ac (HAVE_AS_IX86_REP_LOCK_PREFIX): Also require that the
>         assembler accept 'rep bsf ...', 'rep bsr ...', and 'rep ret'.
>         * configure: Regenerated.
>         * config/i386/i386.md (simple_return_internal_long): Use %;
>         (ctz<mode>2): Likewise.

Based on the observation above, the patch is OK for mainline, but
please also handle "rep nop" case.

Thanks,
Uros.

Patch

diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 879b87b..16056c1 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -11902,7 +11902,7 @@ 
   [(simple_return)
    (unspec [(const_int 0)] UNSPEC_REP)]
   "reload_completed"
-  "rep\;ret"
+  "rep%; ret"
   [(set_attr "length" "2")
    (set_attr "atom_unit" "jeu")
    (set_attr "length_immediate" "0")
@@ -12234,8 +12234,8 @@ 
   else if (optimize_function_for_size_p (cfun))
     ;
   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}";
+    /* 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}";
 }
diff --git a/gcc/configure b/gcc/configure
index 1fdf0af..5e8107f 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -24791,7 +24791,10 @@  else
   if test x$gcc_cv_as != x; then
     $as_echo 'rep movsl
 	 lock addl %edi, (%eax,%esi)
-	 lock orl $0, (%esp)' > conftest.s
+	 lock orl $0, (%esp)
+	 rep bsf %ecx, %eax
+	 rep bsr %ecx, %eax
+	 rep ret' > conftest.s
     if { ac_try='$gcc_cv_as $gcc_cv_as_flags  -o conftest.o conftest.s >&5'
   { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
   (eval $ac_try) 2>&5
@@ -28698,4 +28701,3 @@  if test -n "$ac_unrecognized_opts" && test
"$enable_option_checking" != no; then
 $as_echo "$as_me: WARNING: unrecognized options: $ac_unrecognized_opts" >&2;}
 fi

-
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 22dab55..428a21a 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -3642,7 +3642,10 @@  foo:	nop
         gcc_cv_as_ix86_rep_lock_prefix,,,
 	[rep movsl
 	 lock addl %edi, (%eax,%esi)
-	 lock orl $0, (%esp)],,
+	 lock orl $0, (%esp)
+	 rep bsf %ecx, %eax
+	 rep bsr %ecx, %eax
+	 rep ret],,
         [AC_DEFINE(HAVE_AS_IX86_REP_LOCK_PREFIX, 1,
           [Define if the assembler supports 'rep <insn>, lock <insn>'.])])

@@ -5163,4 +5166,3 @@  done
 ],
 [subdirs='$subdirs'])
 AC_OUTPUT
-