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 <[EMAIL PROTECTED]> 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
