JIT Generator (and Async func) loops #6990
Conversation
|
I think it is a pretty good idea. |
|
I've moved all generator JIT test cases into a new folder and added additional test cases for testing loop body jit. Note, with this PR we now have: Default behaviour (no flag): do not Jit generator (or async functions) BUT do jit hot loops inside them that don't contain Flagged behaviour |
| @@ -118,7 +118,7 @@ IRBuilder::DoBailOnNoProfile() | |||
| return false; | |||
| } | |||
|
|
|||
| if (m_func->GetTopFunc()->GetJITFunctionBody()->IsCoroutine()) | |||
| if (m_func->GetTopFunc()->GetJITFunctionBody()->IsCoroutine() && !m_func->IsLoopBody()) | |||
There was a problem hiding this comment.
BailOnNoProfile caused problems when put in a loop containing a Yield; not a problem when jitting loop bodies that cannot contain yield.
| @@ -441,7 +441,7 @@ IRBuilder::Build() | |||
| // Note that for generators, we insert the bailout after the jump table to allow | |||
| // the generator's execution to proceed before bailing out. Otherwise, we would always | |||
| // bail to the beginning of the function in the interpreter, creating an infinite loop. | |||
| if (m_func->IsJitInDebugMode() && !this->m_func->GetJITFunctionBody()->IsCoroutine()) | |||
| if (m_func->IsJitInDebugMode() && (!this->m_func->GetJITFunctionBody()->IsCoroutine() || this->IsLoopBody())) | |||
There was a problem hiding this comment.
Different debug break point for Generator functions BUT not relevant for LoopBodies.
| @@ -5467,7 +5467,7 @@ Lowerer::LowerPrologEpilog() | |||
| instr = m_func->m_exitInstr; | |||
| AssertMsg(instr->IsExitInstr(), "Last instr isn't an ExitInstr..."); | |||
|
|
|||
| if (m_func->GetJITFunctionBody()->IsCoroutine()) | |||
| if (m_func->GetJITFunctionBody()->IsCoroutine() && !m_func->IsLoopBody()) | |||
There was a problem hiding this comment.
Generator function required custom Epilog, irrelevant for a loopBody.
| @@ -11544,7 +11545,7 @@ Lowerer::LowerArgIn(IR::Instr *instrArgIn) | |||
| if (argIndex == 1) | |||
| { | |||
| // The "this" argument is not source-dependent and doesn't need to be checked. | |||
| if (m_func->GetJITFunctionBody()->IsCoroutine()) | |||
| if (m_func->GetJITFunctionBody()->IsCoroutine() && !m_func->IsLoopBody()) | |||
There was a problem hiding this comment.
Generator function arguments loaded from a different location; not relevant for LoopBodies.
| <files>jit-gen-loop-body.js</files> | ||
| <compile-flags>-testtrace:Backend</compile-flags> | ||
| <baseline>jit-gen-loop-body.baseline</baseline> | ||
| <tags>exclude_test, exclude_nonative, exclude_dynapogo</tags> |
There was a problem hiding this comment.
Have to exclude test builds from this test and 2 more of the below as the -testrace:backend flag that allows verification of which loops were jitted only functions in a debug build.
Jitting of generator and async functions is buggy and has some potential performance cliffs. As an alternative this PR enables Jitting of loops that don't contain await or yield.
e.g. In this examples if the profiler found them to be hot loop 2 and loop 4 would be jitted:
Whilst jitting the whole function may in theory offer better performance it's not actually clear that it would in light of the overheads of yielding out of jitted code AND the problems yield brings to optimising jitted code.
This alternative is much simpler; hopefully far more stable AND should target the most optimisable parts of these functions.