backedge checks

[Gary Benson]

Edward Nevill wrote:
> In bytecodeInterpreter.c is this wonderful macro
> 
> #define DO_BACKEDGE_CHECKS(skip, branch_pc)\
>     if ((skip) <= 0) {\
>       if (UseCompiler && UseLoopCounter) {\
>         bool do_OSR = UseOnStackReplacement;\
>         BACKEDGE_COUNT->increment();\
>         if (do_OSR) do_OSR = BACKEDGE_COUNT->reached_InvocationLimit();\
>         if (do_OSR) {\
>           nmethod*  osr_nmethod;\
>           OSR_REQUEST(osr_nmethod, branch_pc);\
>           if (osr_nmethod != NULL && osr_nmethod->osr_entry_bci() !=InvalidOSREntryBci) {          \
>             intptr_t* buf;\
>             CALL_VM(buf=SharedRuntime::OSR_migration_begin(THREAD), handle_exception);              \
>             istate->set_msg(do_osr);\
>             istate->set_osr_buf((address)buf);\
>             istate->set_osr_entry(osr_nmethod->osr_entry());\
>             return;\
>           }\
>         } else {\
>           INCR_INVOCATION_COUNT;\
>           SAFEPOINT;\
>         }\
>       }  /* UseCompiler ... */\
>       INCR_INVOCATION_COUNT;\
>       SAFEPOINT;\
>     }
> 
> 
> This macro is invoked in every branch (although the body is only
> executed on backwards branches).
> 
> Firstly, is it just me, or does it do INCR_INVOCATION_COUNT;
> SAFEPOINT twice on every backwards branch. Surely a mistake?

It looks that way.  I'll take it out...

> Secondly, can someone tell me under what circumstances in zero
> UseCompiler would ever be true (and no, it doesn't resolve to a
> constant.

It's true whenever you're using Shark.  Shark doesn't do OSR at the
moment though, so most of that macro is unentered.

> Thirdly, it should be possible to avoid the SAFEPOINT
> checks. SAFEPOINT does...
> 
> #define SAFEPOINT \
>     if ( SafepointSynchronize::is_synchronizing()) { \
>         { \
>           /* zap freed handles rather than GC'ing them */ \
>           HandleMarkCleaner __hmc(THREAD); \
>         } \
>         CALL_VM(SafepointSynchronize::block(THREAD), handle_exception); \
>     }
> 
> However, there is an upcall notice_safepoints() which safepoint.cpp
> calls whenever it is setting is_synchronizing(). This upcall is
> there to notify the interpreter that it needs to run to a GC
> safepoint. A corresponding call ignore_safepoints() is called when
> the interpreter can safely ignore safepoints. So there should be no
> need for the interpreter to continually check for safepoints.
> 
> notice_safepoints() and ignore_safepoints() in the template
> interpreter do indeed do something sensible. However, in the
> bytecode Interpreter the implementation is just {}

Sadly the one person who could answer that question categorically is
no longer with us, so it'll be trial and error.  If you make a patch
I can run it past the TCK for you, if you like...

> Finally, can someone enlighten me as to the purpose of
> INCR_INVOCATION_COUNT, if we can get rid of that we could get rid of
> all backedge checks.

It's how methods are selected for compilation, when you're running
with a JIT.  Methods are considered for compilation when their
invocation count passes a certain threshold.

Cheers,
Gary

-- 
http://gbenson.net/