[kaffe] MIPS Port Update
Kevin D. Kissell
kevink@mips.com
Wed, 17 Jul 2002 12:07:18 +0200
[Note to mailing list - this thread was continued off-line,
but I want to make sure that Dylan and the other MIPS
hackers see it]
1) The proposed code appears to do a compare-and-exchange,
but it does not do an *atomic* compare and exchange.
Check out the definition of SC. If a context switch
occurs between the LL and the SC (such that the
value of the memory location might have been
modified by another process), the SC will fail
and one must repeat the operation from the
beginning.
2) The proposed code does not deal with the fact
that the function is specified to return 1 on
success, and zero if no exchange occurred.
2) One should really try to avoid assigning
fixed registers in an inline macro if at
all possible. It plays hell with the compiler's
register assignment scheme. If at all possible,
you should simply declare "r" variables and
let the compiler decide what to do in a given
instantiation. And I really don't think you need
four registers assgned!
I'm not going to have time to try it out today, but
I think the code needs to look a bit more like:
.set noreorder
1:
ll %0,%2
bne %0,%3,2f
move %0,$0 /* branch delay slot */
move %0,%4 /* copy "N" value */
sc %0,%1
beqz %0,1b /* if not atomic, retry */
nop
2:
: "=&r" (ret), "=m" (*(A))
: "m" (*(A)), "r" (O), "r" (N)
: "memory");
Note that I'm using the return value
as a temporary, and taking advantage
of the fact that the SC operation is
defined to replace the stored value
with a "1" if the operation is successful.
This should execute just as quickly as
the MIPS IV version in the distribution.
The "sync" instruction in the MIPS IV
version was redundant: LL and SC
implicitly perform syncs.
Again, I don't have time to test this today,
and I don't do gcc inline assembler macro
templates often enough to know the syntax
by heart, but something very close to it will
work
Regards,
Kevin K.
----- Original Message -----
> Attached is the patch I have been working on for
> the COMPARE_AND_EXCHANGE macro. This macro is the
> one used by the pocketlinux people in their version
> of kaffe. It seems simpler than the old kaffe version.
>
> I have tested this in some standalone code, but
> not yet tested it in a real version of kaffe.
>
> This patch is against the 1.0.7 tarball.
--------------------------------------------------------------------------------
> diff -Naur kaffe-1.0.7/config/mips/common.h kaffe-1.0.7.modified/config/mips/common.h
> --- kaffe-1.0.7/config/mips/common.h Wed Mar 7 10:38:17 2001
> +++ kaffe-1.0.7.modified/config/mips/common.h Tue Jul 9 04:54:22 2002
> @@ -42,30 +42,35 @@
>
> #if defined(HAVE_MIPSII_INSTRUCTIONS)
> /*
> - * Do an atomic compare and exchange. The address 'A' is checked against
> - * value 'O' and if they match it's exchanged with value 'N'.
> - * We return '1' if the exchange is sucessful, otherwise 0.
> + * Do an atomic compare and exchange. The address 'addr' is checked
> + * against value 'oldVal' and if they match it's exchanged with value
> + * 'newVal'. We return '1' if the exchange is sucessful, otherwise 0.
> + *
> + * Assembler code arguments:
> + * %0 : addr
> + * %1 : oldVal
> + * %2 : newVal
> + * Clobber : r8, r9, r10, r11
> */
> -#define COMPARE_AND_EXCHANGE(A,O,N) \
> -({ \
> - unsigned int tmp, ret; \
> - \
> - asm volatile( \
> - " .set noreorder\n" \
> - " .set mips2\n" \
> - "1: ll %0, %3\n" \
> - " xor %1, %0, %4\n" \
> - " sltiu %1, %1, 1\n" \
> - " movn %0, %5, %1\n" \
> - " sc %0, %2\n" \
> - " beqz %0, 1b\n" \
> - " sync\n" \
> - " .set mips0\n" \
> - " .set reorder\n" \
> - : "=&r" (tmp), "=&r" (ret), "=m" (*(A)) \
> - : "m" (*(A)), "r" (O), "r" (N) \
> - : "memory"); \
> - ret; \
> +#define COMPARE_AND_EXCHANGE(addr, oldVal, newVal) \
> +({ \
> + asm volatile( \
> + " .set noreorder \n" \
> + " .set mips2 \n" \
> + " move $9,%0 \n" \
> + " move $11,%1 \n" \
> + " move $10,%2 \n" \
> + " ll $8,0($9) \n" \
> + " nop \n" \
> + " bne $8,$11,1f \n" \
> + " nop \n" \
> + " sc $10,0($9) \n" \
> + "1: nop \n" \
> + " .set mips0 \n" \
> + " .set reorder \n" \
> + : : "r" (addr), "r" (oldVal), "r" (newVal) \
> + : "$8","$9","$10","$11","memory"); \
> + (*(addr)) == (newVal) ? 1 : 0; \
> })
>
> #else