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

[li guang]

在 2013-05-28二的 09:41 +0200，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.
> 

OK, I'll send a v4

Thanks a lot!