Re: [BUGS] Compile fails on AIX 6.1 - Mailing list pgsql-hackers

A year ago we had a thread about assembler syntax errors on AIX, but it
died off when the original complainant stopped responding.  I was recently
contacted off-list by Steve Underwood, who was seeing the same symptoms
on AIX 7.1.  After some investigation, we found that the problem is that
IBM's assembler doesn't understand the "local symbol" notation supported
by the GNU assembler ("bne 1f" referencing the next occurrence of "1:").
So s_lock.h's PowerPC assembly code works if you have gcc configured to
use gas as backend, but not if it's configured to use the native AIX
assembler.  Steve says the latter configuration is pretty common.

Curiously, our build goes through just fine if you use the gcc 4.2.0
build available from IBM's website.  But it turns out it's using the
"generic AIX" stanza of s_lock.h, ie _check_lock(), rather than the
lwarx assembly code.  AFAICS this must mean that that gcc build does
not define __ppc__ --- very weird.  But a bit offtopic.

So now that we know what is happening, what do we want to do about it?
AFAICS there are two plausible ways to fix it:

1. Add a configure-time test to see if the assembler supports local
symbols.  If not, don't try to use the lwarx assembly stanza, but let
it fall through to using _check_lock().  This would be simple but
there would presumably be some performance hit.

2. Don't rely on local symbols in the PPC spinlock assembly code.  This
is a bit ugly, because the only way to do that is to hard-code branch
offsets, as in the attached draft patch.  If there were any likelihood
that we'd be changing the PPC spinlock code in future, I would regard
this as unmaintainable ... but really, that code is pretty static.
So I think this is a viable alternative.

Comments?

            regards, tom lane

diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index ef66644..9e709f8 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -447,6 +447,12 @@ typedef unsigned int slock_t;
  * NOTE: per the Enhanced PowerPC Architecture manual, v1.0 dated 7-May-2002,
  * an isync is a sufficient synchronization barrier after a lwarx/stwcx loop.
  * On newer machines, we can use lwsync instead for better performance.
+ *
+ * Ordinarily, we'd code the branches here using GNU-style local symbols, that
+ * is "1f" referencing "1:" and so on.  But some people run gcc on AIX with
+ * IBM's assembler as backend, and IBM's assembler doesn't do local symbols.
+ * So hand-code the branch offsets; fortunately, all PPC instructions are
+ * exactly 4 bytes each, so it's not too hard to count.
  */
 static __inline__ int
 tas(volatile slock_t *lock)
@@ -461,20 +467,18 @@ tas(volatile slock_t *lock)
 "    lwarx   %0,0,%3        \n"
 #endif
 "    cmpwi   %0,0        \n"
-"    bne     1f            \n"
+"    bne     .+16        \n"        /* branch to li %1,1 */
 "    addi    %0,%0,1        \n"
 "    stwcx.  %0,0,%3        \n"
-"    beq     2f             \n"
-"1:    li      %1,1        \n"
-"    b        3f            \n"
-"2:                        \n"
+"    beq     .+12           \n"        /* branch to lwsync/isync */
+"    li      %1,1        \n"
+"    b        .+12        \n"        /* branch to end of asm sequence */
 #ifdef USE_PPC_LWSYNC
 "    lwsync                \n"
 #else
 "    isync                \n"
 #endif
 "    li      %1,0        \n"
-"3:                        \n"

 :    "=&r"(_t), "=r"(_res), "+m"(*lock)
 :    "r"(lock)