From owner-freebsd-hackers  Tue Aug 13 11:46:28 1996
Return-Path: owner-hackers
Received: (from root@localhost)
          by freefall.freebsd.org (8.7.5/8.7.3) id LAA03040
          for hackers-outgoing; Tue, 13 Aug 1996 11:46:28 -0700 (PDT)
Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.2.228.19])
          by freefall.freebsd.org (8.7.5/8.7.3) with SMTP id LAA03032
          for <hackers@freefall.freebsd.org>; Tue, 13 Aug 1996 11:46:21 -0700 (PDT)
Received: (from bde@localhost) by godzilla.zeta.org.au (8.6.12/8.6.9) id EAA15809; Wed, 14 Aug 1996 04:44:15 +1000
Date: Wed, 14 Aug 1996 04:44:15 +1000
From: Bruce Evans <bde@zeta.org.au>
Message-Id: <199608131844.EAA15809@godzilla.zeta.org.au>
To: ajones@ctron.com, hackers@freefall.freebsd.org
Subject: Re: <machine/spl.h> and C++
Sender: owner-hackers@FreeBSD.ORG
X-Loop: FreeBSD.org
Precedence: bulk

>  I'm writing a device driver in C++, and have run into a problem with
>the GENSPL macro in <machine/spl.h>.  g++ is croaking on the statement:

>__asm __volatile("":::"memory");

>with the following error:

>parse error before "::"

>  Without getting into an argument about whether g++ is in error about
>accepting the code as it is, I'm asking whether or not this code
>fragment even needs to exist.

Yes, it does.  It tells gcc that memory may have changed so that gcc is
forced to reload cached variables in code like this:

	extern int glob;
	int s;

	/*
	 * Do an cheap although imperfect test of `glob' to minimize
	 * overheads.
	 */
	if (glob) {
		/*
		 * Probably something to do.  An interrupt may have
		 * cleared `glob' since we tested it before calling
		 * splfoo(), so we have to test it again to be sure
		 * that there is someting to do.
		 */
		s = splfoo();
		if (glob)
			dostuff(glob);
		splx(s);
	}

Without the dummy memory-accessing asm, `glob' isn't changed by
splfoo().  It isn't declared volatile, so the second test of `glob' can
be omitted if it really hasn't changed.  The asm stops this invalid
optimization.  The problem could also be avoided by declaring `glob' as
volatile.  This method isn't used because
1) drivers are generally sloppy about declaring things volatile.
2) declaring things volatile would mainly slow things down.  In the
   above, `glob' is non-volatile iff it is protected by an splfoo(),
   normal accesses to it are protected, so declaring it as volatile
   would be wasteful.

>  Now, I just tried an interesting experiment.  I generated the assembly
>code (using init_main.c) with the asm declaration in there.  It produced

>#APP
>#NO_APP

>where the asm declaration should be.  Then I generated the assembly with
>the asm declaration removed, and the code was the exact same except the
>#APP/#NO_APP code was gone.  gas also says:

I checked all the spl's in last year's kernel for changes
(pessimizations) caused by the asm.  There was no significant change in
most cases.  There were a few small pesimizations (usually for _local_
variables being vulnerable to memory being changed, although it would
take very poorly designed assembler to change a local variable), and no
cases where the asm fixed a bug.  

>  So it seems like the asm statement is a no-op.  Is this a fair
>assumption?  Can I just safely get rid of it?  What are people's
>thoughts about this?

This was fixed almost a 12 months ago in -current and 8 months ago
in -stable.  Change `:::' to ` : : : ' and complain to anyone who
uses insufficient whitespace between tokens.

Bruce