NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: port-sh3/60773: sh3 __sync_val_compare_and_swap_1 test failures



The following reply was made to PR toolchain/60773; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: Valery Ushakov <uwe%stderr.spb.ru@localhost>,
Cc: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost,
	Nick Hudson <skrll%NetBSD.org@localhost>
Subject: Re: port-sh3/60773: sh3 __sync_val_compare_and_swap_1 test failures
Date: Fri, 25 Sep 2026 15:51:47 +0000

 This is a multi-part message in MIME format.
 --=_8xiQiGqtxVZ2LhiPkIgdNm2J38qsNMwx
 Content-Transfer-Encoding: quoted-printable
 
 Oops, attached now...
 
 
 > Date: Fri, 25 Sep 2026 14:41:44 +0000
 > From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
 >=20
 > > Date: Thu, 24 Sep 2026 20:17:00 +0300
 > > From: Valery Ushakov <uwe%stderr.spb.ru@localhost>
 > >=20
 > > For the record, just to make it explicit.  Consider simplified:
 > > [...]
 >=20
 > There's probably some shenanigans going on because __sync_* are not
 > normal functions but magic builtins with implicit prototypes.  Is
 > gcc's implicit prototype signed or unsigned?
 >=20
 > uint8_t u8_sync_val_compare_and_swap_1(volatile uint8_t *, uint8_t, uint8=
 _t);
 > int8_t s8_sync_val_compare_and_swap_1(volatile int8_t *, int8_t, int8_t);
 >=20
 > Under these prototypes, I compared three functions:
 >=20
 > - main__ calls the __sync_* builtin
 > - main_u8 calls u8_sync_*
 > - main_s8 calls s8_sync_*
 >=20
 > All three actually use uint8_t for all local variables, just like
 > t___sync_compare_and_swap.c does.
 >=20
 > (I don't natively speak sh3, so please correct me if I've guessed any
 > of the meaning wrong.)
 >=20
 > Here are the relevant excerpts -- in each case, the first jsr calls
 > the sync function, and the second calls printf, so what's interesting
 > is what happens between the two:
 >=20
 > 00000000 <main__>:
 > ...
 >   10:	d0 07       	mov.l	30 <main__+0x30>,r0	! 0 <main__>
 >   12:	96 0c       	mov.w	2e <main__+0x2e>,r6	! f0
 >   14:	40 0b       	jsr	@r0
 >   16:	74 03       	add	#3,r4
 >   18:	66 03       	mov	r0,r6
 >   1a:	d0 06       	mov.l	34 <main__+0x34>,r0	! 0 <main__>
 >   1c:	d4 06       	mov.l	38 <main__+0x38>,r4	! 0 <main__>
 >   1e:	40 0b       	jsr	@r0
 > ...
 > 			30: R_SH_DIR32	__sync_val_compare_and_swap_1
 > 			34: R_SH_DIR32	printf
 > ...
 > 0000003c <main_u8>:
 > ...
 >   4c:	d0 07       	mov.l	6c <main_u8+0x30>,r0	! 0 <main__>
 >   4e:	96 0c       	mov.w	6a <main_u8+0x2e>,r6	! f0
 >   50:	40 0b       	jsr	@r0
 >   52:	74 03       	add	#3,r4
 >   54:	66 03       	mov	r0,r6
 >   56:	d0 06       	mov.l	70 <main_u8+0x34>,r0	! 0 <main__>
 >   58:	d4 06       	mov.l	74 <main_u8+0x38>,r4	! 0 <main__>
 >   5a:	40 0b       	jsr	@r0
 > ...
 > 			6c: R_SH_DIR32	u8_sync_val_compare_and_swap_1
 > 			70: R_SH_DIR32	printf
 > ...
 > 00000078 <main_s8>:
 > ...
 >   82:	d0 08       	mov.l	a4 <main_s8+0x2c>,r0	! 0 <main__>
 >   84:	e6 f0       	mov	#-16,r6
 >   86:	e5 88       	mov	#-120,r5
 >   88:	40 0b       	jsr	@r0
 >   8a:	74 03       	add	#3,r4
 >   8c:	66 0c       	extu.b	r0,r6
 >   8e:	d0 06       	mov.l	a8 <main_s8+0x30>,r0	! 0 <main__>
 >   90:	95 07       	mov.w	a2 <main_s8+0x2a>,r5	! 88
 >   92:	d4 06       	mov.l	ac <main_s8+0x34>,r4	! 0 <main__>
 >   94:	40 0b       	jsr	@r0
 > ...
 > 			a4: R_SH_DIR32	s8_sync_val_compare_and_swap_1
 > 			a8: R_SH_DIR32	printf
 > ...
 >=20
 > Note that main_s8 has an additional extu.b instruction in it!  I'm
 > guessing this means `extend unsigned byte', i.e., zero-extend result
 > in r0 to printf argument in r6.  So it looks like gcc treats
 > __sync_val_compare_and_swap_1 as if it has the signed prototype:
 > main__ matches main_u8, not main_s8.
 >=20
 > If I disassemble t___sync_compare_and_swap.o, I see there's also an
 > extu.b instruction:
 >=20
 > 000009b8 <atfu___sync_val_compare_and_swap_1_body>:
 > ...
 >=20
 > 	// Load expval 0xf0 into r11, which is callee-saves so
 > 	// __sync_val_compare_and_swap_1 should preserve it:
 >      9d0:       9b 52           mov.w   a78 <atfu___sync_val_compare_and_=
 swap_1_body+0xc0>,r11  ! f0
 > ...
 > 	// Load expres 0x88 into r10, which is also callee-saves so
 > 	// __sync_val_compare_and_swap_1 should preserve it:
 >      9d4:       9a 51           mov.w   a7a <atfu___sync_val_compare_and_=
 swap_1_body+0xc2>,r10  ! 88
 > ...
 > 	// Load __sync_val_compare_and_swap_1 function into r1 to
 > 	// call:
 >      9d8:       d1 2a           mov.l   a84 <atfu___sync_val_compare_and_=
 swap_1_body+0xcc>,r1   ! 98
 > ...
 >      9e8:       01 03           bsrf    r1
 > 	// res in r0.  Move res to r9, reuse r0 for the stored val,
 > 	// move stored val to r6, and check whether the stored val is
 > 	// what we expect:
 >      9ea:       64 d3           mov     r13,r4
 >      9ec:       69 03           mov     r0,r9
 >      9ee:       84 8f           mov.b   @(15,r8),r0
 >      9f0:       66 0c           extu.b  r0,r6
 > 	// val =3D=3D expval?
 >      9f2:       36 b0           cmp/eq  r11,r6
 >      9f4:       8d 0b           bt.s    a0e <atfu___sync_val_compare_and_=
 swap_1_body+0x56>
 > 	// res =3D=3D expres?
 >      9f6:       39 a0           cmp/eq  r10,r9
 > ...
 >      a78:       00 f0           .word 0x00f0
 >      a7a:       00 88           .word 0x0088
 > ...
 >      a84:       00 00           .word 0x0000
 >                         a84: R_SH_PLT32 __sync_val_compare_and_swap_1
 >=20
 > Now I'm a little lost with the sh3 branch delay slot shenanigans (is
 > that two comparisons fed into a single conditional branch with a delay
 > slot??), but what I see here is that the _stored_ val is zero-extended
 > with extu.b, but the _returned_ res is not!  That's in stark contrast
 > to the attached program I examined, where the returned result _is_
 > zero-extended with extu.b.
 
 --=_8xiQiGqtxVZ2LhiPkIgdNm2J38qsNMwx
 Content-Type: text/plain; charset="ISO-8859-1"; name="sh3atomic"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename="sh3atomic.c"
 
 #include <inttypes.h>
 #include <stdint.h>
 #include <stdio.h>
 
 #define OLDVAL (0x1122334455667788UL)
 #define NEWVAL (0x8090a0b0c0d0e0f0UL)
 
 uint8_t u8_sync_val_compare_and_swap_1(volatile uint8_t *, uint8_t, uint8_t=
 );
 int8_t s8_sync_val_compare_and_swap_1(volatile int8_t *, int8_t, int8_t);
 
 int
 main__()
 {
     volatile uint8_t val;
     uint8_t oldval;
     uint8_t newval;
     uint8_t expval;
     uint8_t expres;
     uint8_t res;
 
 
     val    =3D (uint8_t)OLDVAL;
     oldval =3D (uint8_t)OLDVAL;
     newval =3D (uint8_t)NEWVAL;
     expval =3D (uint8_t)NEWVAL;
     expres =3D (uint8_t)OLDVAL;
     res =3D __sync_val_compare_and_swap_1(&val, oldval, newval);
 
     printf("expected %x, res =3D %x\n", expres, res);
     return 0;
 }
 
 int
 main_u8()
 {
     volatile uint8_t val;
     uint8_t oldval;
     uint8_t newval;
     uint8_t expval;
     uint8_t expres;
     uint8_t res;
 
 
     val    =3D (uint8_t)OLDVAL;
     oldval =3D (uint8_t)OLDVAL;
     newval =3D (uint8_t)NEWVAL;
     expval =3D (uint8_t)NEWVAL;
     expres =3D (uint8_t)OLDVAL;
     res =3D u8_sync_val_compare_and_swap_1(&val, oldval, newval);
 
     printf("expected %x, res =3D %x\n", expres, res);
     return 0;
 }
 
 int
 main_s8()
 {
     volatile uint8_t val;
     uint8_t oldval;
     uint8_t newval;
     uint8_t expval;
     uint8_t expres;
     uint8_t res;
 
 
     val    =3D (uint8_t)OLDVAL;
     oldval =3D (uint8_t)OLDVAL;
     newval =3D (uint8_t)NEWVAL;
     expval =3D (uint8_t)NEWVAL;
     expres =3D (uint8_t)OLDVAL;
     res =3D s8_sync_val_compare_and_swap_1(&val, oldval, newval);
 
     printf("expected %x, res =3D %x\n", expres, res);
     return 0;
 }
 
 --=_8xiQiGqtxVZ2LhiPkIgdNm2J38qsNMwx--
 



Home | Main Index | Thread Index | Old Index