PATCH: Add pause intrinsic

PATCH: Add pause intrinsic

[H.J. Lu]

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

Hi,

This patch implements pause intrinsic suggested by Andi.  OK
for trunk?

Thanks.


-- 
H.J.
---
gcc/

2011-05-24  H.J. Lu  <hongjiu.lu@intel.com>

	* config/i386/i386.c (ix86_builtins): Add IX86_BUILTIN_PAUSE.
	(bdesc_special_args): Add pause intrinsic.

	* config/i386/i386.md (UNSPEC_PAUSE): New.
	(pause): Likewise.
	(*pause): Likewise.
	* config/i386/ia32intrin.h (__pause): Likewise.

	* doc/extend.texi (X86 Built-in Functions): Add documentation for
	pause intrinsic.

gcc/testsuite/

2011-05-24  H.J. Lu  <hongjiu.lu@intel.com>

	 * gcc.target/i386/pause-1.c: New.

[-- Attachment #2: gcc-pause-2.patch --]
[-- Type: text/x-diff, Size: 3903 bytes --]

gcc/

2011-05-24  H.J. Lu  <hongjiu.lu@intel.com>

	* config/i386/i386.c (ix86_builtins): Add IX86_BUILTIN_PAUSE.
	(bdesc_special_args): Add pause intrinsic.

	* config/i386/i386.md (UNSPEC_PAUSE): New.
	(pause): Likewise.
	(*pause): Likewise.
	* config/i386/ia32intrin.h (__pause): Likewise.

	* doc/extend.texi (X86 Built-in Functions): Add documentation for
	pause intrinsic.

gcc/testsuite/

2011-05-24  H.J. Lu  <hongjiu.lu@intel.com>

	 * gcc.target/i386/pause-1.c: New.

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 854e376..174caa0 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -23912,6 +23912,7 @@ enum ix86_builtins
   IX86_BUILTIN_CLFLUSH,
   IX86_BUILTIN_MFENCE,
   IX86_BUILTIN_LFENCE,
+  IX86_BUILTIN_PAUSE,
 
   IX86_BUILTIN_BSRSI,
   IX86_BUILTIN_BSRDI,
@@ -24664,6 +24665,7 @@ static const struct builtin_description bdesc_special_args[] =
 {
   { ~OPTION_MASK_ISA_64BIT, CODE_FOR_rdtsc, "__builtin_ia32_rdtsc", IX86_BUILTIN_RDTSC, UNKNOWN, (int) UINT64_FTYPE_VOID },
   { ~OPTION_MASK_ISA_64BIT, CODE_FOR_rdtscp, "__builtin_ia32_rdtscp", IX86_BUILTIN_RDTSCP, UNKNOWN, (int) UINT64_FTYPE_PUNSIGNED },
+  { ~OPTION_MASK_ISA_64BIT, CODE_FOR_pause, "__builtin_ia32_pause", IX86_BUILTIN_PAUSE, UNKNOWN, (int) VOID_FTYPE_VOID },
 
   /* MMX */
   { OPTION_MASK_ISA_MMX, CODE_FOR_mmx_emms, "__builtin_ia32_emms", IX86_BUILTIN_EMMS, UNKNOWN, (int) VOID_FTYPE_VOID },
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 49f1ee7..801573c 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -108,6 +108,7 @@
   UNSPEC_TRUNC_NOOP
   UNSPEC_DIV_ALREADY_SPLIT
   UNSPEC_CALL_NEEDS_VZEROUPPER
+  UNSPEC_PAUSE
 
   ;; For SSE/MMX support:
   UNSPEC_FIX_NOTRUNC
@@ -17716,6 +17717,25 @@
   [(set_attr "type" "other")
    (set_attr "prefix_extra" "1")])
 
+(define_expand "pause"
+  [(set (match_dup 0)
+	(unspec:BLK [(match_dup 0)] UNSPEC_PAUSE))]
+  ""
+{
+  operands[0] = gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (Pmode));
+  MEM_VOLATILE_P (operands[0]) = 1;
+})
+
+;; Use "rep; nop", instead of "pause", to support older assemblers.
+;; They have the same encoding.
+(define_insn "*pause"
+  [(set (match_operand:BLK 0 "" "")
+	(unspec:BLK [(match_dup 0)] UNSPEC_PAUSE))]
+  ""
+  "rep; nop"
+  [(set_attr "length" "2")
+   (set_attr "memory" "unknown")])
+
 (include "mmx.md")
 (include "sse.md")
 (include "sync.md")
diff --git a/gcc/config/i386/ia32intrin.h b/gcc/config/i386/ia32intrin.h
index 76c20a6..42ebd17 100644
--- a/gcc/config/i386/ia32intrin.h
+++ b/gcc/config/i386/ia32intrin.h
@@ -153,6 +153,14 @@ __rord (unsigned int __X, int __C)
   return (__X >> __C) | (__X << (32 - __C));
 }
 
+/* Pause */
+extern __inline void
+__attribute__((__gnu_inline__, __always_inline__, __artificial__))
+__pause (void)
+{
+  __builtin_ia32_pause ();
+}
+
 #ifdef __x86_64__
 /* 64bit bsf */
 extern __inline int
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index b5702db..bd2f080 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -8695,6 +8695,13 @@ __float128 __builtin_fabsq (__float128)
 __float128 __builtin_copysignq (__float128, __float128)
 @end smallexample
 
+The following built-in function is always available.
+
+@table @code
+@item void __builtin_ia32_pause (void)
+Generates the @code{pause} machine instruction with full memory barrier.
+@end table
+
 The following floating point built-in functions are made available in the
 64-bit mode.
 
diff --git a/gcc/testsuite/gcc.target/i386/pause-1.c b/gcc/testsuite/gcc.target/i386/pause-1.c
new file mode 100644
index 0000000..50eb8e7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pause-1.c
@@ -0,0 +1,11 @@
+/* Test that we generate pause instruction.  */
+/* { dg-do compile } */
+/* { dg-options "-O2 -dp" } */
+/* { dg-final { scan-assembler-times "\\*pause" 1 } } */
+
+#include <x86intrin.h>
+
+void foo(void)
+{
+  __pause();
+}

Re: PATCH: Add pause intrinsic

[Andrew Haley]

On 05/24/2011 07:28 PM, H.J. Lu wrote:

> This patch implements pause intrinsic suggested by Andi.  OK
> for trunk?

What does "full memory barrier" here mean?

+@table @code
+@item void __builtin_ia32_pause (void)
+Generates the @code{pause} machine instruction with full memory barrier.
+@end table

There a memory clobber, but no barrier instruction AFAICS.  The
doc needs to explain it a bit better.

Andrew.

Re: PATCH: Add pause intrinsic

[Richard Guenther]

On Wed, May 25, 2011 at 12:26 PM, Andrew Haley <aph@redhat.com> wrote:
> On 05/24/2011 07:28 PM, H.J. Lu wrote:
>
>> This patch implements pause intrinsic suggested by Andi.  OK
>> for trunk?
>
> What does "full memory barrier" here mean?
>
> +@table @code
> +@item void __builtin_ia32_pause (void)
> +Generates the @code{pause} machine instruction with full memory barrier.
> +@end table
>
> There a memory clobber, but no barrier instruction AFAICS.  The
> doc needs to explain it a bit better.

The name also sounds odd to me (reminds me of Fortran PAUSE ...).

Richard.

> Andrew.
>

Re: PATCH: Add pause intrinsic

[H.J. Lu]

On Wed, May 25, 2011 at 3:31 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, May 25, 2011 at 12:26 PM, Andrew Haley <aph@redhat.com> wrote:
>> On 05/24/2011 07:28 PM, H.J. Lu wrote:
>>
>>> This patch implements pause intrinsic suggested by Andi.  OK
>>> for trunk?
>>
>> What does "full memory barrier" here mean?
>>
>> +@table @code
>> +@item void __builtin_ia32_pause (void)
>> +Generates the @code{pause} machine instruction with full memory barrier.
>> +@end table
>>
>> There a memory clobber, but no barrier instruction AFAICS.  The
>> doc needs to explain it a bit better.
>
> The name also sounds odd to me (reminds me of Fortran PAUSE ...).
>

__builtin_ia32_pause is the C intrinsic for x86 machine instruction.
I don't think people will get confused by its name.


-- 
H.J.

Re: PATCH: Add pause intrinsic

[H.J. Lu]

On Wed, May 25, 2011 at 3:26 AM, Andrew Haley <aph@redhat.com> wrote:
> On 05/24/2011 07:28 PM, H.J. Lu wrote:
>
>> This patch implements pause intrinsic suggested by Andi.  OK
>> for trunk?
>
> What does "full memory barrier" here mean?
>
> +@table @code
> +@item void __builtin_ia32_pause (void)
> +Generates the @code{pause} machine instruction with full memory barrier.
> +@end table
>
> There a memory clobber, but no barrier instruction AFAICS.  The
> doc needs to explain it a bit better.
>

There are read/load memory barrier, write/store memory barrier and full/general
memory barrier.  You can find them at

http://www.kernel.org/doc/Documentation/memory-barriers.txt

Should I include a pointer to it?

-- 
H.J.

Re: PATCH: Add pause intrinsic

[Andrew Haley]

On 05/25/2011 01:34 PM, H.J. Lu wrote:
> On Wed, May 25, 2011 at 3:26 AM, Andrew Haley <aph@redhat.com> wrote:
>> On 05/24/2011 07:28 PM, H.J. Lu wrote:
>>
>>> This patch implements pause intrinsic suggested by Andi.  OK
>>> for trunk?
>>
>> What does "full memory barrier" here mean?
>>
>> +@table @code
>> +@item void __builtin_ia32_pause (void)
>> +Generates the @code{pause} machine instruction with full memory barrier.
>> +@end table
>>
>> There a memory clobber, but no barrier instruction AFAICS.  The
>> doc needs to explain it a bit better.
> 
> There are read/load memory barrier, write/store memory barrier and full/general
> memory barrier.  You can find them at
> 
> http://www.kernel.org/doc/Documentation/memory-barriers.txt
> 
> Should I include a pointer to it?

No.  I know perfectly well what memory barriers are.  I'm not asking
what "full memory barrier" means.

What barrier instruction(s) does __builtin_ia32_pause() generate?
All I see in the patch is "rep; nop".  Is that really a full memory
barrier?

Andrew.

Re: PATCH: Add pause intrinsic

[H.J. Lu]

On Wed, May 25, 2011 at 7:36 AM, Andrew Haley <aph@redhat.com> wrote:
> On 05/25/2011 01:34 PM, H.J. Lu wrote:
>> On Wed, May 25, 2011 at 3:26 AM, Andrew Haley <aph@redhat.com> wrote:
>>> On 05/24/2011 07:28 PM, H.J. Lu wrote:
>>>
>>>> This patch implements pause intrinsic suggested by Andi.  OK
>>>> for trunk?
>>>
>>> What does "full memory barrier" here mean?
>>>
>>> +@table @code
>>> +@item void __builtin_ia32_pause (void)
>>> +Generates the @code{pause} machine instruction with full memory barrier.
>>> +@end table
>>>
>>> There a memory clobber, but no barrier instruction AFAICS.  The
>>> doc needs to explain it a bit better.
>>
>> There are read/load memory barrier, write/store memory barrier and full/general
>> memory barrier.  You can find them at
>>
>> http://www.kernel.org/doc/Documentation/memory-barriers.txt
>>
>> Should I include a pointer to it?
>
> No.  I know perfectly well what memory barriers are.  I'm not asking
> what "full memory barrier" means.
>
> What barrier instruction(s) does __builtin_ia32_pause() generate?
> All I see in the patch is "rep; nop".  Is that really a full memory
> barrier?
>

It is a full memory barrier in the sense that compiler won't move
load/store across it.  It is intended for kernel.

-- 
H.J.

Re: PATCH: Add pause intrinsic

[Richard Guenther]

On Wed, May 25, 2011 at 4:47 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, May 25, 2011 at 7:36 AM, Andrew Haley <aph@redhat.com> wrote:
>> On 05/25/2011 01:34 PM, H.J. Lu wrote:
>>> On Wed, May 25, 2011 at 3:26 AM, Andrew Haley <aph@redhat.com> wrote:
>>>> On 05/24/2011 07:28 PM, H.J. Lu wrote:
>>>>
>>>>> This patch implements pause intrinsic suggested by Andi.  OK
>>>>> for trunk?
>>>>
>>>> What does "full memory barrier" here mean?
>>>>
>>>> +@table @code
>>>> +@item void __builtin_ia32_pause (void)
>>>> +Generates the @code{pause} machine instruction with full memory barrier.
>>>> +@end table
>>>>
>>>> There a memory clobber, but no barrier instruction AFAICS.  The
>>>> doc needs to explain it a bit better.
>>>
>>> There are read/load memory barrier, write/store memory barrier and full/general
>>> memory barrier.  You can find them at
>>>
>>> http://www.kernel.org/doc/Documentation/memory-barriers.txt
>>>
>>> Should I include a pointer to it?
>>
>> No.  I know perfectly well what memory barriers are.  I'm not asking
>> what "full memory barrier" means.
>>
>> What barrier instruction(s) does __builtin_ia32_pause() generate?
>> All I see in the patch is "rep; nop".  Is that really a full memory
>> barrier?
>>
>
> It is a full memory barrier in the sense that compiler won't move
> load/store across it.  It is intended for kernel.

There is no such thing if you include accesses to automatic variables.

Richard.

> --
> H.J.
>

Re: PATCH: Add pause intrinsic

[Andrew Haley]

On 05/25/2011 03:47 PM, H.J. Lu wrote:
> On Wed, May 25, 2011 at 7:36 AM, Andrew Haley <aph@redhat.com> wrote:
>> On 05/25/2011 01:34 PM, H.J. Lu wrote:
>>> On Wed, May 25, 2011 at 3:26 AM, Andrew Haley <aph@redhat.com> wrote:
>>>> On 05/24/2011 07:28 PM, H.J. Lu wrote:
>>>>
>>>>> This patch implements pause intrinsic suggested by Andi.  OK
>>>>> for trunk?
>>>>
>>>> What does "full memory barrier" here mean?
>>>>
>>>> +@table @code
>>>> +@item void __builtin_ia32_pause (void)
>>>> +Generates the @code{pause} machine instruction with full memory barrier.
>>>> +@end table
>>>>
>>>> There a memory clobber, but no barrier instruction AFAICS.  The
>>>> doc needs to explain it a bit better.
>>>
>>> There are read/load memory barrier, write/store memory barrier and full/general
>>> memory barrier.  You can find them at
>>>
>>> http://www.kernel.org/doc/Documentation/memory-barriers.txt
>>>
>>> Should I include a pointer to it?
>>
>> No.  I know perfectly well what memory barriers are.  I'm not asking
>> what "full memory barrier" means.
>>
>> What barrier instruction(s) does __builtin_ia32_pause() generate?
>> All I see in the patch is "rep; nop".  Is that really a full memory
>> barrier?
> 
> It is a full memory barrier in the sense that compiler won't move
> load/store across it.  It is intended for kernel.

Right, so it is, in fact, not a full memory barrier.  I thought not.
I's no more a full memory barrier than a simple asm volatile("") .
The doc needs to explain that a bit better.

Andrew.

Re: PATCH: Add pause intrinsic

[Richard Guenther]

On Wed, May 25, 2011 at 4:54 PM, Andrew Haley <aph@redhat.com> wrote:
> On 05/25/2011 03:47 PM, H.J. Lu wrote:
>> On Wed, May 25, 2011 at 7:36 AM, Andrew Haley <aph@redhat.com> wrote:
>>> On 05/25/2011 01:34 PM, H.J. Lu wrote:
>>>> On Wed, May 25, 2011 at 3:26 AM, Andrew Haley <aph@redhat.com> wrote:
>>>>> On 05/24/2011 07:28 PM, H.J. Lu wrote:
>>>>>
>>>>>> This patch implements pause intrinsic suggested by Andi.  OK
>>>>>> for trunk?
>>>>>
>>>>> What does "full memory barrier" here mean?
>>>>>
>>>>> +@table @code
>>>>> +@item void __builtin_ia32_pause (void)
>>>>> +Generates the @code{pause} machine instruction with full memory barrier.
>>>>> +@end table
>>>>>
>>>>> There a memory clobber, but no barrier instruction AFAICS.  The
>>>>> doc needs to explain it a bit better.
>>>>
>>>> There are read/load memory barrier, write/store memory barrier and full/general
>>>> memory barrier.  You can find them at
>>>>
>>>> http://www.kernel.org/doc/Documentation/memory-barriers.txt
>>>>
>>>> Should I include a pointer to it?
>>>
>>> No.  I know perfectly well what memory barriers are.  I'm not asking
>>> what "full memory barrier" means.
>>>
>>> What barrier instruction(s) does __builtin_ia32_pause() generate?
>>> All I see in the patch is "rep; nop".  Is that really a full memory
>>> barrier?
>>
>> It is a full memory barrier in the sense that compiler won't move
>> load/store across it.  It is intended for kernel.
>
> Right, so it is, in fact, not a full memory barrier.  I thought not.
> I's no more a full memory barrier than a simple asm volatile("") .
> The doc needs to explain that a bit better.

asm volatile ("" : : : "memory") in fact will work as a full memory barrier
because we are very very lazy in disambiguating against asms
(but that should change, at least a tiny bit).  Function calls otoh are
pretty well optimized.

Richard.

> Andrew.
>

Re: PATCH: Add pause intrinsic

[Andrew Haley]

On 05/25/2011 03:57 PM, Richard Guenther wrote:
> 
> asm volatile ("" : : : "memory") in fact will work as a full memory barrier

How?  You surely need MFENCE or somesuch, unless all you
care about is a compiler barrier.  That's what I think needs
to be clarified.

Andrew.

Re: PATCH: Add pause intrinsic

[Richard Guenther]

On Wed, May 25, 2011 at 5:09 PM, Andrew Haley <aph@redhat.com> wrote:
> On 05/25/2011 03:57 PM, Richard Guenther wrote:
>>
>> asm volatile ("" : : : "memory") in fact will work as a full memory barrier
>
> How?  You surely need MFENCE or somesuch, unless all you
> care about is a compiler barrier.  That's what I think needs
> to be clarified.

Well, yes, I'm talking about the compiler memory barrier.

Richard.

> Andrew.
>

Re: PATCH: Add pause intrinsic

[Michael Matz]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 527 bytes --]

Hi,

On Wed, 25 May 2011, Richard Guenther wrote:

> >> asm volatile ("" : : : "memory") in fact will work as a full memory 
> >> barrier
> >
> > How?  You surely need MFENCE or somesuch, unless all you care about is 
> > a compiler barrier.  That's what I think needs to be clarified.
> 
> Well, yes, I'm talking about the compiler memory barrier.

Something that we conventionally call "optimization barrier" :)  memory 
barrier has a fixed meaning which we shouldn't use in this case, it's 
confusing.


Ciao,
Michael.

Re: PATCH: Add pause intrinsic

[Richard Guenther]

On Wed, May 25, 2011 at 5:20 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Wed, 25 May 2011, Richard Guenther wrote:
>
>> >> asm volatile ("" : : : "memory") in fact will work as a full memory
>> >> barrier
>> >
>> > How?  You surely need MFENCE or somesuch, unless all you care about is
>> > a compiler barrier.  That's what I think needs to be clarified.
>>
>> Well, yes, I'm talking about the compiler memory barrier.
>
> Something that we conventionally call "optimization barrier" :)  memory
> barrier has a fixed meaning which we shouldn't use in this case, it's
> confusing.

Sure ;)

And to keep the info in a suitable thread what I'd like to improve here
is to make us disambiguate memory loads/stores against asms that
have no memory outputs/inputs.

Richard.

Re: PATCH: Add pause intrinsic

[H.J. Lu]

On Wed, May 25, 2011 at 8:27 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, May 25, 2011 at 5:20 PM, Michael Matz <matz@suse.de> wrote:
>> Hi,
>>
>> On Wed, 25 May 2011, Richard Guenther wrote:
>>
>>> >> asm volatile ("" : : : "memory") in fact will work as a full memory
>>> >> barrier
>>> >
>>> > How?  You surely need MFENCE or somesuch, unless all you care about is
>>> > a compiler barrier.  That's what I think needs to be clarified.
>>>
>>> Well, yes, I'm talking about the compiler memory barrier.
>>
>> Something that we conventionally call "optimization barrier" :)  memory
>> barrier has a fixed meaning which we shouldn't use in this case, it's
>> confusing.
>
> Sure ;)
>
> And to keep the info in a suitable thread what I'd like to improve here
> is to make us disambiguate memory loads/stores against asms that
> have no memory outputs/inputs.
>

Please let me know how I should improve the document,

Thanks.


-- 
H.J.

Re: PATCH: Add pause intrinsic

[Andrew Haley]

On 05/25/2011 04:32 PM, H.J. Lu wrote:
> On Wed, May 25, 2011 at 8:27 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Wed, May 25, 2011 at 5:20 PM, Michael Matz <matz@suse.de> wrote:
>>> Hi,
>>>
>>> On Wed, 25 May 2011, Richard Guenther wrote:
>>>
>>>>>> asm volatile ("" : : : "memory") in fact will work as a full memory
>>>>>> barrier
>>>>>
>>>>> How?  You surely need MFENCE or somesuch, unless all you care about is
>>>>> a compiler barrier.  That's what I think needs to be clarified.
>>>>
>>>> Well, yes, I'm talking about the compiler memory barrier.
>>>
>>> Something that we conventionally call "optimization barrier" :)  memory
>>> barrier has a fixed meaning which we shouldn't use in this case, it's
>>> confusing.
>>
>> Sure ;)
>>
>> And to keep the info in a suitable thread what I'd like to improve here
>> is to make us disambiguate memory loads/stores against asms that
>> have no memory outputs/inputs.
>>
> 
> Please let me know how I should improve the document,

"Compiler memory barrier" seems to be well-understood.  I suggest

+Generates the @code{pause} machine instruction with a compiler memory barrier.

It's clear enough.

Andrew.

Re: PATCH: Add pause intrinsic

[H.J. Lu]

On Wed, May 25, 2011 at 9:43 AM, Andrew Haley <aph@redhat.com> wrote:
> On 05/25/2011 04:32 PM, H.J. Lu wrote:
>> On Wed, May 25, 2011 at 8:27 AM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Wed, May 25, 2011 at 5:20 PM, Michael Matz <matz@suse.de> wrote:
>>>> Hi,
>>>>
>>>> On Wed, 25 May 2011, Richard Guenther wrote:
>>>>
>>>>>>> asm volatile ("" : : : "memory") in fact will work as a full memory
>>>>>>> barrier
>>>>>>
>>>>>> How?  You surely need MFENCE or somesuch, unless all you care about is
>>>>>> a compiler barrier.  That's what I think needs to be clarified.
>>>>>
>>>>> Well, yes, I'm talking about the compiler memory barrier.
>>>>
>>>> Something that we conventionally call "optimization barrier" :)  memory
>>>> barrier has a fixed meaning which we shouldn't use in this case, it's
>>>> confusing.
>>>
>>> Sure ;)
>>>
>>> And to keep the info in a suitable thread what I'd like to improve here
>>> is to make us disambiguate memory loads/stores against asms that
>>> have no memory outputs/inputs.
>>>
>>
>> Please let me know how I should improve the document,
>
> "Compiler memory barrier" seems to be well-understood.  I suggest
>
> +Generates the @code{pause} machine instruction with a compiler memory barrier.
>
> It's clear enough.
>
> Andrew.
>

I checked in this.

Thanks.


-- 
H.J.
---
Index: doc/extend.texi
===================================================================
--- doc/extend.texi	(revision 174216)
+++ doc/extend.texi	(working copy)
@@ -8699,7 +8699,8 @@ The following built-in function is alway

 @table @code
 @item void __builtin_ia32_pause (void)
-Generates the @code{pause} machine instruction with full memory barrier.
+Generates the @code{pause} machine instruction with a compiler memory
+barrier.
 @end table

 The following floating point built-in functions are made available in the
Index: ChangeLog
===================================================================
--- ChangeLog	(revision 174216)
+++ ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2011-05-25  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* doc/extend.texi (X86 Built-in Functions): Update pause
+	intrinsic.
+
 2011-05-25  Bernd Schmidt  <bernds@codesourcery.com>

 	PR bootstrap/49160

Re: PATCH: Add pause intrinsic

[Andrew Pinski]

On Wed, May 25, 2011 at 10:19 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> --
> H.J.
> ---
> Index: doc/extend.texi
> ===================================================================
> --- doc/extend.texi     (revision 174216)
> +++ doc/extend.texi     (working copy)
> @@ -8699,7 +8699,8 @@ The following built-in function is alway
>
>  @table @code
>  @item void __builtin_ia32_pause (void)
> -Generates the @code{pause} machine instruction with full memory barrier.
> +Generates the @code{pause} machine instruction with a compiler memory
> +barrier.

What is the pause machine instruction do?  How is it different from a
normal nop?

Also pause to me means it waits for input or an interrupt.

Thanks,
Andrew Pinski

Re: PATCH: Add pause intrinsic

[Andrew Haley]

On 05/25/2011 06:26 PM, Andrew Pinski wrote:
> On Wed, May 25, 2011 at 10:19 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> --
>> H.J.
>> ---
>> Index: doc/extend.texi
>> ===================================================================
>> --- doc/extend.texi     (revision 174216)
>> +++ doc/extend.texi     (working copy)
>> @@ -8699,7 +8699,8 @@ The following built-in function is alway
>>
>>  @table @code
>>  @item void __builtin_ia32_pause (void)
>> -Generates the @code{pause} machine instruction with full memory barrier.
>> +Generates the @code{pause} machine instruction with a compiler memory
>> +barrier.
> 
> What is the pause machine instruction do?

That's documented by Intel in the architecture manual.  Surely
we don't have to explain it all.

Andrew.


PAUSE—Spin Loop Hint

Improves the performance of spin-wait loops. When executing a “spin-wait loop,” a
Pentium 4 or Intel Xeon processor suffers a severe performance penalty when exiting
the loop because it detects a possible memory order violation. The PAUSE instruction
provides a hint to the processor that the code sequence is a spin-wait loop. The
processor uses this hint to avoid the memory order violation in most situations,
which greatly improves processor performance. For this reason, it is recommended
that a PAUSE instruction be placed in all spin-wait loops.

An additional function of the PAUSE instruction is to reduce the power consumed by
a Pentium 4 processor while executing a spin loop. The Pentium 4 processor can
execute a spin-wait loop extremely quickly, causing the processor to consume a lot of
power while it waits for the resource it is spinning on to become available. Inserting
a pause instruction in a spin-wait loop greatly reduces the processor’s power
consumption.

This instruction was introduced in the Pentium 4 processors, but is backward compat-
ible with all IA-32 processors. In earlier IA-32 processors, the PAUSE instruction
operates like a NOP instruction. The Pentium 4 and Intel Xeon processors implement
the PAUSE instruction as a pre-defined delay. The delay is finite and can be zero for
some processors. This instruction does not change the architectural state of the
processor (that is, it performs essentially a delaying no-op operation).
This instruction’s operation is the same in non-64-bit modes and 64-bit mode.

Re: PATCH: Add pause intrinsic

[Richard Guenther]

On Wed, May 25, 2011 at 7:19 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, May 25, 2011 at 9:43 AM, Andrew Haley <aph@redhat.com> wrote:
>> On 05/25/2011 04:32 PM, H.J. Lu wrote:
>>> On Wed, May 25, 2011 at 8:27 AM, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> On Wed, May 25, 2011 at 5:20 PM, Michael Matz <matz@suse.de> wrote:
>>>>> Hi,
>>>>>
>>>>> On Wed, 25 May 2011, Richard Guenther wrote:
>>>>>
>>>>>>>> asm volatile ("" : : : "memory") in fact will work as a full memory
>>>>>>>> barrier
>>>>>>>
>>>>>>> How?  You surely need MFENCE or somesuch, unless all you care about is
>>>>>>> a compiler barrier.  That's what I think needs to be clarified.
>>>>>>
>>>>>> Well, yes, I'm talking about the compiler memory barrier.
>>>>>
>>>>> Something that we conventionally call "optimization barrier" :)  memory
>>>>> barrier has a fixed meaning which we shouldn't use in this case, it's
>>>>> confusing.
>>>>
>>>> Sure ;)
>>>>
>>>> And to keep the info in a suitable thread what I'd like to improve here
>>>> is to make us disambiguate memory loads/stores against asms that
>>>> have no memory outputs/inputs.
>>>>
>>>
>>> Please let me know how I should improve the document,
>>
>> "Compiler memory barrier" seems to be well-understood.  I suggest
>>
>> +Generates the @code{pause} machine instruction with a compiler memory barrier.
>>
>> It's clear enough.
>>
>> Andrew.
>>
>
> I checked in this.
>
> Thanks.
>
>
> --
> H.J.
> ---
> Index: doc/extend.texi
> ===================================================================
> --- doc/extend.texi     (revision 174216)
> +++ doc/extend.texi     (working copy)
> @@ -8699,7 +8699,8 @@ The following built-in function is alway
>
>  @table @code
>  @item void __builtin_ia32_pause (void)
> -Generates the @code{pause} machine instruction with full memory barrier.
> +Generates the @code{pause} machine instruction with a compiler memory
> +barrier.
>  @end table

This isn't true.  It is _not_ a compiler memory barrier.

Richard.

Re: PATCH: Add pause intrinsic

[Andrew Haley]

On 05/26/2011 10:34 AM, Richard Guenther wrote:

>> Index: doc/extend.texi
>> ===================================================================
>> --- doc/extend.texi     (revision 174216)
>> +++ doc/extend.texi     (working copy)
>> @@ -8699,7 +8699,8 @@ The following built-in function is alway
>>
>>  @table @code
>>  @item void __builtin_ia32_pause (void)
>> -Generates the @code{pause} machine instruction with full memory barrier.
>> +Generates the @code{pause} machine instruction with a compiler memory
>> +barrier.
>>  @end table
> 
> This isn't true.  It is _not_ a compiler memory barrier.

Please elucidate.  Please suggest alternative wording.

Andrew.

Re: PATCH: Add pause intrinsic

[Richard Guenther]

On Thu, May 26, 2011 at 3:30 PM, Andrew Haley <aph@redhat.com> wrote:
> On 05/26/2011 10:34 AM, Richard Guenther wrote:
>
>>> Index: doc/extend.texi
>>> ===================================================================
>>> --- doc/extend.texi     (revision 174216)
>>> +++ doc/extend.texi     (working copy)
>>> @@ -8699,7 +8699,8 @@ The following built-in function is alway
>>>
>>>  @table @code
>>>  @item void __builtin_ia32_pause (void)
>>> -Generates the @code{pause} machine instruction with full memory barrier.
>>> +Generates the @code{pause} machine instruction with a compiler memory
>>> +barrier.
>>>  @end table
>>
>> This isn't true.  It is _not_ a compiler memory barrier.
>
> Please elucidate.  Please suggest alternative wording.

+Generates the @code{pause} machine instruction.

Richard.

> Andrew.
>

Re: PATCH: Add pause intrinsic

[Andrew Haley]

On 05/26/2011 02:51 PM, Richard Guenther wrote:
> On Thu, May 26, 2011 at 3:30 PM, Andrew Haley <aph@redhat.com> wrote:
>> On 05/26/2011 10:34 AM, Richard Guenther wrote:
>>
>>>> Index: doc/extend.texi
>>>> ===================================================================
>>>> --- doc/extend.texi     (revision 174216)
>>>> +++ doc/extend.texi     (working copy)
>>>> @@ -8699,7 +8699,8 @@ The following built-in function is alway
>>>>
>>>>  @table @code
>>>>  @item void __builtin_ia32_pause (void)
>>>> -Generates the @code{pause} machine instruction with full memory barrier.
>>>> +Generates the @code{pause} machine instruction with a compiler memory
>>>> +barrier.
>>>>  @end table
>>>
>>> This isn't true.  It is _not_ a compiler memory barrier.
>>
>> Please elucidate.  Please suggest alternative wording.
> 
> +Generates the @code{pause} machine instruction.

But that's missing the fact that it generates a compiler memory barrier,
which is important.  And if you think it's not a compiler memory barrier,
please explain

a.  Why it's not a compiler memory barrier,
b.  What you'd call it.

Andrew.

Re: PATCH: Add pause intrinsic

[Richard Guenther]

On Thu, May 26, 2011 at 3:53 PM, Andrew Haley <aph@redhat.com> wrote:
> On 05/26/2011 02:51 PM, Richard Guenther wrote:
>> On Thu, May 26, 2011 at 3:30 PM, Andrew Haley <aph@redhat.com> wrote:
>>> On 05/26/2011 10:34 AM, Richard Guenther wrote:
>>>
>>>>> Index: doc/extend.texi
>>>>> ===================================================================
>>>>> --- doc/extend.texi     (revision 174216)
>>>>> +++ doc/extend.texi     (working copy)
>>>>> @@ -8699,7 +8699,8 @@ The following built-in function is alway
>>>>>
>>>>>  @table @code
>>>>>  @item void __builtin_ia32_pause (void)
>>>>> -Generates the @code{pause} machine instruction with full memory barrier.
>>>>> +Generates the @code{pause} machine instruction with a compiler memory
>>>>> +barrier.
>>>>>  @end table
>>>>
>>>> This isn't true.  It is _not_ a compiler memory barrier.
>>>
>>> Please elucidate.  Please suggest alternative wording.
>>
>> +Generates the @code{pause} machine instruction.
>
> But that's missing the fact that it generates a compiler memory barrier,
> which is important.  And if you think it's not a compiler memory barrier,
> please explain
>
> a.  Why it's not a compiler memory barrier,

It is not a compiler memory barrier because it is a builtin function call
which is never assumed to be a barrier for local automatic storage
that does not have its address taken.

> b.  What you'd call it.

Not a compiler memory barrier ;)

To make it a compiler memory barrier you have to "expand" the
builtin already in the frontend and present the middle-end with
__asm__ ("...." : : : "memory").  That will serve as a compiler
memory barrier also covering local non-address taken storage
(global and practically most of address-taken local storage
is covered by a builtin function call already).

Richard.

>
> Andrew.
>

Re: PATCH: Add pause intrinsic

[Jakub Jelinek]

On Thu, May 26, 2011 at 04:29:50PM +0200, Richard Guenther wrote:
> To make it a compiler memory barrier you have to "expand" the
> builtin already in the frontend and present the middle-end with
> __asm__ ("...." : : : "memory").  That will serve as a compiler
> memory barrier also covering local non-address taken storage
> (global and practically most of address-taken local storage
> is covered by a builtin function call already).

But then, what is the point of the builtin when
__asm__ __volatile__ ("rep; nop" : : : "memory");
does all of that already and has been supported for years...

	Jakub

Re: PATCH: Add pause intrinsic

[Richard Guenther]

On Thu, May 26, 2011 at 4:34 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, May 26, 2011 at 04:29:50PM +0200, Richard Guenther wrote:
>> To make it a compiler memory barrier you have to "expand" the
>> builtin already in the frontend and present the middle-end with
>> __asm__ ("...." : : : "memory").  That will serve as a compiler
>> memory barrier also covering local non-address taken storage
>> (global and practically most of address-taken local storage
>> is covered by a builtin function call already).
>
> But then, what is the point of the builtin when
> __asm__ __volatile__ ("rep; nop" : : : "memory");
> does all of that already and has been supported for years...

Good question ;)

Richard.

Re: PATCH: Add pause intrinsic

[Andrew Haley]

On 05/26/2011 03:29 PM, Richard Guenther wrote:
> On Thu, May 26, 2011 at 3:53 PM, Andrew Haley <aph@redhat.com> wrote:
>> On 05/26/2011 02:51 PM, Richard Guenther wrote:
>>> On Thu, May 26, 2011 at 3:30 PM, Andrew Haley <aph@redhat.com> wrote:
>>>> On 05/26/2011 10:34 AM, Richard Guenther wrote:
>>>>
>>>>>> Index: doc/extend.texi
>>>>>> ===================================================================
>>>>>> --- doc/extend.texi     (revision 174216)
>>>>>> +++ doc/extend.texi     (working copy)
>>>>>> @@ -8699,7 +8699,8 @@ The following built-in function is alway
>>>>>>
>>>>>>  @table @code
>>>>>>  @item void __builtin_ia32_pause (void)
>>>>>> -Generates the @code{pause} machine instruction with full memory barrier.
>>>>>> +Generates the @code{pause} machine instruction with a compiler memory
>>>>>> +barrier.
>>>>>>  @end table
>>>>>
>>>>> This isn't true.  It is _not_ a compiler memory barrier.
>>>>
>>>> Please elucidate.  Please suggest alternative wording.
>>>
>>> +Generates the @code{pause} machine instruction.
>>
>> But that's missing the fact that it generates a compiler memory barrier,
>> which is important.  And if you think it's not a compiler memory barrier,
>> please explain
>>
>> a.  Why it's not a compiler memory barrier,
> 
> It is not a compiler memory barrier because it is a builtin function call
> which is never assumed to be a barrier for local automatic storage
> that does not have its address taken.

OK.  How would you tell the difference between the kind of barrier
that it is and a real compiler memory barrier?  If an auto does not
have its address taken, it isn't visible anyway.

>> b.  What you'd call it.
> 
> Not a compiler memory barrier ;)

I don't want to know what not to call it, though.

> To make it a compiler memory barrier you have to "expand" the
> builtin already in the frontend and present the middle-end with
> __asm__ ("...." : : : "memory").  That will serve as a compiler
> memory barrier also covering local non-address taken storage
> (global and practically most of address-taken local storage
> is covered by a builtin function call already).

Well, the fact that it's also a memory clobber has to be documented
somehow.  If the present documentation is to be changed, it should
not be changed by deleting a vital piece of information.

Andrew.

Re: PATCH: Add pause intrinsic

[Michael Matz]

Hi,

On Thu, 26 May 2011, Andrew Haley wrote:

> >>> +Generates the @code{pause} machine instruction.
> >>
> >> But that's missing the fact that it generates a compiler memory 
> >> barrier, which is important.  And if you think it's not a compiler 
> >> memory barrier, please explain
> >>
> >> a.  Why it's not a compiler memory barrier,
> > 
> > It is not a compiler memory barrier because it is a builtin function call
> > which is never assumed to be a barrier for local automatic storage
> > that does not have its address taken.
> 
> OK.  How would you tell the difference between the kind of barrier
> that it is and a real compiler memory barrier?

First we have to determine if this builtin really does what its users 
intend to use it for.  I believe they _do_ want to use it also with 
regards to auto variables (it includes also address-takens whose address 
doesn't escape).  A normal builtin call is not a barrier for operations on 
such entities, hence it might very well be that the implementation of HJ 
actually doesn't what he wanted.

I don't have a good word for what functions calls are in their barrierness 
part of pre/post conditions.  "global memory movement barrier" perhaps, 
with an appropriate definition of global memory (which funnily include 
address-taken escaped local storage, ugh).

> > To make it a compiler memory barrier you have to "expand" the
> > builtin already in the frontend and present the middle-end with
> > __asm__ ("...." : : : "memory").  That will serve as a compiler
> > memory barrier also covering local non-address taken storage
> > (global and practically most of address-taken local storage
> > is covered by a builtin function call already).
> 
> Well, the fact that it's also a memory clobber has to be documented
> somehow.  If the present documentation is to be changed, it should
> not be changed by deleting a vital piece of information.

It's not only about the docu.  As implemented right now it's neither an 
optimization barrier nor a memory clobber.


Ciao,
Michael.

Re: PATCH: Add pause intrinsic

[Andi Kleen]

Richard Guenther <richard.guenther@gmail.com> writes:
>
> To make it a compiler memory barrier you have to "expand" the
> builtin already in the frontend and present the middle-end with
> __asm__ ("...." : : : "memory").  That will serve as a compiler

Those are the intended semantics (at least those I asked
for :-). For all practical purposes the same as
asm volatile("pause" ::: "memory")

HJ? Can it be expanded earlier?

As for why having a builtin: one reason would be portability.
Various other architectures have a similar instruction
(e.g. PPC). They could be added later to this as a next 
step.

Then it also seems cleaner to me to cover the instruction
set with builtins like the others.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only

Re: PATCH: Add pause intrinsic

[Jakub Jelinek]

On Thu, May 26, 2011 at 09:10:32AM -0700, Andi Kleen wrote:
> Richard Guenther <richard.guenther@gmail.com> writes:
> As for why having a builtin: one reason would be portability.

You mean portability to other compilers (I think reasonable amount
of them support gcc-ish inline asm), or to other architectures?
__builtin_ia32_pause () doesn't look like a builtin you would
want to use on PPC.

> Then it also seems cleaner to me to cover the instruction
> set with builtins like the others.

No idea why in this case.  Builtins have the advantage that they
can be better scheduled, but in this case you don't want to move
it around.

	Jakub

Re: PATCH: Add pause intrinsic

[Andi Kleen]

On Thu, May 26, 2011 at 06:46:39PM +0200, Jakub Jelinek wrote:
> On Thu, May 26, 2011 at 09:10:32AM -0700, Andi Kleen wrote:
> > Richard Guenther <richard.guenther@gmail.com> writes:
> > As for why having a builtin: one reason would be portability.
> 
> You mean portability to other compilers (I think reasonable amount
> of them support gcc-ish inline asm), or to other architectures?

Both.

> __builtin_ia32_pause () doesn't look like a builtin you would
> want to use on PPC.

That's true, it should probably have a different name.

__builtin_pause()? 

The Linux kernel calls it cpu_relax() on all architectures.
The following architectures implement it: ia64, powerpc, x86
On others it just acts like a barrier.

I suppose most CPUs that implement SMT will have some equivalent.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

Re: PATCH: Add pause intrinsic

[Paul Koning]

On May 26, 2011, at 1:37 PM, Andi Kleen wrote:

> On Thu, May 26, 2011 at 06:46:39PM +0200, Jakub Jelinek wrote:
>> On Thu, May 26, 2011 at 09:10:32AM -0700, Andi Kleen wrote:
>>> Richard Guenther <richard.guenther@gmail.com> writes:
>>> As for why having a builtin: one reason would be portability.
>> 
>> You mean portability to other compilers (I think reasonable amount
>> of them support gcc-ish inline asm), or to other architectures?
> 
> Both.
> 
>> __builtin_ia32_pause () doesn't look like a builtin you would
>> want to use on PPC.
> 
> That's true, it should probably have a different name.
> 
> __builtin_pause()? 
> 
> The Linux kernel calls it cpu_relax() on all architectures.
> The following architectures implement it: ia64, powerpc, x86
> On others it just acts like a barrier.

Relax?  Weird.  "Pause" is just as weird.  It might be an ia32 instruction, so as an ia32 builtin it is a reasonable name  But if you want a generic builtin, you need a name that actually has some plausible connection with what it does, and neither "pause" nor "relax" do that.

	paul

Re: PATCH: Add pause intrinsic

[Andi Kleen]

> Relax?  Weird.  "Pause" is just as weird.  It might be an ia32 instruction, so as an ia32 builtin it is a reasonable name  But if you want a generic builtin, you need a name that actually has some plausible connection with what it does, and neither "pause" nor "relax" do that.

It's a short pause for the CPU. Both names fit quite well.

-Andi

Re: PATCH: Add pause intrinsic

[Basile Starynkevitch]

On Thu, 26 May 2011 13:48:13 -0400
Paul Koning <paul_koning@dell.com> wrote:

> Relax?  Weird.  "Pause" is just as weird.  It might be an ia32 instruction, 
> so as an ia32 builtin it is a reasonable name  But if you want a generic 
> builtin, you need a name that actually has some plausible connection with 
> what it does, and neither "pause" nor "relax" do that.

I still think that having a builtin which do a "compiler flush" that is
which spill all registers to memory is useful, eg a
builtin_compiler_flush()

And I even think there is another reason to use it. If you are
debugging a program compiled with -O2 -g, and if you know where there
could be a bug or a fault, temporarily adding a call to that
builtin_compiler_flush () would probably help the gdb debugger a lot.

Regards.

-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

Re: PATCH: Add pause intrinsic

[Andrew Haley]

On 05/26/2011 08:37 PM, Basile Starynkevitch wrote:
> On Thu, 26 May 2011 13:48:13 -0400
> Paul Koning <paul_koning@dell.com> wrote:
> 
>> Relax?  Weird.  "Pause" is just as weird.  It might be an ia32 instruction, 
>> so as an ia32 builtin it is a reasonable name  But if you want a generic 
>> builtin, you need a name that actually has some plausible connection with 
>> what it does, and neither "pause" nor "relax" do that.
> 
> I still think that having a builtin which do a "compiler flush" that is
> which spill all registers to memory is useful, eg a
> builtin_compiler_flush()

I don't see how it can do that without causing reload failures.  You'd
have to be very careful somehow to identify user variables.

Andrew.

Re: PATCH: Add pause intrinsic

[Basile Starynkevitch]

On Wed, 25 May 2011 11:26:51 +0100
Andrew Haley <aph@redhat.com> wrote:

> On 05/24/2011 07:28 PM, H.J. Lu wrote:
> 
> > This patch implements pause intrinsic suggested by Andi.  OK
> > for trunk?
> 
> What does "full memory barrier" here mean?
> 
> +@table @code
> +@item void __builtin_ia32_pause (void)
> +Generates the @code{pause} machine instruction with full memory barrier.
> +@end table
> 
> There a memory clobber, but no barrier instruction AFAICS.  The
> doc needs to explain it a bit better.

Perhaps the doc might explain why is it necessary to have a builtin for
two independent roles: first, the full compiler memory barrier (which
probably means to spill all the registers on the stack - definitely a
task for a compiler); second, to "pause" the processor (which might
also mean to flush or invalidate some data caches). In particular, I
would naively imagine that we might have a more generic builtin for the
compiler memory barrier (which probably could be independent of the
particular ia32 target), and in that case which can't we just implement
the pause ia32 builtin as builtin_compiler_barrier(); asm ("pause")?

I find the above documentation too short and (being a non native
English speaker) I would prefer it to be much longer. I am not able to
suggest better phrasing (because I still did not entirely understood
what that builtin_ia32_pause is useful or needed).

And if there was a builtin_compiler_barrier () I would believe it can
have a lot of other uses. Any generated C code which wants some
introspection or some garbage collection write barrier might want it
too! [perhaps even I might find later such thing useful in C code
generated by MELT]

Regards.

-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

Re: PATCH: Add pause intrinsic

[H.J. Lu]

On Wed, May 25, 2011 at 12:17 PM, Basile Starynkevitch
<basile@starynkevitch.net> wrote:
> On Wed, 25 May 2011 11:26:51 +0100
> Andrew Haley <aph@redhat.com> wrote:
>
>> On 05/24/2011 07:28 PM, H.J. Lu wrote:
>>
>> > This patch implements pause intrinsic suggested by Andi.  OK
>> > for trunk?
>>
>> What does "full memory barrier" here mean?
>>
>> +@table @code
>> +@item void __builtin_ia32_pause (void)
>> +Generates the @code{pause} machine instruction with full memory barrier.
>> +@end table
>>
>> There a memory clobber, but no barrier instruction AFAICS.  The
>> doc needs to explain it a bit better.
>
> Perhaps the doc might explain why is it necessary to have a builtin for
> two independent roles: first, the full compiler memory barrier (which
> probably means to spill all the registers on the stack - definitely a
> task for a compiler); second, to "pause" the processor (which might
> also mean to flush or invalidate some data caches). In particular, I
> would naively imagine that we might have a more generic builtin for the
> compiler memory barrier (which probably could be independent of the
> particular ia32 target), and in that case which can't we just implement
> the pause ia32 builtin as builtin_compiler_barrier(); asm ("pause")?
>

We may need

 builtin_compiler_barrier();
 asm ("pause");
builtin_compiler_barrier();



-- 
H.J.

Re: PATCH: Add pause intrinsic

[Basile Starynkevitch]

On Wed, 25 May 2011 12:31:17 -0700
"H.J. Lu" <hjl.tools@gmail.com> wrote:

> On Wed, May 25, 2011 at 12:17 PM, Basile Starynkevitch
> <basile@starynkevitch.net> wrote:
> > Perhaps the doc might explain why is it necessary to have a builtin for
> > two independent roles: first, the full compiler memory barrier (which
> > probably means to spill all the registers on the stack - definitely a
> > task for a compiler); second, to "pause" the processor (which might
> > also mean to flush or invalidate some data caches). In particular, I
> > would naively imagine that we might have a more generic builtin for the
> > compiler memory barrier (which probably could be independent of the
> > particular ia32 target), and in that case which can't we just implement
> > the pause ia32 builtin as builtin_compiler_barrier(); asm ("pause")?
> >
> 
> We may need
> 
>  builtin_compiler_barrier();
>  asm ("pause");
> builtin_compiler_barrier();

I don't understand why the second builtin_compiler_barrier() after the
asm ("pause") would be needed? Could you please explain why should we
need it? My feeling was that after the first builtin_compiler_barrier
() and hence after the asm ("pause") no register would contain valid
data, and the compiler would have to reload from memory everything. So
why do you think the second is needed???

Or perhaps I misunderstood completely all the issues!


-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

Re: PATCH: Add pause intrinsic

[Andrew Haley]

On 25/05/11 20:17, Basile Starynkevitch wrote:
> On Wed, 25 May 2011 11:26:51 +0100
> Andrew Haley <aph@redhat.com> wrote:
> 
>> On 05/24/2011 07:28 PM, H.J. Lu wrote:
>>
>>> This patch implements pause intrinsic suggested by Andi.  OK
>>> for trunk?
>>
>> What does "full memory barrier" here mean?
>>
>> +@table @code
>> +@item void __builtin_ia32_pause (void)
>> +Generates the @code{pause} machine instruction with full memory barrier.
>> +@end table
>>
>> There a memory clobber, but no barrier instruction AFAICS.  The
>> doc needs to explain it a bit better.
> 
> Perhaps the doc might explain why is it necessary to have a builtin for
> two independent roles: first, the full compiler memory barrier (which
> probably means to spill all the registers on the stack

No.  It clobbers all of _memory_, which doesn't involve auto variables.
I don't understand why __builtin_ia32_pause needs to have a barrier either.

> - definitely a
> task for a compiler); second, to "pause" the processor (which might
> also mean to flush or invalidate some data caches).

No, it doesn't.  I posted exactly what pause does in an earlier mail.

Andrew.

Re: PATCH: Add pause intrinsic

[Andi Kleen]

Basile Starynkevitch <basile@starynkevitch.net> writes:
>
> Perhaps the doc might explain why is it necessary to have a builtin for
> two independent roles: first, the full compiler memory barrier (which
> probably means to spill all the registers on the stack - definitely a
> task for a compiler); second, to "pause" the processor (which might
> also mean to flush or invalidate some data caches). In particular, I
> would naively imagine that we might have a more generic builtin for the
> compiler memory barrier (which probably could be independent of the
> particular ia32 target), and in that case which can't we just implement
> the pause ia32 builtin as builtin_compiler_barrier(); asm ("pause")?

Because a pause() which can be freely moved around is pretty much
useless. Think about it. And a memory barrier is the standard
way to prevent moving around in relation to other code.

You would always need to combine those too.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: PATCH: Add pause intrinsic

[Basile Starynkevitch]

On Thu, 26 May 2011 09:12:06 -0700
Andi Kleen <andi@firstfloor.org> wrote:

> Basile Starynkevitch <basile@starynkevitch.net> writes:
> >
> > Perhaps the doc might explain why is it necessary to have a builtin for
> > two independent roles: first, the full compiler memory barrier (which
> > probably means to spill all the registers on the stack - definitely a
> > task for a compiler); second, to "pause" the processor (which might
> > also mean to flush or invalidate some data caches). In particular, I
> > would naively imagine that we might have a more generic builtin for the
> > compiler memory barrier (which probably could be independent of the
> > particular ia32 target), and in that case which can't we just implement
> > the pause ia32 builtin as builtin_compiler_barrier(); asm ("pause")?
> 
> Because a pause() which can be freely moved around is pretty much
> useless. Think about it. And a memory barrier is the standard
> way to prevent moving around in relation to other code.

I thought that there is a syntax, perhaps volatile asm("pause"), which
forbids the compiler to move an asm statement? Or is the "memory" thing? 
(I am not very used to GCC asm).

Still, is there a way to ask GCC to spill all data from registers to
memory, and to require GCC to consider all registers crappy and to
reload them when needed? I would imagine that such a feature is useful,
and most importantly, that it should not be specific to ia32!

Regards.
-- 
Basile STARYNKEVITCH         http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***

Re: PATCH: Add pause intrinsic

[Uros Bizjak]

On Tue, May 24, 2011 at 8:28 PM, H.J. Lu <hjl.tools@gmail.com> wrote:

> This patch implements pause intrinsic suggested by Andi.  OK
> for trunk?

> gcc/
>
> 2011-05-24  H.J. Lu  <hongjiu.lu@intel.com>
>
>        * config/i386/i386.c (ix86_builtins): Add IX86_BUILTIN_PAUSE.
>        (bdesc_special_args): Add pause intrinsic.
>
>        * config/i386/i386.md (UNSPEC_PAUSE): New.
>        (pause): Likewise.
>        (*pause): Likewise.
>        * config/i386/ia32intrin.h (__pause): Likewise.
>
>        * doc/extend.texi (X86 Built-in Functions): Add documentation for
>        pause intrinsic.
>
> gcc/testsuite/
>
> 2011-05-24  H.J. Lu  <hongjiu.lu@intel.com>
>
>         * gcc.target/i386/pause-1.c: New.

OK.

Thanks,
Uros.