Attachment 220943 Details for Bug 126822 – the patch

[patch] the patch

blah.patch (text/plain), 5.96 KB, created by Filip Pizlo on 2014-01-11 12:14:45 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2014-01-11 12:14:45 PST

Size: 5.96 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 161775)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,24 @@
>+2014-01-11  Filip Pizlo  <fpizlo@apple.com>
>+
>+        FTL tier-up should behave the same with CountExecution's as it does without
>+        https://bugs.webkit.org/show_bug.cgi?id=126822
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        CountExecution nodes are inserted by the profiler. Prior to this change, their
>+        presence would change how we inserted CheckTierUp nodes, since a CountExecution
>+        might be inserted before a LoopHint thereby making it so the LoopHint was no
>+        longer at the top of a loop header. This makes the code resilient against such
>+        things. The result is that when you enable the profiler, you get the same OSR
>+        behavior as with the profiler disabled.
>+
>+        * dfg/DFGNode.h:
>+        (JSC::DFG::Node::isSemanticallySkippable):
>+        * dfg/DFGOSREntrypointCreationPhase.cpp:
>+        (JSC::DFG::OSREntrypointCreationPhase::run):
>+        * dfg/DFGTierUpCheckInjectionPhase.cpp:
>+        (JSC::DFG::TierUpCheckInjectionPhase::run):
>+
> 2014-01-10  Filip Pizlo  <fpizlo@apple.com>
> 
>         FTL should work with the bytecode profiler
>Index: Source/JavaScriptCore/dfg/DFGNode.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGNode.h	(revision 161738)
>+++ Source/JavaScriptCore/dfg/DFGNode.h	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2011, 2012, 2013 Apple Inc. All rights reserved.
>+ * Copyright (C) 2011, 2012, 2013, 2014 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -1186,6 +1186,11 @@ struct Node {
>             return shouldGenerate();
>         }
>     }
>+    
>+    bool isSemanticallySkippable()
>+    {
>+        return op() == CountExecution;
>+    }
> 
>     unsigned refCount()
>     {
>Index: Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp	(revision 161738)
>+++ Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2013 Apple Inc. All rights reserved.
>+ * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -63,7 +63,10 @@ public:
>             BasicBlock* block = m_graph.block(blockIndex);
>             if (!block)
>                 continue;
>+            unsigned nodeIndex = 0;
>             Node* firstNode = block->at(0);
>+            while (firstNode->isSemanticallySkippable())
>+                firstNode = block->at(++nodeIndex);
>             if (firstNode->op() == LoopHint
>                 && firstNode->codeOrigin == CodeOrigin(bytecodeIndex)) {
>                 target = block;
>Index: Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp	(revision 161738)
>+++ Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp	(working copy)
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2013 Apple Inc. All rights reserved.
>+ * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
>  *
>  * Redistribution and use in source and binary forms, with or without
>  * modification, are permitted provided that the following conditions
>@@ -61,15 +61,41 @@ public:
>             if (!block)
>                 continue;
>             
>-            if (block->at(0)->op() == LoopHint) {
>-                CodeOrigin codeOrigin = block->at(0)->codeOrigin;
>-                NodeType nodeType;
>-                if (level == FTL::CanCompileAndOSREnter && !codeOrigin.inlineCallFrame) {
>-                    nodeType = CheckTierUpAndOSREnter;
>-                    RELEASE_ASSERT(block->bytecodeBegin == codeOrigin.bytecodeIndex);
>-                } else
>-                    nodeType = CheckTierUpInLoop;
>-                insertionSet.insertNode(1, SpecNone, nodeType, codeOrigin);
>+            for (unsigned nodeIndex = 0; nodeIndex < block->size(); ++nodeIndex) {
>+                Node* node = block->at(nodeIndex);
>+                if (node->op() != LoopHint)
>+                    continue;
>+                
>+                // We only put OSR checks for the first LoopHint in the block. Note that
>+                // more than one LoopHint could happen in cases where we did a lot of CFG
>+                // simplification in the bytecode parser, but it should be very rare.
>+                
>+                CodeOrigin codeOrigin = node->codeOrigin;
>+                
>+                if (level != FTL::CanCompileAndOSREnter || codeOrigin.inlineCallFrame) {
>+                    insertionSet.insertNode(
>+                        nodeIndex + 1, SpecNone, CheckTierUpInLoop, codeOrigin);
>+                    break;
>+                }
>+                
>+                bool isAtTop = true;
>+                for (unsigned subNodeIndex = nodeIndex; subNodeIndex--;) {
>+                    if (!block->at(subNodeIndex)->isSemanticallySkippable()) {
>+                        isAtTop = false;
>+                        break;
>+                    }
>+                }
>+                
>+                if (!isAtTop) {
>+                    insertionSet.insertNode(
>+                        nodeIndex + 1, SpecNone, CheckTierUpInLoop, codeOrigin);
>+                    break;
>+                }
>+                
>+                RELEASE_ASSERT(block->bytecodeBegin == codeOrigin.bytecodeIndex);
>+                insertionSet.insertNode(
>+                    nodeIndex + 1, SpecNone, CheckTierUpAndOSREnter, codeOrigin);
>+                break;
>             }
>             
>             if (block->last()->op() == Return) {

Flags:

sam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 126822: 220943