mips: fcmpset: do not spin on sc failure

[kevans]

Summary

For ll/sc architectures, atomic(9) allows failure modes where *old == val due to write failure and callers should compensate for this. Do not retry on failure, just leave 0 in ret and fail the operation if we couldn't sc it.

[kib]

sys/mips/include/atomic.h

405 ↗

(On Diff #62703)

exit on failure (or abort)

406 ↗

(On Diff #62703)

This label should be '1' etc.

[kevans]

Re-label jumps and revise comment to be more specific

[kib]

sys/mips/include/atomic.h

405 ↗

(On Diff #62709)

So this uses the delay slot to provide the old value, right ? I think it deserves a comment. (Not that the old code did explained it).

[kevans]

sys/mips/include/atomic.h

405 ↗

(On Diff #62709)

I do not believe so- in the comparison failure case, we've jumped to the sw below and provided. If the comparison didn't fail, we've left *cmpval untouched.

[imp]

sys/mips/include/atomic.h

405 ↗

(On Diff #62709)

Then you need a nop here. Otherwise I think the following instruction is used...
unless there's a special rule that a delay slot won't cross a label...

514 ↗

(On Diff #62709)

Same here.

[kevans]

sys/mips/include/atomic.h

405 ↗

(On Diff #62709)

Regardless of whether it can cross a label or not (which would seem odd, my impression was labels aren't actually compiled in but rather just the location noted for replacement in the underlying asm... but I haven't bothered checking this), it would likely be better to provide nop to be explicit.

[imp]

sys/mips/include/atomic.h

405 ↗

(On Diff #62709)

yea... The assembler is 'smart' and helps one manage the delay slots and sometimes inserts NOPs and other times doesn't. It's been 20ish years since I did MIPS assembler enough to have that swapped into my brain all the special times things would be written, and when they wouldn't... explicit nop would make it clear.

[kevans]

Add nops to document that we're expecting no delay slot interactions to be happening here...

I would assume the assembler's implicitly adding this nop anyways based on the old version that we're replacing, but explicit is better than implicit.

[kib]

sys/mips/include/atomic.h

405 ↗

(On Diff #62735)

Actually this is an exit not only on failure.

[kevans]

Revise incorrect comment, drop a more complete comment describing that it's intentionally leaving *cmpval untouched and a pointer to the documentation that says it's ok.