This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC PATCH, i386]: PR 36793: x86-64 does not get __sync_synchronize right

H.J. Lu wrote:

This patch just wires "memory_barrier" named pattern to mfence insn.

2008-11-22 Uros Bizjak <ubizjak@gmail.com>

  PR target/36793
  * config/i386/sync.md (memory_barrier): New expander.

Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu
{,-m32}.

I will wait a day or two before for possible comments on this patch.

BTW: "lock nop" as suggested in PR for !TARGET_SSE2 case generates invalid
insn exception.
It needs to be a "nop" that modifies memory. Like "lock orb $0,(%esp)"
or "xchgl eax,dummy".
Attached patch implements the suggestion with locked no-operation insn. On
!TARGET_SSE2, we generate:

lock orb $0, -1(%esp)

where memory location gets assigned by assign_386_stack_local (). This way,
we avoid various store forwarding stalls, since this is an exclusive QImode
address. Also, it can point into redzone area, when redzone is available.

(BTW: Patch also cleanups usage of UNSPECV_CMPXCHG*.)

Patch was bootstrapped and regression tested on x86_64-pc-linux-gnu. I will
wait a day or two for possible comments before committing it ti SVN.

Thanks,
Uros.

Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md (revision 142131)
+++ config/i386/i386.md (working copy)
@@ -219,8 +219,7 @@
   (UNSPECV_ALIGN              7)
   (UNSPECV_MONITOR            8)
   (UNSPECV_MWAIT              9)
-   (UNSPECV_CMPXCHG_1          10)
-   (UNSPECV_CMPXCHG_2          11)
+   (UNSPECV_CMPXCHG            10)
   (UNSPECV_XCHG               12)
   (UNSPECV_LOCK               13)
   (UNSPECV_PROLOGUE_USE       14)
Index: config/i386/sync.md
===================================================================
--- config/i386/sync.md (revision 142131)
+++ config/i386/sync.md (working copy)
@@ -31,6 +31,25 @@
 (define_mode_attr doublemodesuffix [(DI "8") (TI "16")])
 (define_mode_attr DCASHMODE [(DI "SI") (TI "DI")])

+(define_expand "memory_barrier"
+ [(set (match_dup 0)
+ (unspec:BLK [(match_dup 0)] UNSPEC_MFENCE))]
+ ""
+{
+ if (!TARGET_SSE2)
+ {
+ /* Emit a locked no-operation that accesses
+ memory when SSE2 is not available. */
+ int slot = virtuals_instantiated ? SLOT_TEMP : SLOT_VIRTUAL;
+ rtx temp = assign_386_stack_local (QImode, slot);
+ emit_insn (gen_sync_iorqi (temp, CONST0_RTX (QImode)));
+ DONE;
+ }
+
+ operands[0] = gen_rtx_MEM (BLKmode, gen_rtx_SCRATCH (Pmode));
+ MEM_VOLATILE_P (operands[0]) = 1;
+})
+

I am not sure how useful that is for 32bit since it will generate a nop for most
machines which do need mfence. I think for 32bit, we should generate

if (__cpuid_edx & bit_SSE2)
  mfence

We can initialize __cpuid_edx startup time. In fact, glibc does call cpuid at
startup time, gcc and glibc may corporate on this

The problem is with initialization of this global variable. We can initialize a local variable on a per-function basis, but I don't know how to initialize a global variable during startup time. glibc can do this with __attribute__((constructor)).

Maybe -in future- we can extend psABI to introduce some kind of global data structure with cpuid info into crt.o, but ATM I don't see other solution than to use locked no-operation for generic i386 targets.

Uros.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]