Re: [Qemu-ppc] [PATCH for-1.5?] target-ppc: Drop unnecessary dynamic cast in ppc_env_get_cpu()

[Paolo Bonzini]

Il 10/05/2013 16:47, Andreas Färber ha scritto:
> Am 10.05.2013 16:42, schrieb Peter Maydell:
>> On 10 May 2013 15:39, Andreas Färber <address@hidden> wrote:
>>> A transition from CPUPPCState to PowerPCCPU can be considered safe,
>>> just like PowerPCCPU::env access in the opposite direction.
>>>
>>> This should slightly improve interrupt performance.
>>
>>>  static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
>>>  {
>>> -    return POWERPC_CPU(container_of(env, PowerPCCPU, env));
>>> +    return container_of(env, PowerPCCPU, env);
>>>  }

This doesn't fix the same in problem in ENV_GET_CPU.  It would fix half
of the problem, but not the other half.

I don't want to think in advance of what casts are in hot paths.  The
less I think about such useless details, the less bug I will put in my
code.  I _want_ to code defensively (and have gdb pinpoint problems fast
while developing), I just don't want that to come at a performance cost
for users.

Algorithmic invariants are what you need to assert against.  As far as
type-safety is concerned, people are used to SIGSEGVs and it doesn't
change anything to them if they get SIGABRTs instead.  *If* the
type-safety is a major concern, at least patch 4 of my series should be
a no brainer.  And with the tracing support introduced by patch 5, the
price of having a little less type-safety should be more than acceptable.

>> So if this is worthwhile shouldn't we be doing it for
>> all our CPUs?
> 
> I thought ppc were the exception, but you're right there's 15
> occurrences remaining, i.e. all targets do it that way currently.
> 
> Don't have time right now for large cross-tree cleanups, so feel free to
> profile with and without this patch.

Well, we have a week to get this in.

Paolo