riscv / riscv-isa-manual

RISC-V Instruction Set Manual
https://riscv.org/
Creative Commons Attribution 4.0 International
3.71k stars 643 forks source link

LR/SC loop question #1677

Closed yf13 closed 1 month ago

yf13 commented 1 month ago

Dear experts,

Can you teach if the following loop is a constrained LR/SC loop:


_lock_loop:
   csrrci a0, mstatus, 8
   csrr   t1, mhartid
   auipc  t0, 0x88
   addi   t0, t0, 1308
   lr.w   t2, (t0)
   bnez   t2, _retry
   li     t2, 1
   sc.w   t2, t2, (t0)
   beqz   _got_lock
_retry:
   csrw   mstatus, a0
   wfi
   j      _lock_loop

_got_lock:

Here the LR/SC follows the requirements, but the retry part contains SYSTEM instructions.

allenjbaum commented 1 month ago

The RISC-V Instruction Set Manual Volume I: Unprivileged Architecture

-

The loop comprises only an LR/SC sequence and code to retry the sequence in the case of failure, and must comprise at most 16 instructions placed sequentially in memory

This is condition is met

-

An LR/SC sequence begins with an LR instruction and ends with an SC instruction. The dynamic code executed between the LR and SC instructions can only contain instructions from the base ''I'' instruction set, excluding loads, stores, backward jumps, taken backward branches, JALR, FENCE, and SYSTEM instructions. If the ''C'' extension is supported, then compressed forms of the aforementioned ''I'' instructions are also permitted.

The sequence between LR and SC has no backward jumps or branches or System ops so it is met. Note that code could be reordered to move the csrrci and its label to be just before the lr.w to avoid executing redundant code

-

The code to retry a failing LR/SC sequence can contain backwards jumps and/or branches to repeat the LR/SC sequence, but otherwise has the same constraint as the code between the LR and SC.

This code has a system op (WFI) so fails

-

The LR and SC addresses must lie within a memory region with the LR/SC eventuality property. The execution environment is responsible for communicating which regions have this property.

The SC must be to the same effective address and of the same data size as the latest LR executed by the same hart.

This is met

On Thu, Oct 10, 2024 at 4:44 PM yf13 @.***> wrote:

Dear experts,

Can you teach if the following loop is a constrained LR/SC loop:

_lock_loop: csrrci a0, mstatus, 8 csrr t1, mhartid auipc t0, 0x88 addi t0, t0, 1308 lr.w t2, (t0) bnez t2, _retry li t2, 1 sc.w t2, t2, (t0) beqz _got_lock _retry: csrw mstatus, a0 wfi j _lock_loop

_got_lock:

Here the LR/SC follows the requirements, but the retry part contains SYSTEM instructions.

— Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-manual/issues/1677, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJWBBLLEBIIN53KNZ3TZ24GMXAVCNFSM6AAAAABPX2GSFSVHI2DSMVQWIX3LMV43ASLTON2WKOZSGU4DAMBXGE3DIMI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

yf13 commented 1 month ago

@allenjbaum, appreciate your prompt reply.

Are csrr mstatus,a0 and csrr t1, mhartid also SYSTEM operations?

allenjbaum commented 1 month ago

I didn't mention them because I wasn't sure, but just checked and yes, they are.

On Thu, Oct 10, 2024 at 6:35 PM yf13 @.***> wrote:

@allenjbaum https://github.com/allenjbaum, appreciate your prompt reply.

Are csrr mstatus,a0 and csrr t1, mhartid also SYSTEM operations?

— Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-manual/issues/1677#issuecomment-2406364793, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJUAVPDHIXM2PFH4PCLZ24TOZAVCNFSM6AAAAABPX2GSFSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBWGM3DINZZGM . You are receiving this because you were mentioned.Message ID: @.***>

aswaterman commented 1 month ago

I endorse Allen's answer. The presence of the WFI and CSR instructions in the retry loop are problematic.

If the goal is to save power while the lock is held, you might be able to use the Zawrs extension instead. Consider:

lrsc_loop:
  lr.w t1, (t0)
  bnez t1, sleep
  li t2, 1
  sc.w t2, t2, (t0)
  bnez t2, lrsc_loop

  j done

sleep_loop:
  lr.w t1, (t0)
  beqz t1, retry
sleep:
  wrs.nto
  j sleep_loop

done:
yf13 commented 1 month ago

@aswaterman and @allenjbaum, thank you so much!

yf13 commented 1 month ago

Just one more thing, the spec says backward jump is not allowed in LRSC sequence, so above line bnez t1, outer_loop is not a backward jump, right?

aswaterman commented 1 month ago

You're right, @yf13. I rewrote the example to avoid that pitfall.