From owner-cvs-sys  Sat May 24 23:42:51 1997
Return-Path: <owner-cvs-sys>
Received: (from root@localhost)
          by hub.freebsd.org (8.8.5/8.8.5) id XAA11171
          for cvs-sys-outgoing; Sat, 24 May 1997 23:42:51 -0700 (PDT)
Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.2.228.19])
          by hub.freebsd.org (8.8.5/8.8.5) with ESMTP id XAA11136;
          Sat, 24 May 1997 23:41:07 -0700 (PDT)
Received: (from bde@localhost) by godzilla.zeta.org.au (8.8.5/8.6.9) id QAA03068; Sun, 25 May 1997 16:27:27 +1000
Date: Sun, 25 May 1997 16:27:27 +1000
From: Bruce Evans <bde@zeta.org.au>
Message-Id: <199705250627.QAA03068@godzilla.zeta.org.au>
To: cvs-all@FreeBSD.ORG, cvs-committers@FreeBSD.ORG, cvs-sys@FreeBSD.ORG,
        peter@FreeBSD.ORG
Subject: Re: cvs commit: src/sys/i386/isa seagate.c
Sender: owner-cvs-sys@FreeBSD.ORG
X-Loop: FreeBSD.org
Precedence: bulk

>  Modified files:
>    sys/i386/isa         seagate.c 
>  Log:
>  Fix(?) two volatile cast warnings.  The buffer is volatile within the
>  function so as to avoid compiler side effects, but functions calling it
>  shouldn't be affected (gcc shouldn't cache memory scope past a function
>  call)

I hoped that the driver maintainer would fix this.

`len' doesn't need to be volatile.

The buffer is only modified without the compiler's knowledge in
sea_data_input() (unless it is aliased to the output buffer in
sea_data_output()).  This is either due to low quality asm statements
or wrong optimization.  The corresponding problem in insw() is handled
by declaring that the asm statement clobbers all of memory, but this is
stronger than necessary, assuming that the buffer isn't aliased to any
non-volatile variables.

Since the buffer isn't accessed outside of the asm statements in
sea_data_{in,out}put, gcc is guaranteed not to cache anything in it
and the volatile declarations just hide bugs.  sea_data_output()
does essentially the same thing as clobber() in the following
example:

---
static void clobber(int *p)
{
    volatile int *vp;

    vp = p;

    /* Modify *vp, but don't tell anyone. */
    asm volatile("incl %0" : : "m" (*vp));
}

int val;

int main(void)
{
    val = 1;
    clobber(&val);
    return val;
}
---

This exits with status 1 if it is compiled with -O3, since -O3 inlines
clobber() and gcc believes that clobber() doesn't change anything.
It's not true that "gcc shouldn't cache memory scope past a function
call" when there are inline functions or other agressive optimizations.

Fixes:
Put `"memory"' in the clobber list as in insw(), or put `"=m" (*p)' in
the output operands of the asm.  This works by making the aliases too
complicated for gcc to optimize.  (`p' should point to a suitably
sized object and not just to the first element in the buffer case, in
case gcc is smart.  In practice, gcc probably has to assume that p
may point to arbitrary memory, so there are no advantage to not using
the simple "memory" clobber method.)

Don't use `volatile' here.  Driver buffers are only volatile for DMA
devices.  For PIO, the Program should change its buffers explicitly.

Bruce