On Fri, 22 Jul 2005, Adrian Bunk wrote:
If this patch makes a difference, could you do me a favour and check whether replacing the current cpu_has_fxsr #define in
#define cpu_has_fxsr 1
on top of your patch brings an additional improvement?
It would be really sad if it made a difference. There might be a branch
mispredict, but the real expense of the fnsave/fxsave will be that
instruction itself, and any cache misses associated with it. The 9%
performace difference would almost have to be due to a memory bank
conflict or something (likely some unnecessary I$ prefetching that
interacts badly with the writeback needed for the _big_ memory write
forced by the fxsave).
I can't see any way that a single branch mispredict could make that big of a difference, but I _can_ see how bad memory access patterns could do it.
Btw, the switch from fnsave to fxsave (and thus the change from a 112-byte
save area to a 512-byte one, or whatever the exact details are) caused
_huge_ performance degradation for various context switching benchmarks. I
really hated that, but obviously the need to support SSE2 made it
non-optional. The point being that the real overhead is that big memory read/write in fxrestor/fxsave.
What _could_ make a bigger difference is not doing the lazy FPU at all. That lazy FPU is a huge optimization on 99.9% of all loads, but it sounds
like java/volanomark are broken and always use the FPU, and then we take a
big hit on doing the FP restore exception (an exception is a lot more
expensive than a mispredict).
Something like the following (totally untested) should make it be
non-lazy. It's going to slow down normal task switches, but might speed up the "restoring FP context all the time" case.
Chuck? This should work fine with or without your inline thing. Does it make any difference?