The DFG CFGSimplification phase shouldn’t jettison a block when it’s the target of...
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 Aug 2018 16:55:19 +0000 (16:55 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 22 Aug 2018 16:55:19 +0000 (16:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188298
<rdar://problem/42888427>

Reviewed by Saam Barati.

JSTests:

* stress/bug-188298.js: Added.

Source/JavaScriptCore:

In the event that both targets of a Branch is the same block, then even if we'll
always take one path of the branch, the other target is not unreachable because
it is the same target as the one in the taken path.  Hence, it should not be
jettisoned.

* JavaScriptCore.xcodeproj/project.pbxproj:
- Added DFGCFG.h which is in use and should have been added to the project.
* dfg/DFGCFGSimplificationPhase.cpp:
(JSC::DFG::CFGSimplificationPhase::run):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@235177 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/bug-188298.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/dfg/DFGCFGSimplificationPhase.cpp

index 6455ff4..9612fcb 100644 (file)
@@ -1,3 +1,13 @@
+2018-08-22  Mark Lam  <mark.lam@apple.com>
+
+        The DFG CFGSimplification phase shouldn’t jettison a block when it’s the target of both branch directions.
+        https://bugs.webkit.org/show_bug.cgi?id=188298
+        <rdar://problem/42888427>
+
+        Reviewed by Saam Barati.
+
+        * stress/bug-188298.js: Added.
+
 2018-08-20  Saam barati  <sbarati@apple.com>
 
         Inline DataView accesses into DFG/FTL
diff --git a/JSTests/stress/bug-188298.js b/JSTests/stress/bug-188298.js
new file mode 100644 (file)
index 0000000..6691bff
--- /dev/null
@@ -0,0 +1,12 @@
+// This test passes if it does not crash.
+
+function foo() {
+    if (1 < 2);
+    while (true) {
+        if (1 < 2) break;
+    }
+}
+
+for (var i = 0; i < 10000; i++)
+    foo();
+
index bfd244a..f34aef4 100644 (file)
@@ -1,3 +1,21 @@
+2018-08-22  Mark Lam  <mark.lam@apple.com>
+
+        The DFG CFGSimplification phase shouldn’t jettison a block when it’s the target of both branch directions.
+        https://bugs.webkit.org/show_bug.cgi?id=188298
+        <rdar://problem/42888427>
+
+        Reviewed by Saam Barati.
+
+        In the event that both targets of a Branch is the same block, then even if we'll
+        always take one path of the branch, the other target is not unreachable because
+        it is the same target as the one in the taken path.  Hence, it should not be
+        jettisoned.
+
+        * JavaScriptCore.xcodeproj/project.pbxproj:
+        - Added DFGCFG.h which is in use and should have been added to the project.
+        * dfg/DFGCFGSimplificationPhase.cpp:
+        (JSC::DFG::CFGSimplificationPhase::run):
+
 2018-08-20  Yusuke Suzuki  <yusukesuzuki@slowstart.org>
 
         [JSC] HeapUtil should care about pointer overflow
index 6bb35ca..17a48bd 100644 (file)
                FEA08620182B7A0400F6D851 /* Breakpoint.h in Headers */ = {isa = PBXBuildFile; fileRef = FEA0861E182B7A0400F6D851 /* Breakpoint.h */; settings = {ATTRIBUTES = (Private, ); }; };
                FEA08621182B7A0400F6D851 /* DebuggerPrimitives.h in Headers */ = {isa = PBXBuildFile; fileRef = FEA0861F182B7A0400F6D851 /* DebuggerPrimitives.h */; settings = {ATTRIBUTES = (Private, ); }; };
                FEA0C4031CDD7D1D00481991 /* FunctionWhitelist.h in Headers */ = {isa = PBXBuildFile; fileRef = FEA0C4011CDD7D0E00481991 /* FunctionWhitelist.h */; };
+               FEA3BBAC212C97CB00E93AD1 /* DFGCFG.h in Headers */ = {isa = PBXBuildFile; fileRef = FEA3BBAB212C97CB00E93AD1 /* DFGCFG.h */; };
                FEB51F6C1A97B688001F921C /* Regress141809.mm in Sources */ = {isa = PBXBuildFile; fileRef = FEB51F6B1A97B688001F921C /* Regress141809.mm */; };
                FEB58C15187B8B160098EF0B /* ErrorHandlingScope.h in Headers */ = {isa = PBXBuildFile; fileRef = FEB58C13187B8B160098EF0B /* ErrorHandlingScope.h */; settings = {ATTRIBUTES = (Private, ); }; };
                FECB8B271D25BB85006F2463 /* FunctionOverridesTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FECB8B251D25BB6E006F2463 /* FunctionOverridesTest.cpp */; };
                FEA0861F182B7A0400F6D851 /* DebuggerPrimitives.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DebuggerPrimitives.h; sourceTree = "<group>"; };
                FEA0C4001CDD7D0E00481991 /* FunctionWhitelist.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = FunctionWhitelist.cpp; sourceTree = "<group>"; };
                FEA0C4011CDD7D0E00481991 /* FunctionWhitelist.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FunctionWhitelist.h; sourceTree = "<group>"; };
+               FEA3BBAB212C97CB00E93AD1 /* DFGCFG.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = DFGCFG.h; path = dfg/DFGCFG.h; sourceTree = "<group>"; };
                FEB137561BB11EEE00CD5100 /* MacroAssemblerARM64.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MacroAssemblerARM64.cpp; sourceTree = "<group>"; };
                FEB41CCB1F73284200C5481E /* ProbeFrame.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ProbeFrame.h; sourceTree = "<group>"; };
                FEB51F6A1A97B688001F921C /* Regress141809.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = Regress141809.h; path = API/tests/Regress141809.h; sourceTree = "<group>"; };
                                0FD82E1F14172C2F00179C94 /* DFGCapabilities.h */,
                                0FFFC94B14EF909500C72532 /* DFGCFAPhase.cpp */,
                                0FFFC94C14EF909500C72532 /* DFGCFAPhase.h */,
+                               FEA3BBAB212C97CB00E93AD1 /* DFGCFG.h */,
                                0F3B3A241544C991003ED0FF /* DFGCFGSimplificationPhase.cpp */,
                                0F3B3A251544C991003ED0FF /* DFGCFGSimplificationPhase.h */,
                                0F9D36921AE9CC33000D4DFB /* DFGCleanUpPhase.cpp */,
                                C2FCAE1317A9C24E0034C735 /* BytecodeLivenessAnalysis.h in Headers */,
                                0F666EC0183566F900D017F1 /* BytecodeLivenessAnalysisInlines.h in Headers */,
                                E328DAEB1D38D005001A2529 /* BytecodeRewriter.h in Headers */,
+                               FEA3BBAC212C97CB00E93AD1 /* DFGCFG.h in Headers */,
                                6514F21918B3E1670098FF8B /* Bytecodes.h in Headers */,
                                0F885E111849A3BE00F1E3FA /* BytecodeUseDef.h in Headers */,
                                0F8023EA1613832B00A0BA45 /* ByValInfo.h in Headers */,
index 6e897f5..7783527 100644 (file)
@@ -106,7 +106,10 @@ public:
                             if (extremeLogging)
                                 m_graph.dump();
                             m_graph.dethread();
-                            mergeBlocks(block, targetBlock, oneBlock(jettisonedBlock));
+                            if (targetBlock == jettisonedBlock)
+                                mergeBlocks(block, targetBlock, noBlocks());
+                            else
+                                mergeBlocks(block, targetBlock, oneBlock(jettisonedBlock));
                         } else {
                             if (extremeLogging)
                                 m_graph.dump();
@@ -116,7 +119,8 @@ public:
                             ASSERT(terminal->isTerminal());
                             NodeOrigin boundaryNodeOrigin = terminal->origin;
 
-                            jettisonBlock(block, jettisonedBlock, boundaryNodeOrigin);
+                            if (targetBlock != jettisonedBlock)
+                                jettisonBlock(block, jettisonedBlock, boundaryNodeOrigin);
 
                             block->replaceTerminal(
                                 m_graph, SpecNone, Jump, boundaryNodeOrigin,
@@ -134,7 +138,7 @@ public:
                         innerChanged = outerChanged = true;
                         break;
                     }
-                    
+
                     // Branch to same destination -> jump.
                     // FIXME: this will currently not be hit because of the lack of jump-only
                     // block simplification.