Re: [Qemu-devel] Re: [PATCH 01/12] TCG "sync" op

[Alexander Graf]

Paul Brook wrote:
> On Thursday 22 October 2009, Aurelien Jarno wrote:
>   
>> On Wed, Oct 21, 2009 at 03:52:22PM +0200, Ulrich Hecht wrote:
>>     
>>> sync allows concurrent accesses to locations in memory through different
>>> TCG variables. This comes in handy when you are emulating CPU registers
>>> that can be used as either 32 or 64 bit, as TCG doesn't know anything
>>> about aliases. See the s390x target for an example.
>>>
>>> Fixed sync_i64 build failure on 32-bit targets.
>>>       
>> Looking more in details to the use case of this patch, I think it can be
>> useful in QEMU. However I don't feel very comfortable in merging it
>> without having the opinion of more persons. Paul, Malc Blue Swirl or
>> others, any opinion?
>>     
>
> I don't think this is the right solution.
>
> IIUC the basic problem is that we have a register file where adjacent pairs 
> of 
> 32-bit registers are also accessed as a 64-bit value.  This is something many 
> other targets need to do (at least ARM, PPC, MIPS and SPARC).
>
> While sync appears attractive as a quick hack to achieve this, I think it is 
> liable to be abused, and cause us serious pain long-term. If you need an easy 
> solution then use ld/st (as with ARM VFP registers). If you want a good 
> solution then fix whichever bit of TCG makes accessing a pair of registers 
> horribly slow. We already have some support for this (concat_i32_i64).
>   

Could we please include it nevertheless? I don't want to see S390 TCG
and KVM targets not included because of this sync operation.

If you like, add some big fat warning around it and maybe break
compilation if anyone but the s390 target uses that sync op, but let's
not keep a whole target from inclusion because of a single feature that
_might_ one day affect others.


Alex