On Wed, Jun 02, 2021 at 09:25:54PM +0200, Volker Rümelin wrote:
On Tue, Jun 01, 2021 at 09:06:54PM +0200, Volker Rümelin wrote:
The variables data, func, thread and cur are input operands for
the asm statement in run_thread(). The assembly code clobbers
these inputs.
>From the gcc documentation chapter Extended-Asm, Input Operands:
"It is not possible to use clobbers to inform the compiler that
the values in these inputs are changing. One common work-around
is to tie the changing input variable to an output variable that
never gets used."
Thanks. However, this change doesn't look correct to me. The
variables data, func, thread and cur were all *output* operands. They
were output operands that use "+" to indicate that they also have
inputs associated with them. That is, unless I'm missing something,
the asm already followed the gcc documentation (use an output operand
and don't use the results of it).
Hi Kevin,
the "+" output constraint indicates that the assembly code uses
the variable as input and updates the same variable.
The gcc manual does not say the compiler will not use the output
operand result. It actually uses the updated variable.
Your code works because you never use the output variables.
Did you observe a problem during runtime with the original code?
My next patch does not work, because the compiler assumes edx is
the updated variable cur at the end of the assembly code, but
edx is actually clobbered. Just use a dprintf() to print cur
before and after the asm statement to see I'm right.
objdump -dS src/stacks.o will show the same.
Okay, I think I understand the issue. The asm constrains aren't
wrong, but "cur" is clobbered by the asm. I think it's probably
simpler to use this in the new code:
if (getCurThread() == &MainThread)
check_irqs();
And do something similar in yield(). That is, not save/restore
getCurThread() during stack switches. If it helps, we can also rename
"cur" to "edx" to make it more clear that the C variable is clobbered.
The compiler on my computer decides it's better to save/restore
getCurThread() during stack switches. It optimizes the second
getCurThread() away. My compiler also applies that optimization
to yield().
I will send version 4 patch series where I drop patch 1/3, rename
cur to edx in patch 2/2 and keep patch 3/3.
With best regards,
Volker
Add unused output variables and fix the asm constraints in
run_thread().
Signed-off-by: Volker Rümelin<vr_q...@t-online.de>
---
src/stacks.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/stacks.c b/src/stacks.c
index 2fe1bfb..ef0aba1 100644
--- a/src/stacks.c
+++ b/src/stacks.c
@@ -565,6 +565,7 @@ run_thread(void (*func)(void*), void *data)
thread->stackpos = (void*)thread + THREADSTACKSIZE;
struct thread_info *cur = getCurThread();
hlist_add_after(&thread->node, &cur->node);
+ u32 unused_a, unused_b, unused_c, unused_d;
asm volatile(
// Start thread
" pushl $1f\n" // store return pc
@@ -576,14 +577,15 @@ run_thread(void (*func)(void*), void *data)
// End thread
" movl %%ebx, %%eax\n" // %eax = thread
" movl 4(%%ebx), %%ebx\n" // %ebx = thread->node.next
- " movl (%5), %%esp\n" // %esp = MainThread.stackpos
- " calll %4\n" // call __end_thread(thread)
+ " movl (%9), %%esp\n" // %esp = MainThread.stackpos
+ " calll %8\n" // call __end_thread(thread)
" movl -4(%%ebx), %%esp\n" // %esp = next->stackpos
" popl %%ebp\n" // restore %ebp
" retl\n" // restore pc
"1:\n"
- : "+a"(data), "+c"(func), "+b"(thread), "+d"(cur)
- : "m"(*(u8*)__end_thread), "m"(MainThread)
+ : "=a"(unused_a), "=c"(unused_c), "=b"(unused_b), "=d"(unused_d)
+ : "0"(data), "1"(func), "2"(thread), "3"(cur),
+ "m"(*(u8*)__end_thread), "m"(MainThread)
The original code made sure data was in eax, func in ecx, thread in
ebx, and cur in edx. The altered code no longer enforces this
association and I think that could introduce a bug.
The digit input constraints tie the variables to the correct
register just like the old code. data, func, thread and cur are
still in eax, ecx, ebx and edx at the beginning of the assembly
code.
Sorry, you are correct, I missed the association. That said, I don't
think we need to tell gcc to save/restore the original values during
the stack switch as they can be easily recalculated.
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
