Re: [Qemu-devel] [PATCH v2 06/27] target/sh4: Handle user-space atomics

[Aurelien Jarno]

On 2017-07-16 09:35, Richard Henderson wrote:
> On 07/16/2017 05:18 AM, Aurelien Jarno wrote:
> > That said for further improvements did you consider decoding the gUSA
> > section in a helper. It might avoid having to emulate the atomic
> > sequence with 3 TBs in the worst case (the original one, the one to
> > decode the sequence and the one holding the exclusive lock). The helper
> > should directly have access to the r0 value, can decode the atomic
> > sequence and translate it into a call to the corresponding atomic
> > helpers. In the best case that means the sequence can be done in the
> > same TB. In the worst case, where the gUSA sequence is not recognized
> > it means two TB.
> 
> I did not consider decoding the sequence in a helper.
> 
> I do want to cache the result in a TB, so that when we execute the atomic
> sequence for a second time we do not have to re-interpret it.  And I don't
> see how to do that with a helper.

Indeed, if the same atomic code is used often it might be better to have
it cached. That said it's only true for TB that are recognized, as IIRC
TB with the exclusive lock are not cached.

> I *could* remember a mova into R0 earlier within the same block that
> performs the mov #imm,r15.  It's not guaranteed to be in the same TB, but
> looking through dumps from debian unstable it's highly likely.
> 
> That could end up with the atomic sequence in the same TB that set SP, or
> with the EXCP_ATOMIC exception in the same TB.

That would be a nice improvement indeed.

> If it's ok with you, I'd prefer to handle that as a follow up.  We would
> still have to have the current setup as a backup just in case the mova isn't
> visible.

Yes, of course. I am fine with this version of the patch (hence the
R-b). I was just giving the ideas I got when reviewing this patch before
I forget about them.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net