Re: [HACKERS] configure on linux - Mailing list pgsql-hackers

Bruce Momjian wrote:

> Don't break my optimizations.  The locking stuff is in *.h files for a
> reason.  They get called thousands of times, and inlining this code has
> produced a good speedup and they aren't that big.

You misunderstand me.  I didn't suggest removing the S_LOCK() et al
macros.  What I meant was that the actual assembly implementation of
tas() itself might be better off in a separate source file.  As an
example, here is my current version of the locking code for the VAX,
in s_lock.h (bbssi is "branch on bit set and set, interlocked"):

static int tas(slock_t *lock) {
    register ret;

asm("    clrl r0
    bbssi $0,(%1),1f
    incl r0
1:    movl r0,%0"
    : "=r"(ret)    /* return value, in register */
    : "r"(lock)    /* argument, 'lock pointer', in register */
    : "r0");    /* assembly code uses this register */

    return ret;
}

#define    S_LOCK(addr)        do { while (!tas(addr)) ; } while (0)
#define    S_UNLOCK(addr)        (*(addr) = 0)
#define    S_INIT_LOCK(addr)    (*(addr) = 0)

The inlining of lock handling through the macros is fine, and it all
works great with "gcc -O2".  Go to "gcc -O3", however, and GCC will
drop tas() as a function (it's declared static, so it can't be called
from outside the current compilation unit), and inline the given code
wherever tas() was called, which is wherever S_LOCK() is used.  This
can actually break it.  Specifically, testing the above with a source
file that includes that version of tas() and does

    lock = 0;
    retv = tas(&lock);
    printf("%d %d", lock, retv);
    retv = tas(&lock);
    printf(" %d %d\n", lock, retv);

will print "1 1 1 0" when compiled with -O2, but "1 1 1 1" with -O3.
I believe this can be fixed by proper use of "__volatile", but I don't
yet have a complete understanding of this.  In all probability, you
have to use a pathological case such as this to even produce this
behaviour, though.  Still, I would be quite wary of using "-O3" and up
in the presence of static functions with asm() code within them.  (I
just checked, and none of the PostgreSQL ports uses higher than "-O2",
so there's no current danger.  Heck, I've never seen anything except
operating system kernels be compiled at higher than "-O2".)

About the VAX stuff:

> I certainly think we want those changes.  6.3 beta is the place we
> expect to be making platform-specific patches.

Scrappy sounded positive as well, so I'll send them in -- but not
until tomorrow, when I've had a chance to test the latest version of
them with a fresh snapshot.  My aging VAX will be busy installing an
operating system upgrade tonight, but will be free to test tomorrow's
snapshot as soon as it's made available.  Once that looks good, I'll
send in diffs.

-tih
--
Popularity is the hallmark of mediocrity.  --Niles Crane, "Frasier"