On Feb 25, 2008, at 6:40 PM, Avi Kivity wrote:
> Alexander Graf wrote:
>>
>> The ebx store was done because of PIC code, which does not allow
>> ebx to get clobbered. If we are not in PIC code, =r contains ebx as
>> GPR though, so the assumption that ebx needs to be restored was
>> wrong then. This new version only enables the store/restore code if
>> i386 and PIC code are used. There is no need to distinguish between
>> x86_64 and i386 for the other cases.
>>
>> So does this version work?
>>
>
> It probably will, but it seems fragile to depend on the details of
> PIC. I committed something more generic:
>
> #ifdef __x86_64__
> asm volatile("cpuid"
> : "=a"(vec[0]), "=b"(vec[1]),
> "=c"(vec[2]), "=d"(vec[3])
> : "0"(function) : "cc");
This code works fine for all targets, including i386. With PIC
enabled, gcc registers the ebx registers and complains about this,
thus errors out. This is the only special case I am aware of, so I
doubt we should treat any case different from the "normal" case but
the PIC one.
>
> #else
> asm volatile("pusha \n\t"
>
> "cpuid \n\t"
> "mov %%eax, 0(%1) \n\t"
> "mov %%ebx, 4(%1) \n\t"
> "mov %%ecx, 8(%1) \n\t"
> "mov %%edx, 12(%1) \n\t"
>
> "popa"
> : "a"(function), "S"(vec) : "memory", "cc");
> #endif
Basically #ifdef __x86_64__ is even wrong, as the problem is not that
too many registers are being used, but that ebx is reserved and can't
be saved/restored automatically.
Furthermore I believe that the less assembler is used, the better the
code looks. So for cases the snippet above is not required, why use
it? Overusing assembler is imho exactly the reason the previous code
broke.
There's one more thing I'd like to add here. Gcc optimizes really well
when one lets it to. So for this exact case with -O2 used, there are
no memory accesses. The vector is simply stored in 4 registers and
thus no more movs are required.
Alex
-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
