...
Created attachment 403753 [details] patch
Comment on attachment 403753 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=403753&action=review r=me > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:14718 > + patchpoint->effects.writesLocalState = true; patchpoint->clobber(RegisterSet::macroScratchRegisters()); is required. > Source/JavaScriptCore/runtime/VM.cpp:1562 > + uint64_t* ptr = static_cast<uint64_t*>(fastMalloc(sizeof(uint64_t))); > + *ptr = 0; > + addResult.iterator->value.second = ptr; How about using `std::unique_ptr<uint64_t>` & `makeUnique<uint64_t>()`? > Source/JavaScriptCore/runtime/VM.cpp:1582 > + fastFree(iter->value.second); Then, we can remove this.
Comment on attachment 403753 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=403753&action=review >> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:14718 >> + patchpoint->effects.writesLocalState = true; > > patchpoint->clobber(RegisterSet::macroScratchRegisters()); is required. Actually I tried to not make it so that we use them. I added AllowMacroScratxh RAII before I refactored the code. I will remove the RAII >> Source/JavaScriptCore/runtime/VM.cpp:1562 >> + addResult.iterator->value.second = ptr; > > How about using `std::unique_ptr<uint64_t>` & `makeUnique<uint64_t>()`? Sounds good
Comment on attachment 403753 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=403753&action=review > Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:468 > + *ptr += codeBlock->llintExecuteCounter().m_activeThreshold; Why inc by m_activeThreshold here and only by 1 in the JITs?
Comment on attachment 403753 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=403753&action=review >> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:468 >> + *ptr += codeBlock->llintExecuteCounter().m_activeThreshold; > > Why inc by m_activeThreshold here and only by 1 in the JITs? this is just a heuristic of how much we've executed. This is actually slightly higher than what we've executed. It's because I don't want to change the LLInt's runtime code. But changing the JITs is trivial, since it's controlled behind a runtime option.
Created attachment 403757 [details] patch for landing
Comment on attachment 403753 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=403753&action=review >>> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:468 >>> + *ptr += codeBlock->llintExecuteCounter().m_activeThreshold; >> >> Why inc by m_activeThreshold here and only by 1 in the JITs? > > this is just a heuristic of how much we've executed. This is actually slightly higher than what we've executed. It's because I don't want to change the LLInt's runtime code. But changing the JITs is trivial, since it's controlled behind a runtime option. OK, I understand now. The detail to note is that this function (llint_loop_osr) is only called once after the loop_hint has been visited m_activeThreshold times. It is not called on every loop_hint.
Created attachment 403758 [details] patch for landing Now skipping this for builtin functions, since we rely on some of those not early returning out of loops for the integrity of data structures we create.
Committed r264105: <https://trac.webkit.org/changeset/264105> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403758 [details].
<rdar://problem/65227843>