harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "chunrong lai" <chunrong...@gmail.com>
Subject Re: Enable the MACRO _DEBUG_CHECK_NULL_
Date Wed, 12 Nov 2008 16:03:42 GMT
Ah. Thanks for the reply.
So you propose to change the rule (2) of MIkhail from
 (2) JIT in LIR can replace checknull with hardware exception check only if
'checknull' op is not in a try-catch block.
to
 (2)* JIT in LIR can replace checknull with hardware exception check only if
'checknull' op is not in a try-catch block, include the try-catch block of
any level of caller (require unwinding check).

With the enhancement I see that the chknull will not be removed/replaced as
observed.
Please correct me if I misunderstand you.
If so Mihail's judgement of "void sync() is not inlined" is irrelevant
because we always need to do the unwinding check.
Mikhail, what is your advice?

Anyway this modification looks not a minor change to me.
On Wed, Nov 12, 2008 at 11:34 PM, bu qi cheng <buqi.cheng@gmail.com> wrote:

> On Wed, Nov 12, 2008 at 10:48 PM, chunrong lai <chunronglai@gmail.com
> >wrote:
>
> > hi, Buqi:
> >     I am not very convinced to add some MonitorEnter specific code in
> > IRBuilder Code. Why only MonitorEnter is impacted here?
>
>
> Currently, we only know that moniorEnter and monitorExit have no null
> pointer check code.
> An assumption is that if MonitorExit is excuted, there will alway be a
> MonitorEnter before it. I think java verfier will do this checking. Please
> double check if it's right.
>
> >
> >
> >    We can see that the NPE is from the removal of two checknull
> operations.
> >    If we target to the inproper removal of the first checknull from JIT.
> > The modification or the optimization rule should be MonitorEnter
> > independent. So I do not see MonitorEnter related issue in Mikhail's
> > description.
>
>
> It seems for monitorEnter/monitorExit, we can not do in this way. Because
> hardware segment fault can not replace the null pointer check. Especially
> for the test case.
> Currently, the implementation of HARMONY is that if the check is not
> catched
> by any code of current, or caller(after inline), it will be eliminated. I
> don't think it's right for all situation.
>
>
> >
> >    If we target to the removal of the second checknull in the
> stub/helper.
> > The modification is MonitorEnter dependent. It is clear that the removal
> is
> > due to r659128 who changed the helpers. (It is a little strange to only
> > change the IRBuilder::genMonitorEnter, with the knowledge of no null
> > checking code in the stub/helpers).
>
>
> r659128 exposed more optimization chance for the JIT.
> HARMONY did other work to make sure that the check is right. Such as, if we
> pass a NULL object by arguement, HARMONY will make the chknull always work.
> It's just like what we are doing.
>
>
> >
> >   I myself am a little confused about the hardware exception check. What
> is
> > the background, expected behavior and triggering condition? (Mikhail only
> > mentioned that the optimization happens only if some conditions, does it
> > always heppen if so?) Maybe we can just give up the hardware exception
> > check.
>
>
> I think hardware exception only work when there is no catcher in all
> callers. Or the caller can catch all exceptions. So, when doing so, I think
> the unwind checking for exception handler tables in all callers' is needed.
>
>
> >
> >  On Wed, Nov 12, 2008 at 10:20 PM, bu qi cheng <buqi.cheng@gmail.com>
> > wrote:
> >
> > > Hi Mikhail:
> > >
> > >     One question is that if Harmony did NULL Pointer Check Elimination?
> I
> > > am not sure if 1) is done.
> > >    For this problem caused by 2). A patch is like following:
> > >
> > >
> > >
> > > IRBuilder.cpp
> > >
> > > void
> > >
> > > IRBuilder::genMonitorEnter(Opnd* src) {
> > >
> > >    src = propagateCopy(src);
> > >
> > >    Opnd *tauNullChecked = genTauCheckNull(src);
> > >
> > > *    Inst* inst = tauNullChecked->getInst();               //Added*
> > >
> > > *     if (inst->getOpcode() == Op_TauCheckNull)            //Added*
> > >
> > > *         inst->setDefArgModifier(NonNullThisArg);         //Added*
> > >
> > >    appendInst(instFactory->makeTauMonitorEnter(src, tauNullChecked));
> > >
> > > }
> > >
> > > By set the modifier of TauCheckNull instruction before the MonitorEnter
> > > instruction. If the "chknull" is not optimized by 1). HARMONY will
> always
> > > generate the LIR check code for it.
> > > With this way, there is no need to generate two version helper code for
> > > monitorEnter.
> > >
> > > Please double check if it's right.
> > >
> > >
> > > Thanks!
> > >
> > > Buqi
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message