Re: monitor+mwait and volatile-ish

2012-09-26 Thread Konstantin Belousov
On Wed, Sep 26, 2012 at 11:14:41AM +0300, Andriy Gapon wrote:
 
 Typical x86 MONITOR+MWAIT is like this (taken from Intel manual):
 
 EAX = Logical Address(Trigger)
 ECX = 0 (*Hints *)
 EDX = 0 (* Hints *)
 IF ( !trigger_store_happened) {
   MONITOR EAX, ECX, EDX
   IF ( !trigger_store_happened ) {
   MWAIT EAX, ECX
   }
 }
 
 In FreeBSD we have this helper function for MONITOR:
 static __inline void
 cpu_monitor(const void *addr, u_long extensions, u_int hints)
 {
 
 __asm __volatile(monitor
 : : a (addr), c (extensions), d (hints));
 }
 
 Now, let's assume that 'Trigger' is a global variable and
 'trigger_store_happened' is a check for a particular value in this variable.
 
 Then, with current state of matters, we must either declare the global 
 variable
 to be volatile or use a volatile cast in the second IF.  Otherwise, a compiler
 is free to assume that the variable doesn't change between the first and the
 second IF and drop the second IF altogether.  And that would make 
 MONITOR+MWAIT
 to miss an event if it happens between the first check and MONITOR.
 
 So what's my point.
 - using volatile variable with cpu_monitor requires DEVOLATILE to silence
 compiler warning about discarding volatile; this is unnecessary code bloat
 - adding volatile cast in the checks is easy to forget and adds code bloat
 
 Possible improvements:
 - make the argument of cpu_monitor be 'const volatile void *', the most
 permissive type; this would also be a hint that variable should be volatile
 - add some magic dust to cpu_monitor that would tell compiler to not cache
   the variable; right now I can only think of the memory constraint, but it
   seems to be too big of a hummer
 
 What do you think about this?

You might claim that the asm writes to *addr by specifying it in the
output constraint. This should fool the compiler into reload *addr after
the monitor execution.


pgpAahBMIChsi.pgp
Description: PGP signature


Re: monitor+mwait and volatile-ish

2012-09-26 Thread Andriy Gapon
on 26/09/2012 12:10 Konstantin Belousov said the following:
 On Wed, Sep 26, 2012 at 11:14:41AM +0300, Andriy Gapon wrote:
[snip]
 So what's my point. - using volatile variable with cpu_monitor requires
 DEVOLATILE to silence compiler warning about discarding volatile; this is
 unnecessary code bloat - adding volatile cast in the checks is easy to
 forget and adds code bloat
 
 Possible improvements: - make the argument of cpu_monitor be 'const
 volatile void *', the most permissive type; this would also be a hint
 that variable should be volatile - add some magic dust to cpu_monitor
 that would tell compiler to not cache the variable; right now I can only
 think of the memory constraint, but it seems to be too big of a hummer
 
 What do you think about this?
 
 You might claim that the asm writes to *addr by specifying it in the output
 constraint. This should fool the compiler into reload *addr after the
 monitor execution.
 

You mean something like:
static __inline void
cpu_monitor(const void *addr, u_long extensions, u_int hints)
{

__asm __volatile(monitor
:  =m (*(char *)addr)
: a (addr), c (extensions), d (hints));
}

This seems to do the job with base gcc, 4.6, 4.7 and clang.
Thank you!
-- 
Andriy Gapon
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org