From 947b49ee82fbb3ba6d0f5771d570d624ffc87890 Mon Sep 17 00:00:00 2001
From: Nadav Har'El <nyh@cloudius-systems.com>
Date: Sun, 26 May 2013 23:12:25 +0300
Subject: [PATCH] sched: avoid unnecessary FPU saving

Because of Linux's calling convention, it should not be necessary to
save the FPU state when a reschedule is caused by a function call.

Because we had a bug and forgot to save the FPU state when calling
a signal handler, and because this signal handler can cause a reschedule,
we had to save the FPU on any reschedule. But after fixing that bug, we
no longer need these unnecessary FPU saves.

The "sunflow" benchmark still runs well after this patch.
---
 core/sched.cc | 11 -----------
 todo/fpu      | 16 ----------------
 2 files changed, 27 deletions(-)

diff --git a/core/sched.cc b/core/sched.cc
index 1994293ab..103a25a0e 100644
--- a/core/sched.cc
+++ b/core/sched.cc
@@ -93,22 +93,11 @@ void cpu::reschedule_from_interrupt(bool preempt)
     if (n != thread::current()) {
         if (preempt) {
             p->_fpu.save();
-        } else {
-            // FIXME: In this case, in theory, we do not need to save the FPU
-            // state (perhaps just mxcsr and cw) because we got here through a
-            // function call and the calling conventions guarantee the caller
-            // clears the FPU state. Unfortunately, in practice, the SPECjvm
-            // "sunflow" benchmark breaks (gets wrong results) if we don't
-            // save the SSE registers here. We don't know why.
-            p->_fpu.save();
         }
         trace_switch(n);
         n->switch_to();
         if (preempt) {
             p->_fpu.restore();
-        } else {
-            // FIXME: This shouldn't be here! See comment above.
-            p->_fpu.restore();
         }
     }
 }
diff --git a/todo/fpu b/todo/fpu
index bc20d5ed7..080e2bfce 100644
--- a/todo/fpu
+++ b/todo/fpu
@@ -1,21 +1,5 @@
 Avoiding unnecessary FPU state saving
 =====================================
 
-1. Understand why the "sunflow" benchmark fails unless we also save FPU
-   state on reschedule_from_interrupt with !preempt:
-
-   1. One guess is that Java has a bug where it doesn't follow the calling
-      conventions and doesn't save its fpu state before calling some library
-      function. Avi tried to reproduce this in Linux (with a layer that
-      ruins the FPU state before calling library functions) and couldn't
-      so this may not be the correct explanation.
-
-   2. Another guess (by Avi) is that a signal handler is involved and
-      we don't save the FPU on signals! Need to fix that.
-
 2. Even if we don't need to save the whole fpu state on !preempt, we probably
    need to save the fcw and mxcsr.
-
-3. If for some reason (?) this problem is unfixable and we must (why?) save
-   FPU on every context switch, we may be forced to consider lazy FPU
-   switching - which we tried to avoid.
-- 
GitLab