Re: [Qemu-devel] [update][PATCH 00/12] target-i386: remove some macros

[Andreas Färber]

Am 28.05.2013 04:11, schrieb li guang:
> 在 2013-05-26日的 17:55 +0200，Andreas Färber写道：
>> Am 24.05.2013 13:37, schrieb Andreas Färber:
>>> Am 23.04.2013 10:16, schrieb liguang:
>>>> remove macros EAX, EBX, ECX, EDX, EBP, ESP, ESI, EDI, EIP, DF
>>>> as suggested by Richard Henderson <address@hidden>
>>>>
>>>> Li Guang (12)
>>>>     target-i386/helper: remove EAX macro
>>>>     target-i386/helper: remove EBX macro
>>>>     target-i386/helper: remove ECX macro
>>>>     target-i386/helper: remove EDX macro
>>>>     target-i386/helper: remove EBP macro
>>>>     target-i386/helper: remove ESP macro
>>>>     target-i386/helper: remove ESI macro
>>>>     target-i386/helper: remove EDI macro
>>>>     target-i386/helper: remove EIP macro
>>>>     target-i386/helper: remove DF macro
>>>>     target-i386/helper: remove redundant env->eip assignment
>>>>     target-i386: fix over 80 chars warnings
>>>
>>> Hard Freeze is over, so more time to look into refactorings:
>>>
>>> There's one thing to be aware of here, macros would make it easier to
>>> transition from CPUX86State to X86CPU fields. However I am guessing that
>>> all these registers are accessed by TCG code via offsets from cpu_env -
>>> please verify that.
>>
>> Confirmed, I just stumbled over it myself in translate.c:
>> * all but EIP and DF are assigned to cpu_regs[]
>> * EIP is manually stored with offsetof() twice
>> * DF is manually loaded/stored with offsetof() thrice
>>
> 
> Hi, Andreas
> 
> do you mean I should not remove macros EIP and DF ?

I'm OK with all 12 patches as stated before. However as pointed out
below ...

> 
> or can you pick these patches or cherry-pick some of them.
> 
> Thanks!
> 
>>
>>> If so,
>>>
>>> Reviewed-by: Andreas Färber <address@hidden>
>>>
>>> However, it would be nice if you could fix the \ alignment in patch
>>> 06/12 or in the cleanup patch 12/12.

... patch 06/12 breaks the alignment of the '\' characters through your
search-and-replace, so please fix that up in patch 06/12 or in 12/12 or
append a 13th patch.

I'm not the maintainer of target-i386 TCG code, so this is not for me to
pick up, therefore my Reviewed-by. I'm expecting rth or Blue to pick
them up once you've sent a v4.

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg