Re: [Qemu-devel] qemu hw/ppc_oldworld.c target-ppc/cpu.h target-...

[Jocelyn Mayer]

On Fri, 2007-11-23 at 18:22 +0000, Paul Brook wrote:
> > Furthermore this patch was made in a brainless way, it will be reverted
> > asap.
> > If you think there is a bug in someone else code, submit it a patch, if
> > it's cleaver and addresses a real bug (which is not the case here) it
> > will be accepted and merged.
> 
> The old code before the patch is obviously broken. It's mixing 64-bit 
> (ppc_gpr_t) and 32-bit (target_ulong) values.

It seems you do not understand that what was done was correct. It's not
mixing two different types. GPR are of ppc_gpr_t type and should be
displayed this way. The only case which is incorrect is not addressed by
your patch. But your patch breaks the general case which was OK.

> 
> As implied by the comments aboce the definition of ppc_gpt_t, and now 
> explicitly in the above the definition of REGX, printing a ppc_gpr_t is 
> obviously not meaningful.
> 
> I don't claim that my patch is perfect, the code is still a bit of a mess. 
> However, unlike the original code, it is at least self-consistent, and won't 
> crash 64-bit hosts (The fact that it usually prints garbage rather than 
> crashing is an accident of the x64-64 ABI).

It's not garbage. On 64 bits hosts, the 64 bits GPR dump is correct. GPR
_are 64 bits_ when compiling the ppcemb target and should be displayed
as 64 bits value. It's not correct on 32 bits targets, because the
highest 32 bits of the GPR should be printed and they are not. Here's
the real bug. Your patch break the first case, which was OK, and does
not fix any actual bug.