On 14/08/2024 06:31, Srirama Kucherlapati wrote:
Hi Heikki & Team,
I tried to look at the assembly code changes with our team, in the below
file.
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index 29ac6cdcd9..69582f4ae7 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
static __inline__ int
tas(volatile slock_t *lock)
@@ -424,17 +430,15 @@ tas(volatile slock_t *lock)
__asm__ __volatile__(
" lwarx %0,0,%3,1 \n"
" cmpwi %0,0 \n"
" bne $+16 \n" /* branch to li
%1,1 */
" addi %0,%0,1 \n"
" stwcx. %0,0,%3 \n"
" beq $+12 \n" /* branch to
lwsync */
" li %1,1 \n"
" b $+12 \n" /* branch to end
of asm sequence */
" lwsync \n"
" li %1,0 \n"
: "=&b"(_t), "=r"(_res), "+m"(*lock)
: "r"(lock)
: "memory", "cc");
For the changes in the above file, this code is very specific to power
architecture we need to use the IBM Power specific asm code only, rather
than using the GNU assembler. Also, all these asm specific code is under
the macro __ppc__, which should not impact any other platforms. I see
there is a GCC specific implementation (under this macro #if
defined(HAVE_GCC__SYNC_INT32_TAS)) in the same file as well.
I'm sorry, I don't understand what you're saying here. Do you mean that
we don't need to do anything here, and the code we have in s_lock.h in
'master' now will work fine on AIX? Or do we need to (re-)do some
changes to support AIX again? If we only support GCC, can we use the
__sync_lock_test_and_set() builtin instead?
If any changes are required, please include them in the patch. That'll
make it clear what exactly you're proposing.
+#define TAS(lock) _check_lock((slock_t *) (lock),
0, 1)
+#define S_UNLOCK(lock) _clear_lock((slock_t *) (lock), 0)
The above changes are specific to AIX kernel and it operates on fixed
kernel memory. This is more like a compare_and_swap functionality with
sync capability. For all the assemble code I think it would be better to
use the IBM Power specific asm code to gain additional performance.
You mean we don't need the above? Ok, good.
I was trying to understand here wrt to both the assemble changes if you
are looking for anything specific to the architecture.
I don't know. You tell me what makes most sense on AIX / powerpc.
Attached is the patch for the previous comments, kindly please let me
know your comments.
Is this all that's needed to resurrect AIX support?
--
Heikki Linnakangas
Neon (https://neon.tech)