Skip to contentSkip to conversation

Fix msr register allocation

PATCH In review Glauber de Oliveira Costa 3 participants 4 messages Jul 30, 2007

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix msr register allocation

Since the value in ret will go through a return statement, it does not need to be put in eax register directly. Instead, we let the compiler do his job and choose what to do with it, potentially getting a better register allocation.

include/asm-i386/msr.h +1 -1
index df21ea0..cc4263c 100644
@@ -54,7 +54,7 @@ static inline int native_write_msr_safe(unsigned int msr,
 		     "   .align 4\n\t"
 		     "   .long 	2b,3b\n\t"
 		     ".previous"
-		     : "=a" (err)
+		     : "=r" (err)
 		     : "c" (msr), "0" ((u32)val), "d" ((u32)(val>>32)),
 		       "i" (-EFAULT));
 	return err;
#1 lore raw
Quoted 1 line · Glauber de Oliveira Costa

Since the value in ret will go through a return statement,

                     ^^^

You mean "err" I guess?

Quoted 11 lines

it does not need to be put in eax register directly. Instead, we let the compiler do his job and choose what to do with it, potentially getting a better register allocation.

include/asm-i386/msr.h +0 -0
index df21ea0..cc4263c 100644

include/asm-i386/msr.h static inline int native_write_msr_safe(unsigned int msr, · line 60

Quoted 8 lines
include/asm-i386/msr.h +1 -1
@@ -54,7 +54,7 @@ static inline int native_write_msr_safe(unsigned int msr,
 		     "   .align 4\n\t"
 		     "   .long 	2b,3b\n\t"
 		     ".previous"
-		     : "=a" (err)
+		     : "=r" (err)
 		     : "c" (msr), "0" ((u32)val), "d" ((u32)(val>>32)),
 		       "i" (-EFAULT));
 	return err;

Note that the EAX output constraint is re-used in the input section for (u32)val, i.e. the lower half of the value to be written. And "wrmsr" needs that in EAX, so you cannot change the output constraint without touching the input constraint.

Also, I do not see how the compiler could do any better than having the return value already in EAX, but that doesn't really mean anything ;-)

Björn

#2 lore raw
Quoted 4 lines · Björn Steinbrink
Quoted 1 line · Glauber de Oliveira Costa

Since the value in ret will go through a return statement,

                     ^^^

You mean "err" I guess?

Yeah, you're right.

Quoted 4 lines

Note that the EAX output constraint is re-used in the input section for (u32)val, i.e. the lower half of the value to be written. And "wrmsr" needs that in EAX, so you cannot change the output constraint without touching the input constraint.

Right again. Will send a new patch that also changes the input constraint.

Quoted 2 lines

Also, I do not see how the compiler could do any better than having the return value already in EAX, but that doesn't really mean anything ;-)

Yet, we should only force the compiler to a particular choice when we have to, IMHO. So, unless anyone gives another reason not to, I will send another patch with your warnings fixed.

#3 lore raw
Quoted 5 lines · Glauber de Oliveira Costa
Quoted 4 lines

Note that the EAX output constraint is re-used in the input section for (u32)val, i.e. the lower half of the value to be written. And "wrmsr" needs that in EAX, so you cannot change the output constraint without touching the input constraint.

Right again. Will send a new patch that also changes the input constraint.

Please also "fix" the subject then. You can only fix something that's broken, but this isn't.

-Andi