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

["Valery Ushakov via gnats" <gnats-admin%NetBSD.org@localhost>]

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

From: Valery Ushakov <uwe%stderr.spb.ru@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: Taylor R Campbell <riastradh%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 19:32:04 +0300

 On Fri, Sep 25, 2026 at 14:41:44 +0000, Taylor R Campbell wrote:
 
 > 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
 [...]
 >
 > 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.
 
 You are right.  sh3 psABI specifies that unsigned char is returned in
 the LSB of r0 and is zero-extended, while signed char is returned
 sign-extended.  So that extu.b does the coercion of res to its
 declared uint8_t type and it can be elided for main_u8, b/c ABI
 already requires r0 to be zero-extended.
 
 
 
 > If I disassemble t___sync_compare_and_swap.o, I see there's also an
 > extu.b instruction:
 > 
 > 000009b8 <atfu___sync_val_compare_and_swap_1_body>:
 > ...
 > 
 > 	// 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
 > ...
 
 Side note, there's no mov.b variant for pc-relative load.  mov.w
 sign-extends, but 0x0088 is positive, so we get the the right uint8_t
 value in r10.
 
 
 > 	// 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
 >      9ea:       64 d3           mov     r13,r4
 > 	// 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:
 >      9ec:       69 03           mov     r0,r9
 >      9ee:       84 8f           mov.b   @(15,r8),r0
 >      9f0:       66 0c           extu.b  r0,r6
 
 Side note - on sh3 mov.b from memory (and mov.w) can only load to R0,
 hence the extra song and dance.  Both implicitly sign-extend R0.
 
 > 	// val == expval?
 >      9f2:       36 b0           cmp/eq  r11,r6
 >      9f4:       8d 0b           bt.s    a0e <atfu___sync_val_compare_and_swap_1_body+0x56>
 > 	// res == expres?
 >      9f6:       39 a0           cmp/eq  r10,r9
 > ...
 
 Right, and r10 is (uint8_t)0x88, and r9 (returned as uint8_t) is
 expected to be zero-extended b/c the ABI.
 
 As we know from the observation - it's not, b/c the actual function
 was defined as returning int8_t and hence it retutned sign-extended
 value, but the caller expects otherwise.
 
 
 > 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??),
 
 We do the first comparison and branch on its result.  We do the second
 comarison in the delay slot of that branch to not waste a perfectly
 good clock cycle, the value of that comparison will be used later.
 
 
 > 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.
 
 As I mentioned above, the loaded value must be zero-extended b/c the
 instruction sign-extends it on load.
 
 
 
 -uwe