[riot-notifications] [RIOT-OS/RIOT] fe310: Fix potential deadlock in thread_yield_higher (#15277)

Marian Buschsieweke notifications at github.com
Thu Jan 7 17:25:22 CET 2021


The machine code generated by this PR looks like this:

```asm
Disassembly of section .text.thread_yield_higher:

00000000 <thread_yield_higher>:
   0:   020007b7                lui     a5,0x2000
   4:   4705                    li      a4,1
   6:   c398                    sw      a4,0(a5)
   8:   020006b7                lui     a3,0x2000
   c:   4785                    li      a5,1

0000000e <.L12>:
   e:   4298                    lw      a4,0(a3)
  10:   fef70fe3                beq     a4,a5,e <.L12>
  14:   8082                    ret
```

That is suboptimal because the address of `CLINT_MSIP` is first loaded into `a5` and later again in `a3` - why doesn't GCC reuse `a5` for the subsequent load is a mystery to me. Similar the immediate `1` is stored in `a4` and later again in `a5` - again the register could just be reused. Dropping the duplicated code results in 6 B ROM less consumed:

```asm
Disassembly of section .text.thread_yield_higher:

00000000 <thread_yield_higher>:
   0:   4785                    li      a5,1
   2:   02000737                lui     a4,0x2000
   6:   c31c                    sw      a5,0(a4)

00000008 <loop_until_softirq_handled>:
   8:   4314                    lw      a3,0(a4)
   a:   fef68fe3                beq     a3,a5,8 <loop_until_softirq_handled>
   e:   8082                    ret
```

<details> <summary>patch of <code>thread_yield_higher()</code> to safe 6 B of ROM</summary>

```diff
commit 40117a082b354961ed9088e477963e8fce35ed74 (HEAD -> fe310)
Author: Marian Buschsieweke <marian.buschsieweke at ovgu.de>
Date:   Thu Jan 7 17:10:31 2021 +0100

    cpu/fe310: optimize irq_yield_higher()

diff --git a/cpu/fe310/thread_arch.c b/cpu/fe310/thread_arch.c
index ed5687a49f..e12fe76efb 100644
--- a/cpu/fe310/thread_arch.c
+++ b/cpu/fe310/thread_arch.c
@@ -180,13 +180,32 @@ void cpu_switch_context_exit(void)
 
 void thread_yield_higher(void)
 {
-    /* Use SW intr to schedule context switch */
-    CLINT_REG(CLINT_MSIP) = 1;
-
-    /* Latency of SW intr can be a few cycles; wait for the SW intr.
+    /* Let compiler to register allocation via fake output */
+    uint32_t tmp;
+    uint32_t one = 1;
+    uintptr_t clint_msip = CLINT_CTRL_ADDR + CLINT_MSIP;
+    /* The assembly below implements:
+     *
+     *     *((volatile uint32_t)clint_msip) = 1;
+     *      while (*((volatile uint32_t)clint_msip) == 1) { }
+     *
+     * One would assume that the compiler is able to implement this efficiently,
+     * but alas, this is not the case. The inline assembly version safes 6 B.
+     *
+     * Latency of SW intr can be a few cycles; wait for the SW intr.
      * We cannot use WFI here as the SW intr may be delivered before we
-     * reach the WFI instruction, thereby causing a thread deadlock. */
-    while (CLINT_REG(CLINT_MSIP) == 1) {};
+     * reach the WFI instruction, thereby causing a thread deadlock.
+     */
+    __asm__ volatile (
+        "sw %[one], 0(%[clint_msip])"                                       "\n"
+        "loop_until_softirq_handled:"                                       "\n"
+        "lw %[tmp], 0(%[clint_msip])"                                       "\n"
+        "beq %[tmp], %[one], loop_until_softirq_handled"                    "\n"
+        : [tmp]         "=&r"(tmp)
+        : [one]         "r"(one),
+          [clint_msip]  "r"(clint_msip)
+        : "memory"
+    );
 }
 
 /**

```

</details>

But I would be surprised if two CPU instructions less would do much about the 6-7% performance regression.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/15277#issuecomment-756221953
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210107/4ee98cc1/attachment.htm>


More information about the notifications mailing list