Bug 220718 – [JSC] FTL::prepareOSREntry can clear OSR entry CodeBlock if it is already invalidated

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Yusuke Suzuki]

[JSC] FTL::prepareOSREntry can clear OSR entry CodeBlock if it is already invalidated

[Yusuke Suzuki]

Created attachment 417853 [details]
Patch

[Yusuke Suzuki]

<rdar://problem/70527068>

[Yusuke Suzuki]

Created attachment 417854 [details]
Patch

[Mark Lam]

Comment on attachment 417854 [details]
Patch

r=mer

[EWS]

Committed r271596: <https://trac.webkit.org/changeset/271596>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 417854 [details].

[Michael Catanzaro]

This introduced:

[621/2299] Building CXX object Source/JavaScriptCore/CMak...criptCore/unified-sources/UnifiedSource-bfc896e1-12.cpp.o
In file included from DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-bfc896e1-12.cpp:5:
../../Source/JavaScriptCore/dfg/DFGOperations.cpp: In function ‘char* JSC::DFG::tierUpCommon(JSC::VM&, JSC::CallFrame*, JSC::BytecodeIndex, bool)’:
../../Source/JavaScriptCore/dfg/DFGOperations.cpp:3861:20: warning: unused variable ‘entryBlock’ [-Wunused-variable]
 3861 |     if (CodeBlock* entryBlock = jitCode->osrEntryBlock()) {
      |                    ^~~~~~~~~~

I'm not sure if this trivial fix would be correct:

diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
index 85e74d17ece6..45fdcb24f68c 100644
--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
@@ -3858,7 +3858,7 @@ static char* tierUpCommon(VM& vm, CallFrame* callFrame, BytecodeIndex originByte
     } else
         CODEBLOCK_LOG_EVENT(codeBlock, "delayFTLCompile", ("avoiding replacement compile"));
 
-    if (CodeBlock* entryBlock = jitCode->osrEntryBlock()) {
+    if (jitCode->osrEntryBlock()) {
         if (jitCode->osrEntryRetry < Options::ftlOSREntryRetryThreshold()) {
             CODEBLOCK_LOG_EVENT(codeBlock, "delayFTLCompile", ("OSR entry failed, OSR entry threshold not met"));
             jitCode->osrEntryRetry++;

Maybe that condition is no longer needed at all now?

[Yusuke Suzuki]

(In reply to Michael Catanzaro from comment #6)
> This introduced:
> 
> [621/2299] Building CXX object
> Source/JavaScriptCore/CMak...criptCore/unified-sources/UnifiedSource-
> bfc896e1-12.cpp.o
> In file included from
> DerivedSources/JavaScriptCore/unified-sources/UnifiedSource-bfc896e1-12.cpp:
> 5:
> ../../Source/JavaScriptCore/dfg/DFGOperations.cpp: In function ‘char*
> JSC::DFG::tierUpCommon(JSC::VM&, JSC::CallFrame*, JSC::BytecodeIndex, bool)’:
> ../../Source/JavaScriptCore/dfg/DFGOperations.cpp:3861:20: warning: unused
> variable ‘entryBlock’ [-Wunused-variable]
>  3861 |     if (CodeBlock* entryBlock = jitCode->osrEntryBlock()) {
>       |                    ^~~~~~~~~~
> 
> I'm not sure if this trivial fix would be correct:
> 
> diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp
> b/Source/JavaScriptCore/dfg/DFGOperations.cpp
> index 85e74d17ece6..45fdcb24f68c 100644
> --- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
> +++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
> @@ -3858,7 +3858,7 @@ static char* tierUpCommon(VM& vm, CallFrame*
> callFrame, BytecodeIndex originByte
>      } else
>          CODEBLOCK_LOG_EVENT(codeBlock, "delayFTLCompile", ("avoiding
> replacement compile"));
>  
> -    if (CodeBlock* entryBlock = jitCode->osrEntryBlock()) {
> +    if (jitCode->osrEntryBlock()) {
>          if (jitCode->osrEntryRetry < Options::ftlOSREntryRetryThreshold()) {
>              CODEBLOCK_LOG_EVENT(codeBlock, "delayFTLCompile", ("OSR entry
> failed, OSR entry threshold not met"));
>              jitCode->osrEntryRetry++;
> 
> Maybe that condition is no longer needed at all now?

That condition is required.

if (jitCode->osrEntryBlock()) {

is the right change. I'll land it as an unreviewed patch.

[Yusuke Suzuki]

Committed r271624: <https://trac.webkit.org/changeset/271624>