Re: [Qemu-devel] [PATCH v3 00/29] tcg-aarch64 improvements

[Richard Henderson]

On 09/09/2013 08:02 AM, Claudio Fontana wrote:
> On 09.09.2013 16:08, Richard Henderson wrote:
>> On 09/09/2013 01:13 AM, Claudio Fontana wrote:
>>> after carefully reading and testing your patches, this is how I suggest to 
>>> proceed: 
>>>
>>> first do the implementation of the new functionality (tcg opcodes, jit) in 
>>> a way that is consistent with the existing code.
>>> No type changes, no refactoring, no beautification.
>>>
>>> Once we agree on those, introduce the meaningful restructuring you want to 
>>> do,
>>> like the new INSN type, the "don't handle mov/movi in tcg_out_op", the 
>>> TCG_OPF_64BIT thing, etc.
>>>
>>> Last do the cosmetic stuff if you really want to do it, like the change all 
>>> ext to bool (note that there is no point if the callers still use "1" and 
>>> "0": adapt them as well) etc.
>>
>> No, I don't agree.  Especially with respect to the insn type.
>>
>> I'd much rather do all the "cosmetic stuff", as you put it, first.  It makes
>> all of the "real" changes much easier to understand.
>>
>>
>> r~
>>
> 
> I guess we are stuck then. With the cosmetic and restructuring stuff coming 
> before, I cannot cherry pick the good parts later.
> 
> 

Have you tested the first 9 patches on their own?  I.e.

>   tcg-aarch64: Set ext based on TCG_OPF_64BIT
>   tcg-aarch64: Change all ext variables to bool
>   tcg-aarch64: Don't handle mov/movi in tcg_out_op
>   tcg-aarch64: Hoist common argument loads in tcg_out_op
>   tcg-aarch64: Change enum aarch64_arith_opc to AArch64Insn
>   tcg-aarch64: Merge enum aarch64_srr_opc with AArch64Insn
>   tcg-aarch64: Introduce tcg_fmt_* functions
>   tcg-aarch64: Introduce tcg_fmt_Rdn_aimm
>   tcg-aarch64: Implement mov with tcg_fmt_* functions

There should be no functional change to the backend, producing 100% identical
output code.  There should even be little or no change in tcg.o itself.

This should make it trivial to verify that these patches are not at fault.



r~