Question about which tools to use, bugs, the best way to implement a function, etc should go here. Don't forget to see if your question is answered in the wiki first! When in doubt post here.
giovanig
Member
Posts: 29 Joined: Tue Mar 13, 2012 12:03 pm
Post
by giovanig » Fri May 10, 2013 8:01 am
Hi,
I would like to save the FPU state onto a context pointer in order to switch the context between two threads. I have the following context:
Code: Select all
struct _fpstate _fpu_ctx; //has 108 bytes
Reg32 _edi; //GP registers + eflags + eip = 40 bytes
Reg32 _esi;
Reg32 _ebp;
Reg32 _esp;
Reg32 _ebx;
Reg32 _edx;
Reg32 _ecx;
Reg32 _eax;
Reg32 _eflags;
Reg32 _eip;
The total context size is 148 bytes.
My switch context method is below. I want to save the context onto "*o" and restore the previous saved context from "n".
Code: Select all
void IA32::switch_context(Context * volatile * o, Context * volatile n)
{
ASM(" pushfl \n"
" pushal \n" //the first two instructions pushes 40 bytes
" sub %esp, 108 \n"
" fnsave -108(%esp) \n"
" movl 148(%esp), %eax # old \n"
" movl %esp, (%eax) \n"
" movl 152(%esp), %esp # new \n"
" frstor (%esp) \n"
" add %esp, 108 \n"
" popal \n"
" popfl \n");
}
I am getting a GP fault on this code. Any clues of what I am doing wrong? I am considering that fnsave pushes 108 bytes, as states the manual.
Thanks in advance,
Giovani
Combuster
Member
Posts: 9301 Joined: Wed Oct 18, 2006 3:45 am
Libera.chat IRC: [com]buster
Location: On the balcony, where I can actually keep 1½m distance
Contact:
Post
by Combuster » Fri May 10, 2013 8:21 am
I don't trust any CPU task switcher at all when the save and restore parts aren't properly matched. Regardless of what it should be, this is what it's doing right now:
Code: Select all
old_context->first_field = esp
esp = &(new_context->first_field)
After that, there's no telling what really happens, evenso when you
Code: Select all
fpu_state[-1] = current_state
(...)
current_state = fpu_state[0]
Which evenso tosses things in places where they don't belong.
Last edited by
Combuster on Fri May 10, 2013 8:27 am, edited 1 time in total.
"Certainly avoid yourself. He is a newbie and might not realize it. You'll hate his code deeply a few years down the road." - Sortie
[
My OS ] [
VDisk/SFS ]
Owen
Member
Posts: 1700 Joined: Fri Jun 13, 2008 3:21 pm
Location: Cambridge, United Kingdom
Contact:
Post
by Owen » Fri May 10, 2013 8:26 am
Also you're not even accessing the right stack addresses (GCC built a function call frame around your ASM block!)
giovanig
Member
Posts: 29 Joined: Tue Mar 13, 2012 12:03 pm
Post
by giovanig » Fri May 10, 2013 8:55 am
I'm not using the ebp. g++ generated the exact asm code I posted. The past code was:
Code: Select all
ASM(" pushfl \n"
" pushal \n"
" movl 40(%esp), %eax # old \n"
" movl %esp, (%eax) \n"
" movl 44(%esp), %esp # new \n"
" popal \n"
" popfl \n")
It worked perfectly, saving and restoring only GP registers, eip, and eflags. Then, I added the space for the fpu state to the Context and added the fnsave and frstor instructions to swtich_context method.
Does FPU really save 108 bytes?
Owen
Member
Posts: 1700 Joined: Fri Jun 13, 2008 3:21 pm
Location: Cambridge, United Kingdom
Contact:
Post
by Owen » Fri May 10, 2013 11:14 am
Doesn't matter. GCC will build its own function call frame for all functions - having just an asm block in there doesn't stop it...
either use the parameter addresses GCC passes (not possible in this case) or don't use inline assembly.
Modern AVX state is over 1kb and has tight alignment requirements
giovanig
Member
Posts: 29 Joined: Tue Mar 13, 2012 12:03 pm
Post
by giovanig » Fri May 10, 2013 12:11 pm
Even if the file is compiled with -fomit-frame-pointer?
Owen
Member
Posts: 1700 Joined: Fri Jun 13, 2008 3:21 pm
Location: Cambridge, United Kingdom
Contact:
Post
by Owen » Fri May 10, 2013 12:36 pm
Yes.