harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gregory Shimansky <gshiman...@apache.org>
Subject Re: [drlvm][jit][jet][x86_64] Lazy resolution for JET
Date Thu, 24 Jan 2008 12:05:48 GMT
Mikhail Fursov said the following on 24.01.2008 13:11:
> Gregory, here are the answers to your questions:
> 
> On Jan 23, 2008 8:02 PM, Gregory Shimansky <gshimansky@apache.org> wrote:
> 
>> The code that contained calls for lazy resolution helpers have
>> unbalanced runlock calls when CallSig is not locked, so assertions fail
>> that register is not locked.
> 
> 
> The patch is correct.
> 
> 
>> In call_va function there was no implementation for passing i64
>> arguments. I also added a 64-bit version for passing pointer arguments.
>>
> 
> this is correct.

Can you also take a look at my changes in gen_args? Are they correct too?

>> Patch also sets lazy resolution mode as default.
>>
>> Now that I made these fixes some Java code is executed, but Hello World
>> doesn't work yet. The problem is with calling
>> invokevirtual/invokeinterface resolution helpers. They require "thiz"
>> argument as a pointer to the object. The "thiz" value is locked in the
>> register at the beginning of gen_invoke method and is unlocked only at
>> the end of it. But it so happens that "thiz" may be locked in some
>> register that is
>>
> required to pass arguments to the resolution helper because CallSig is
>> locked only before the helper is called. In this case the value of this
>> register is overwritten by the arguments of the helper because this
>> register is needed for arguments, and therefore the object argument of
>> the helper is corrupted.
>>
> 
> The lock of 'thiz' at the beginning of the method is an old optimization. It
> is not correct today.
> It does not related to lazy resolution mode and theoretically can cause a
> failure for CCONV_MANAGED calls too.
> IMO the best solution here is to move global rlock(thiz) into local places
> that require ''thiz' to be on a register.

I was going to do exactly this, but I am a but uncertain if the 
thiz_depth is the same throughout the whole body of the method. If it is 
the same, then locking of "thiz" can be easily moved into each path.

> Another problem here: in lazy resolution mode we need to 'vpark' all
> registers for CCONV_HELPERS calls too.

Hmm... I am not yet experienced with JIT well enough to understand this. 
  I have seen vpark calls in many places but I don't know what it does 
and why it is inserted. Could you please explain in a few words? BTW 
vpark is done at the beginning of the gen_invoke for the CallSig of the 
method.

>> I think it is necessary to split this method, it is quite complicated
>> and has too many branches to follow so that lazy resolution path and
>> normal call path are compiled independently. This will allow to lock
>> "thiz" after the helper signature is locked. But I am not very familiar
>> with JET, so I am not sure if reordering the locking is the only
>> possible way to fix this bug. Could someone from JIT team enlighten me?
>>
> 
> By splitting the method you have to repeat all the preparation/finalization
> for a call in both versions. It's about a half size of the current method.
> What do you think by moving the <do_smthX> from

You are right, it is probably not easy to split this method. I won't try 
it anyway because I don't want to make any intrusive changes in JIT yet.

> if(meth==NULL) {
>    <do_smth1>
> } else if (opcod == OPCODE_INVOKEINTERFACE) {
>   <do_smth2>
> }...
> 
> into separate methods?

This was my thought to separate lazy and non-lazy paths. I don't insist 
on this.

-- 
Gregory

Mime
View raw message