Changeset 251978 in webkit

Timestamp:

Nov 3, 2019 8:11:43 PM (4 years ago)

Author:

Tadeu Zagallo

Message:

LLIntGenerator should not allocate temporaries in between variables
​https://bugs.webkit.org/show_bug.cgi?id=203787

Reviewed by Yusuke Suzuki.

JSTests:

• wasm/stress/local-ref.js: Added.

Source/JavaScriptCore:

The BytecodeGenerator requires that all variables must be allocated contiguously, before any
temporaries are allocated. Currently, we might end up allocating a temporary to materialize
the null constant to initialize locals of type Anyref/Funcref. Fix it by keeping track of the
locals that need to be initialized and adding a new callback to notify when we have finished
parsing locals. Only then we perform the delayed initialization of local refs.

• wasm/WasmAirIRGenerator.cpp:

(JSC::Wasm::AirIRGenerator::didFinishParsingLocals):

• wasm/WasmB3IRGenerator.cpp:

(JSC::Wasm::B3IRGenerator::didFinishParsingLocals):

• wasm/WasmFunctionParser.h:

(JSC::Wasm::FunctionParser<Context>::parse):

• wasm/WasmLLIntGenerator.cpp:

(JSC::Wasm::LLIntGenerator::addLocal):
(JSC::Wasm::LLIntGenerator::didFinishParsingLocals):

• wasm/WasmValidate.cpp:

(JSC::Wasm::Validate::didFinishParsingLocals):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/wasm/stress/local-ref.js (added)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp (modified) (1 diff)
• Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp (modified) (1 diff)
• Source/JavaScriptCore/wasm/WasmFunctionParser.h (modified) (1 diff)
• Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp (modified) (4 diffs)
• Source/JavaScriptCore/wasm/WasmValidate.cpp (modified) (1 diff)

trunk/JSTests/ChangeLog

@@ +1 @@
+2019-11-03  Tadeu Zagallo  <tzagallo@apple.com>
+
+        LLIntGenerator should not allocate temporaries in between variables
+        https://bugs.webkit.org/show_bug.cgi?id=203787
+
+        Reviewed by Yusuke Suzuki.
+
+        * wasm/stress/local-ref.js: Added.
+
 2019-11-02  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-11-03  Tadeu Zagallo  <tzagallo@apple.com>
+
+        LLIntGenerator should not allocate temporaries in between variables
+        https://bugs.webkit.org/show_bug.cgi?id=203787
+
+        Reviewed by Yusuke Suzuki.
+
+        The BytecodeGenerator requires that all variables must be allocated contiguously, before any
+        temporaries are allocated. Currently, we might end up allocating a temporary to materialize
+        the null constant to initialize locals of type Anyref/Funcref. Fix it by keeping track of the
+        locals that need to be initialized and adding a new callback to notify when we have finished
+        parsing locals. Only then we perform the delayed initialization of local refs.
+
+        * wasm/WasmAirIRGenerator.cpp:
+        (JSC::Wasm::AirIRGenerator::didFinishParsingLocals):
+        * wasm/WasmB3IRGenerator.cpp:
+        (JSC::Wasm::B3IRGenerator::didFinishParsingLocals):
+        * wasm/WasmFunctionParser.h:
+        (JSC::Wasm::FunctionParser<Context>::parse):
+        * wasm/WasmLLIntGenerator.cpp:
+        (JSC::Wasm::LLIntGenerator::addLocal):
+        (JSC::Wasm::LLIntGenerator::didFinishParsingLocals):
+        * wasm/WasmValidate.cpp:
+        (JSC::Wasm::Validate::didFinishParsingLocals):
+
 2019-11-02  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp

@@ -299 +299 @@
     void dump(const Vector<ControlEntry>& controlStack, const Stack* expressionStack);
     void setParser(FunctionParser<AirIRGenerator>* parser) { m_parser = parser; };
+    void didFinishParsingLocals() { }
 
     static Vector<Tmp> toTmpVector(const Vector<TypedTmp>& vector)

trunk/Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp

@@ -244 +244 @@
     void dump(const Vector<ControlEntry>& controlStack, const Stack* expressionStack);
     void setParser(FunctionParser<B3IRGenerator>* parser) { m_parser = parser; };
+    void didFinishParsingLocals() { }
 
     Value* constant(B3::Type, uint64_t bits, Optional<Origin> = WTF::nullopt);

trunk/Source/JavaScriptCore/wasm/WasmFunctionParser.h

@@ -146 +146 @@
         WASM_TRY_ADD_TO_CONTEXT(addLocal(typeOfLocal, numberOfLocals));
     }
+
+    m_context.didFinishParsingLocals();
 
     WASM_FAIL_IF_HELPER_FAILS(parseBody());

trunk/Source/JavaScriptCore/wasm/WasmLLIntGenerator.cpp

@@ -208 +208 @@
     PartialResult WARN_UNUSED_RETURN addUnreachable();
 
+    void didFinishParsingLocals();
+
     void setParser(FunctionParser<LLIntGenerator>* parser) { m_parser = parser; };
 
@@ -273 +275 @@
     HashMap<Label*, Vector<SwitchEntry>> m_switches;
     ExpressionType m_jsNullConstant;
+    ExpressionList m_unitializedLocals;
 };
 
@@ -485 +488 @@
         case Type::Anyref:
         case Type::Funcref:
-            WasmMov::emit(this, local, jsNullConstant());
+            m_unitializedLocals.append(local);
             break;
         default:
@@ -492 +495 @@
     }
     return { };
+}
+
+void LLIntGenerator::didFinishParsingLocals()
+{
+    auto null = jsNullConstant();
+    for (auto local : m_unitializedLocals)
+        WasmMov::emit(this, local, null);
+    m_unitializedLocals.clear();
 }
 

trunk/Source/JavaScriptCore/wasm/WasmValidate.cpp

@@ -176 +176 @@
     void dump(const Vector<ControlEntry>&, const Stack*);
     void setParser(FunctionParser<Validate>*) { }
+    void didFinishParsingLocals() { }
 
 private: