AFAIR checknull is always precedes an operation that can cause exception if
one of its arguments is NULL. Since this operation may have side effects -
it can't be removed nor moved into try/cactch block by optimizer.
In our case if the operation is a helper-call - it must fit this scheme: do
manual nullcheck.

There should not be any performance problem here: if the helper is hot ->
Jitrino.OPT will inline it using magics and eliminate the check.
So I see no problems enabling null-check inside of the VM helper.

-- 
Mikhail Fursov


On Wed, Nov 12, 2008 at 10:03 PM, chunrong lai <chunronglai@gmail.com>wrote:

> 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
> > > >
> > >
> >
>