Changeset 258965 in webkit

Timestamp:

Mar 24, 2020, 5:51:44 PM (6 years ago)

Author:

Tadeu Zagallo

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/JavaScriptCore:

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):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/wasm/stress/terminal-jump-switch-target.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp (modified) (4 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2020-03-24  Tadeu Zagallo  <tzagallo@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  <shvaikalesh@gmail.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-03-24  Tadeu Zagallo  <tzagallo@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  <sbarati@apple.com>
 

trunk/Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp

@@ -260 +260 @@
     LLIntCallInformation callInformationForCaller(const Signature&);
     Vector<VirtualRegister, 2> callInformationForCallee(const Signature&);
+    void linkSwitchTargets(Label&, unsigned location);
 
     VirtualRegister virtualRegisterForWasmLocal(uint32_t index)
@@ -992 +993 @@
 
     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);
@@ -1211 +1213 @@
 }
 
+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);
+    }
+}
+
 }
 
@@ -1221 +1235 @@
 
     Wasm::LLIntGenerator* llintGenerator = static_cast<Wasm::LLIntGenerator*>(&generator);
-
-    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);
-    }
-
+    llintGenerator->linkSwitchTargets(*this, m_location);
 
     for (auto offset : m_unresolvedJumps) {