Why RISC-V up_testset use lr/sc and not amoswap?

[pkarashchenko]

The RISC-V supports "Atomic Memory Operations" instructions. Seems like up_testset can be implemented with the less number of instruction using amoswap.w.aq/amoswap.w.rl than current implementation that use lr/sc instructions.

[xiaoxiang781216]

The implementation is same as you want: https://github.com/apache/incubator-nuttx/blob/master/arch/risc-v/src/common/riscv_testset.S#L34-L40

[pkarashchenko]

@xiaoxiang781216 the issue is that currently it is implemented as you pointed, but it use lr/sc, but I posted an issue that we can rework it to use amoswap. Current code:

  .globl up_testset
  .type  up_testset, %function

up_testset:

  li       a1, SP_LOCKED

  /* Test if the spinlock is locked or not */

retry:
  LR_INST  a2, (a0)       /* Test if spinlock is locked or not */
  beq      a2, a1, locked /* Already locked? Go to locked: */

  /* Not locked ... attempt to lock it */

  SC_INST  a2, a1, (a0)   /* Attempt to set the locked state (a1) to (a0) */
  bnez     a2, retry      /* a2 will not be zero, if sc.b failed, try again */

  /* Lock acquired -- return SP_UNLOCKED */

  fence                   /* Required before accessing protected resource */
  li        a0, SP_UNLOCKED
  jr        ra

  /* Lock not acquired -- return SP_LOCKED */

locked:
  li        a0, SP_LOCKED
  jr        ra
  .size	up_testset, . - up_testset
  .end

But can be something like

  .globl up_testset
  .type  up_testset, %function

up_testset:

  li       a1, SP_LOCKED
  amoswap.w a2, a1, (a0)  /* Attempt to acquire spinlock atomically */
  beq      a2, a1, locked /* Already locked? Go to locked: */

  /* Lock acquired -- return SP_UNLOCKED */

  fence                   /* Required before accessing protected resource */
  li        a0, SP_UNLOCKED
  jr        ra

  /* Lock not acquired -- return SP_LOCKED */

locked:
  li        a0, SP_LOCKED
  jr        ra
  .size	up_testset, . - up_testset
  .end

[xiaoxiang781216]

Sorry, I misunderstand your meaning.

[pkarashchenko]

Maybe my suggestion has some drawbacks compared to current implementation so I'm posting this as an issue so people some discussion may happen

[masayuki2009]

@pkarashchenko I remember I did the initial implementation for K210 SMP. Actually, I referred to the armv7-a implementation that's why lr/sc are used for it. However, I think amoswap instruction should work for up_testset.