Re: [PATCH 10/11] x86: Remove unnecessary ifdefs from i387 code.

Previous thread: [PATCH 07/11] x86-64: Simplify constraints for fxsave/fxtstor by Brian Gerst on Saturday, August 28, 2010 - 9:04 am. (4 messages)

Next thread: [PATCH 01/11] x86: Use correct type for %cr4 by Brian Gerst on Saturday, August 28, 2010 - 9:04 am. (2 messages)
From: Brian Gerst
Date: Saturday, August 28, 2010 - 9:04 am

Remove ifdefs for code that the compiler can optimize away on 64-bit.

Signed-off-by: Brian Gerst <brgerst@gmail.com>
---
 arch/x86/include/asm/i387.h |   12 ++++++------
 arch/x86/kernel/i387.c      |    4 ----
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/i387.h b/arch/x86/include/asm/i387.h
index 58cb6f0..3b4675d 100644
--- a/arch/x86/include/asm/i387.h
+++ b/arch/x86/include/asm/i387.h
@@ -55,6 +55,12 @@ extern int save_i387_xstate_ia32(void __user *buf);
 extern int restore_i387_xstate_ia32(void __user *buf);
 #endif
 
+#ifdef CONFIG_MATH_EMULATION
+extern void finit_soft_fpu(struct i387_soft_struct *soft);
+#else
+static inline void finit_soft_fpu(struct i387_soft_struct *soft) {}
+#endif
+
 #define X87_FSW_ES (1 << 7)	/* Exception Summary */
 
 static __always_inline __pure bool use_xsaveopt(void)
@@ -149,12 +155,6 @@ static inline void fpu_fxsave(struct fpu *fpu)
 
 #else  /* CONFIG_X86_32 */
 
-#ifdef CONFIG_MATH_EMULATION
-extern void finit_soft_fpu(struct i387_soft_struct *soft);
-#else
-static inline void finit_soft_fpu(struct i387_soft_struct *soft) {}
-#endif
-
 /* perform fxrstor iff the processor has extended states, otherwise frstor */
 static inline int fxrstor_checking(struct i387_fxsave_struct *fx)
 {
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index b1a732d..e21c138 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -74,10 +74,8 @@ static void __cpuinit init_thread_xstate(void)
 
 	if (cpu_has_fxsr)
 		xstate_size = sizeof(struct i387_fxsave_struct);
-#ifdef CONFIG_X86_32
 	else
 		xstate_size = sizeof(struct i387_fsave_struct);
-#endif
 }
 
 /*
@@ -113,12 +111,10 @@ void __cpuinit fpu_init(void)
 
 void fpu_finit(struct fpu *fpu)
 {
-#ifdef CONFIG_X86_32
 	if (!HAVE_HWFP) {
 		finit_soft_fpu(&fpu->state->soft);
 		return;
 	}
-#endif
 
 	if (cpu_has_fxsr) {
 		struct i387_fxsave_struct *fx = &fpu->state->fxsave;
-- 
1.7.2.2

--

From: Pekka Enberg
Date: Sunday, August 29, 2010 - 12:00 pm

I guess this is OK but keep in mind that cpu_has_fsxr is _not_
optimized by the compiler on 64-bit so the change probably increases
kernel text by few bytes.

Acked-by: Pekka Enberg <penberg@kernel.org>
--

From: Brian Gerst
Date: Sunday, August 29, 2010 - 4:38 pm

FXSR is a required feature on 64-bit, therefore cpu_has_fxsr is always true.

--
Brian Gerst
--

From: Pekka Enberg
Date: Sunday, August 29, 2010 - 10:44 pm

Hi Brian,

Yes, I realize that but it will still read boot_cpu_data at runtime, no?

             Pekka
--

From: Brian Gerst
Date: Monday, August 30, 2010 - 4:21 am

Look at cpu_has().  It checks REQUIRED_MASK* if the feature bit is a
constant, and returns true without testing the actual bit in
boot_cpu_data for required features.

--
Brian Gerst
--

From: Pekka Enberg
Date: Monday, August 30, 2010 - 4:25 am

Right you are, thanks for the explanation!
--

Previous thread: [PATCH 07/11] x86-64: Simplify constraints for fxsave/fxtstor by Brian Gerst on Saturday, August 28, 2010 - 9:04 am. (4 messages)

Next thread: [PATCH 01/11] x86: Use correct type for %cr4 by Brian Gerst on Saturday, August 28, 2010 - 9:04 am. (2 messages)