Title: [258965] trunk
Revision
258965
Author
tzaga...@apple.com
Date
2020-03-24 17:51:44 -0700 (Tue, 24 Mar 2020)

Log Message

LLIntGenerator must link switch jumps to otherwise redundant labels
https://bugs.webkit.org/show_bug.cgi?id=209333
<rdar://problem/60827987>

Reviewed by Saam Barati.

JSTests:

* wasm/stress/terminal-jump-switch-target.js: Added.

Source/_javascript_Core:

The LLIntGenerator optimizes jumps at the end of blocks. It does so when a block ends, by checking if
the last instruction emitted was a jump, if it pointed to the end of the current block and if it was
the only jump that pointed there. If all those conditions are satisfied, the jump is removed and it's
not necessary to emit the label at the end of block, since the only jump that pointed to it was removed.
However, switches (br_table) are handled specially by the LLIntGenerator and therefore are not counted
in Label::unresolvedJumps, which was used to check whether we could skip emitting the label.
The end result is that we might skip linking a switch jump if it points to a block that ends with a jump.

* wasm/WasmLLIntGenerator.cpp:
(JSC::Wasm::LLIntGenerator::addEndToUnreachable):
(JSC::Wasm::LLIntGenerator::linkSwitchTargets):
(JSC::GenericLabel<Wasm::GeneratorTraits>::setLocation):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (258964 => 258965)


--- trunk/JSTests/ChangeLog	2020-03-25 00:27:56 UTC (rev 258964)
+++ trunk/JSTests/ChangeLog	2020-03-25 00:51:44 UTC (rev 258965)
@@ -1,3 +1,13 @@
+2020-03-24  Tadeu Zagallo  <tzaga...@apple.com>
+
+        LLIntGenerator must link switch jumps to otherwise redundant labels
+        https://bugs.webkit.org/show_bug.cgi?id=209333
+        <rdar://problem/60827987>
+
+        Reviewed by Saam Barati.
+
+        * wasm/stress/terminal-jump-switch-target.js: Added.
+
 2020-03-24  Alexey Shvayka  <shvaikal...@gmail.com>
 
         Update test262 to commit dfc7ecc6785e

Added: trunk/JSTests/wasm/stress/terminal-jump-switch-target.js (0 => 258965)


--- trunk/JSTests/wasm/stress/terminal-jump-switch-target.js	                        (rev 0)
+++ trunk/JSTests/wasm/stress/terminal-jump-switch-target.js	2020-03-25 00:51:44 UTC (rev 258965)
@@ -0,0 +1,19 @@
+/*
+(module
+  (type $0 (func))
+      (type $1 (func (result f64)))
+      (func $0 (type 0))
+      (func $1
+              (type 1)
+              (loop (result f64) (f64.const 0.0) (i32.const 0) (br_table 1) (call 0))
+              (br 0)
+              (unreachable)
+            )
+      (export "runf64" (func 1))
+)
+*/
+
+let buffer = new Uint8Array([ 0,97,115,109,1,0,0,0,1,136,128,128,128,0,2,96,0,0,96,0,1,124,3,131,128,128,128,0,2,0,1,7,138,128,128,128,0,1,6,114,117,110,102,54,52,0,1,10,165,128,128,128,0,2,130,128,128,128,0,0,11,152,128,128,128,0,0,3,124,68,0,0,0,0,0,0,0,0,65,0,14,0,1,16,0,11,12,0,0,11 ]);
+
+let m = new WebAssembly.Instance(new WebAssembly.Module(buffer));
+m.exports.runf64();

Modified: trunk/Source/_javascript_Core/ChangeLog (258964 => 258965)


--- trunk/Source/_javascript_Core/ChangeLog	2020-03-25 00:27:56 UTC (rev 258964)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-03-25 00:51:44 UTC (rev 258965)
@@ -1,3 +1,25 @@
+2020-03-24  Tadeu Zagallo  <tzaga...@apple.com>
+
+        LLIntGenerator must link switch jumps to otherwise redundant labels
+        https://bugs.webkit.org/show_bug.cgi?id=209333
+        <rdar://problem/60827987>
+
+        Reviewed by Saam Barati.
+
+        The LLIntGenerator optimizes jumps at the end of blocks. It does so when a block ends, by checking if
+        the last instruction emitted was a jump, if it pointed to the end of the current block and if it was
+        the only jump that pointed there. If all those conditions are satisfied, the jump is removed and it's
+        not necessary to emit the label at the end of block, since the only jump that pointed to it was removed.
+        However, switches (br_table) are handled specially by the LLIntGenerator and therefore are not counted
+        in Label::unresolvedJumps, which was used to check whether we could skip emitting the label.
+        The end result is that we might skip linking a switch jump if it points to a block that ends with a jump.
+
+
+        * wasm/WasmLLIntGenerator.cpp:
+        (JSC::Wasm::LLIntGenerator::addEndToUnreachable):
+        (JSC::Wasm::LLIntGenerator::linkSwitchTargets):
+        (JSC::GenericLabel<Wasm::GeneratorTraits>::setLocation):
+
 2020-03-24  Saam Barati  <sbar...@apple.com>
 
         Memory::fastMappedBytes() is wrong

Modified: trunk/Source/_javascript_Core/wasm/WasmLLIntGenerator.cpp (258964 => 258965)


--- trunk/Source/_javascript_Core/wasm/WasmLLIntGenerator.cpp	2020-03-25 00:27:56 UTC (rev 258964)
+++ trunk/Source/_javascript_Core/wasm/WasmLLIntGenerator.cpp	2020-03-25 00:51:44 UTC (rev 258965)
@@ -259,6 +259,7 @@
 
     LLIntCallInformation callInformationForCaller(const Signature&);
     Vector<VirtualRegister, 2> callInformationForCallee(const Signature&);
+    void linkSwitchTargets(Label&, unsigned location);
 
     VirtualRegister virtualRegisterForWasmLocal(uint32_t index)
     {
@@ -991,6 +992,7 @@
     }
 
     if (m_lastOpcodeID == wasm_jmp && data.m_continuation->unresolvedJumps().size() == 1 && data.m_continuation->unresolvedJumps()[0] == static_cast<int>(m_lastInstruction.offset())) {
+        linkSwitchTargets(*data.m_continuation, m_lastInstruction.offset());
         m_lastOpcodeID = wasm_unreachable;
         m_writer.rewind(m_lastInstruction);
     } else
@@ -1210,8 +1212,20 @@
     return { };
 }
 
+void LLIntGenerator::linkSwitchTargets(Label& label, unsigned location)
+{
+    auto it = m_switches.find(&label);
+    if (it != m_switches.end()) {
+        for (const auto& entry : it->value) {
+            ASSERT(!*entry.jumpTarget);
+            *entry.jumpTarget = location - entry.offset;
+        }
+        m_switches.remove(it);
+    }
 }
 
+}
+
 template<>
 void GenericLabel<Wasm::GeneratorTraits>::setLocation(BytecodeGeneratorBase<Wasm::GeneratorTraits>& generator, unsigned location)
 {
@@ -1220,17 +1234,8 @@
     m_location = location;
 
     Wasm::LLIntGenerator* llintGenerator = static_cast<Wasm::LLIntGenerator*>(&generator);
+    llintGenerator->linkSwitchTargets(*this, m_location);
 
-    auto it = llintGenerator->m_switches.find(this);
-    if (it != llintGenerator->m_switches.end()) {
-        for (const auto& entry : it->value) {
-            ASSERT(!*entry.jumpTarget);
-            *entry.jumpTarget = m_location - entry.offset;
-        }
-        llintGenerator->m_switches.remove(it);
-    }
-
-
     for (auto offset : m_unresolvedJumps) {
         auto instruction = generator.m_writer.ref(offset);
         int target = m_location - offset;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to