TypedArrays and Wasm should use index masking.
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jan 2018 22:02:31 +0000 (22:02 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 5 Jan 2018 22:02:31 +0000 (22:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181313

Reviewed by Michael Saboff.

Source/JavaScriptCore:

We should have index masking for our TypedArray code in the
DFG/FTL and for Wasm when doing bounds checking. Index masking for
Wasm is added to the WasmBoundsCheckValue. Since we don't CSE any
WasmBoundsCheckValues we don't need to worry about combining a
bounds check for a load and a store. I went with fusing the
pointer masking in the WasmBoundsCheckValue since it should reduce
additional compiler overhead.

* b3/B3LowerToAir.cpp:
* b3/B3Validate.cpp:
* b3/B3WasmBoundsCheckValue.cpp:
(JSC::B3::WasmBoundsCheckValue::WasmBoundsCheckValue):
(JSC::B3::WasmBoundsCheckValue::dumpMeta const):
* b3/B3WasmBoundsCheckValue.h:
(JSC::B3::WasmBoundsCheckValue::pinnedIndexingMask const):
* b3/air/AirCustom.h:
(JSC::B3::Air::WasmBoundsCheckCustom::generate):
* b3/testb3.cpp:
(JSC::B3::testWasmBoundsCheck):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::emitAllocateRawObject):
(JSC::DFG::SpeculativeJIT::loadFromIntTypedArray):
(JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray):
(JSC::DFG::SpeculativeJIT::compileGetByValOnFloatTypedArray):
(JSC::DFG::SpeculativeJIT::compileNewTypedArrayWithSize):
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileAtomicsReadModifyWrite):
(JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
(JSC::FTL::DFG::LowerDFGToB3::compileNewTypedArray):
(JSC::FTL::DFG::LowerDFGToB3::pointerIntoTypedArray):
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emitIntTypedArrayGetByVal):
* runtime/Butterfly.h:
(JSC::Butterfly::computeIndexingMask const):
(JSC::Butterfly::computeIndexingMaskForVectorLength): Deleted.
* runtime/JSArrayBufferView.cpp:
(JSC::JSArrayBufferView::JSArrayBufferView):
* wasm/WasmB3IRGenerator.cpp:
(JSC::Wasm::B3IRGenerator::B3IRGenerator):
(JSC::Wasm::B3IRGenerator::restoreWebAssemblyGlobalState):
(JSC::Wasm::B3IRGenerator::emitCheckAndPreparePointer):
(JSC::Wasm::B3IRGenerator::load):
(JSC::Wasm::B3IRGenerator::store):
(JSC::Wasm::B3IRGenerator::addCallIndirect):
* wasm/WasmBinding.cpp:
(JSC::Wasm::wasmToWasm):
* wasm/WasmMemory.cpp:
(JSC::Wasm::Memory::Memory):
(JSC::Wasm::Memory::grow):
* wasm/WasmMemory.h:
(JSC::Wasm::Memory::offsetOfIndexingMask):
* wasm/WasmMemoryInformation.cpp:
(JSC::Wasm::PinnedRegisterInfo::get):
(JSC::Wasm::PinnedRegisterInfo::PinnedRegisterInfo):
* wasm/WasmMemoryInformation.h:
(JSC::Wasm::PinnedRegisterInfo::toSave const):
* wasm/js/JSToWasm.cpp:
(JSC::Wasm::createJSToWasmWrapper):

Source/WTF:

* wtf/MathExtras.h:
(WTF::computeIndexingMask):

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

24 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/b3/B3LowerToAir.cpp
Source/JavaScriptCore/b3/B3Validate.cpp
Source/JavaScriptCore/b3/B3WasmBoundsCheckValue.cpp
Source/JavaScriptCore/b3/B3WasmBoundsCheckValue.h
Source/JavaScriptCore/b3/air/AirCustom.h
Source/JavaScriptCore/b3/testb3.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/jit/JITPropertyAccess.cpp
Source/JavaScriptCore/runtime/Butterfly.h
Source/JavaScriptCore/runtime/JSArrayBufferView.cpp
Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h
Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp
Source/JavaScriptCore/wasm/WasmBinding.cpp
Source/JavaScriptCore/wasm/WasmMemory.cpp
Source/JavaScriptCore/wasm/WasmMemory.h
Source/JavaScriptCore/wasm/WasmMemoryInformation.cpp
Source/JavaScriptCore/wasm/WasmMemoryInformation.h
Source/JavaScriptCore/wasm/js/JSToWasm.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/MathExtras.h

index 1d62708..6614c04 100644 (file)
@@ -1,3 +1,72 @@
+2018-01-04  Keith Miller  <keith_miller@apple.com>
+
+        TypedArrays and Wasm should use index masking.
+        https://bugs.webkit.org/show_bug.cgi?id=181313
+
+        Reviewed by Michael Saboff.
+
+        We should have index masking for our TypedArray code in the
+        DFG/FTL and for Wasm when doing bounds checking. Index masking for
+        Wasm is added to the WasmBoundsCheckValue. Since we don't CSE any
+        WasmBoundsCheckValues we don't need to worry about combining a
+        bounds check for a load and a store. I went with fusing the
+        pointer masking in the WasmBoundsCheckValue since it should reduce
+        additional compiler overhead.
+
+        * b3/B3LowerToAir.cpp:
+        * b3/B3Validate.cpp:
+        * b3/B3WasmBoundsCheckValue.cpp:
+        (JSC::B3::WasmBoundsCheckValue::WasmBoundsCheckValue):
+        (JSC::B3::WasmBoundsCheckValue::dumpMeta const):
+        * b3/B3WasmBoundsCheckValue.h:
+        (JSC::B3::WasmBoundsCheckValue::pinnedIndexingMask const):
+        * b3/air/AirCustom.h:
+        (JSC::B3::Air::WasmBoundsCheckCustom::generate):
+        * b3/testb3.cpp:
+        (JSC::B3::testWasmBoundsCheck):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::emitAllocateRawObject):
+        (JSC::DFG::SpeculativeJIT::loadFromIntTypedArray):
+        (JSC::DFG::SpeculativeJIT::compileGetByValOnIntTypedArray):
+        (JSC::DFG::SpeculativeJIT::compileGetByValOnFloatTypedArray):
+        (JSC::DFG::SpeculativeJIT::compileNewTypedArrayWithSize):
+        * dfg/DFGSpeculativeJIT.h:
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileAtomicsReadModifyWrite):
+        (JSC::FTL::DFG::LowerDFGToB3::compileGetByVal):
+        (JSC::FTL::DFG::LowerDFGToB3::compileNewTypedArray):
+        (JSC::FTL::DFG::LowerDFGToB3::pointerIntoTypedArray):
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::emitIntTypedArrayGetByVal):
+        * runtime/Butterfly.h:
+        (JSC::Butterfly::computeIndexingMask const):
+        (JSC::Butterfly::computeIndexingMaskForVectorLength): Deleted.
+        * runtime/JSArrayBufferView.cpp:
+        (JSC::JSArrayBufferView::JSArrayBufferView):
+        * wasm/WasmB3IRGenerator.cpp:
+        (JSC::Wasm::B3IRGenerator::B3IRGenerator):
+        (JSC::Wasm::B3IRGenerator::restoreWebAssemblyGlobalState):
+        (JSC::Wasm::B3IRGenerator::emitCheckAndPreparePointer):
+        (JSC::Wasm::B3IRGenerator::load):
+        (JSC::Wasm::B3IRGenerator::store):
+        (JSC::Wasm::B3IRGenerator::addCallIndirect):
+        * wasm/WasmBinding.cpp:
+        (JSC::Wasm::wasmToWasm):
+        * wasm/WasmMemory.cpp:
+        (JSC::Wasm::Memory::Memory):
+        (JSC::Wasm::Memory::grow):
+        * wasm/WasmMemory.h:
+        (JSC::Wasm::Memory::offsetOfIndexingMask):
+        * wasm/WasmMemoryInformation.cpp:
+        (JSC::Wasm::PinnedRegisterInfo::get):
+        (JSC::Wasm::PinnedRegisterInfo::PinnedRegisterInfo):
+        * wasm/WasmMemoryInformation.h:
+        (JSC::Wasm::PinnedRegisterInfo::toSave const):
+        * wasm/js/JSToWasm.cpp:
+        (JSC::Wasm::createJSToWasmWrapper):
+
 2018-01-05  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r226434.
index d39ba7b..850d641 100644 (file)
@@ -3245,9 +3245,10 @@ private:
             WasmBoundsCheckValue* value = m_value->as<WasmBoundsCheckValue>();
 
             Value* ptr = value->child(0);
+            Tmp pointer = tmp(ptr);
 
             Arg ptrPlusImm = m_code.newTmp(GP);
-            append(Inst(Move32, value, tmp(ptr), ptrPlusImm));
+            append(Inst(Move32, value, pointer, ptrPlusImm));
             if (value->offset()) {
                 if (imm(value->offset()))
                     append(Add64, imm(value->offset()), ptrPlusImm);
@@ -3261,7 +3262,7 @@ private:
             Arg limit;
             switch (value->boundsType()) {
             case WasmBoundsCheckValue::Type::Pinned:
-                limit = Arg(value->bounds().pinned);
+                limit = Arg(value->bounds().pinnedSize);
                 break;
 
             case WasmBoundsCheckValue::Type::Maximum:
@@ -3274,6 +3275,10 @@ private:
             }
 
             append(Inst(Air::WasmBoundsCheck, value, ptrPlusImm, limit));
+            // Hypothetically, this could write to the pointer value. Which we didn't claim we did but since we assume the indexing mask
+            // should not actually change the value of the pointer we should be OK.
+            if (value->pinnedIndexingMask() != InvalidGPRReg)
+                append(And32, Arg(value->pinnedIndexingMask()), pointer, pointer);
             return;
         }
 
index d5c38a7..8220eb9 100644 (file)
@@ -473,11 +473,13 @@ public:
                 VALIDATE(value->child(0)->type() == Int32, ("At ", *value));
                 switch (value->as<WasmBoundsCheckValue>()->boundsType()) {
                 case WasmBoundsCheckValue::Type::Pinned:
-                    VALIDATE(m_procedure.code().isPinned(value->as<WasmBoundsCheckValue>()->bounds().pinned), ("At ", *value));
+                    VALIDATE(m_procedure.code().isPinned(value->as<WasmBoundsCheckValue>()->bounds().pinnedSize), ("At ", *value));
                     break;
                 case WasmBoundsCheckValue::Type::Maximum:
                     break;
                 }
+                if (value->as<WasmBoundsCheckValue>()->pinnedIndexingMask() != InvalidGPRReg)
+                    VALIDATE(m_procedure.code().isPinned(value->as<WasmBoundsCheckValue>()->pinnedIndexingMask()), ("At ", *value));
                 VALIDATE(m_procedure.code().wasmBoundsCheckGenerator(), ("At ", *value));
                 break;
             case Upsilon:
index 810dde2..9bcb9a7 100644 (file)
@@ -35,12 +35,13 @@ WasmBoundsCheckValue::~WasmBoundsCheckValue()
 {
 }
 
-WasmBoundsCheckValue::WasmBoundsCheckValue(Origin origin, GPRReg pinnedGPR, Value* ptr, unsigned offset)
+WasmBoundsCheckValue::WasmBoundsCheckValue(Origin origin, GPRReg pinnedSize, GPRReg pinnedIndexingMask, Value* ptr, unsigned offset)
     : Value(CheckedOpcode, WasmBoundsCheck, origin, ptr)
     , m_offset(offset)
     , m_boundsType(Type::Pinned)
+    , m_pinnedIndexingMask(pinnedIndexingMask)
 {
-    m_bounds.pinned = pinnedGPR;
+    m_bounds.pinnedSize = pinnedSize;
 }
 
 WasmBoundsCheckValue::WasmBoundsCheckValue(Origin origin, Value* ptr, unsigned offset, size_t maximum)
@@ -64,10 +65,10 @@ void WasmBoundsCheckValue::dumpMeta(CommaPrinter& comma, PrintStream& out) const
 {
     switch (m_boundsType) {
     case Type::Pinned:
-        out.print(comma, "offset = ", m_offset, ", pinned = ", m_bounds.pinned);
+        out.print(comma, "offset = ", m_offset, comma, "pinnedSize = ", m_bounds.pinnedSize, comma, "pinnedMask", m_pinnedIndexingMask);
         break;
     case Type::Maximum:
-        out.print(comma, "offset = ", m_offset, ", maximum = ", m_bounds.maximum);
+        out.print(comma, "offset = ", m_offset, comma, "maximum = ", m_bounds.maximum);
         break;
     }
 }
index 732d5b7..7c4b3de 100644 (file)
@@ -52,13 +52,14 @@ public:
     };
 
     union Bounds {
-        GPRReg pinned;
+        GPRReg pinnedSize;
         size_t maximum;
     };
 
     unsigned offset() const { return m_offset; }
     Type boundsType() const { return m_boundsType; }
     Bounds bounds() const { return m_bounds; }
+    GPRReg pinnedIndexingMask() const { return m_pinnedIndexingMask; };
 
 protected:
     void dumpMeta(CommaPrinter&, PrintStream&) const override;
@@ -68,12 +69,14 @@ protected:
 private:
     friend class Procedure;
 
-    JS_EXPORT_PRIVATE WasmBoundsCheckValue(Origin, GPRReg pinnedGPR, Value* ptr, unsigned offset);
+    JS_EXPORT_PRIVATE WasmBoundsCheckValue(Origin, GPRReg pinnedGPR, GPRReg pinnedIndexingMask, Value* ptr, unsigned offset);
     JS_EXPORT_PRIVATE WasmBoundsCheckValue(Origin, Value* ptr, unsigned offset, size_t maximum);
 
     unsigned m_offset;
     Type m_boundsType;
     Bounds m_bounds;
+    GPRReg m_pinnedIndexingMask { InvalidGPRReg };
+
 };
 
 } } // namespace JSC::B3
index c6eb9c1..62abfc6 100644 (file)
@@ -343,7 +343,7 @@ struct WasmBoundsCheckCustom : public CommonCustomBase<WasmBoundsCheckCustom> {
                 outOfBounds.link(&jit);
                 switch (value->boundsType()) {
                 case WasmBoundsCheckValue::Type::Pinned:
-                    context.code->wasmBoundsCheckGenerator()->run(jit, value->bounds().pinned);
+                    context.code->wasmBoundsCheckGenerator()->run(jit, value->bounds().pinnedSize);
                     break;
 
                 case WasmBoundsCheckValue::Type::Maximum:
index 0b22906..48cce9c 100644 (file)
@@ -15865,12 +15865,19 @@ void testDepend64()
     CHECK_EQ(invoke<int64_t>(*code, values), 42 + 0xbeef);
 }
 
-void testWasmBoundsCheck(unsigned offset)
+enum class UseIndexingMaskGPR { No, Yes };
+void testWasmBoundsCheck(unsigned offset, UseIndexingMaskGPR useIndexingMask)
 {
     Procedure proc;
     GPRReg pinned = GPRInfo::argumentGPR1;
+    GPRReg indexingMask = InvalidGPRReg;
     proc.pinRegister(pinned);
 
+    if (useIndexingMask == UseIndexingMaskGPR::Yes) {
+        indexingMask = GPRInfo::argumentGPR2;
+        proc.pinRegister(indexingMask);
+    }
+
     proc.setWasmBoundsCheckGenerator([=] (CCallHelpers& jit, GPRReg pinnedGPR) {
         CHECK_EQ(pinnedGPR, pinned);
 
@@ -15885,14 +15892,20 @@ void testWasmBoundsCheck(unsigned offset)
     Value* left = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
     if (pointerType() != Int32)
         left = root->appendNew<Value>(proc, Trunc, Origin(), left);
-    root->appendNew<WasmBoundsCheckValue>(proc, Origin(), pinned, left, offset);
+    root->appendNew<WasmBoundsCheckValue>(proc, Origin(), pinned, indexingMask, left, offset);
     Value* result = root->appendNew<Const32Value>(proc, Origin(), 0x42);
     root->appendNewControlValue(proc, Return, Origin(), result);
 
     auto code = compileProc(proc);
-    CHECK_EQ(invoke<int32_t>(*code, 1, 2 + offset), 0x42);
-    CHECK_EQ(invoke<int32_t>(*code, 3, 2 + offset), 42);
-    CHECK_EQ(invoke<int32_t>(*code, 2, 2 + offset), 42);
+    uint32_t bound = 2 + offset;
+    uint32_t mask = WTF::computeIndexingMask(bound);
+    auto computeResult = [&] (uint32_t input) {
+        return input + offset < bound ? 0x42 : 42;
+    };
+
+    CHECK_EQ(invoke<int32_t>(*code, 1, bound, mask), computeResult(1));
+    CHECK_EQ(invoke<int32_t>(*code, 3, bound, mask), computeResult(3));
+    CHECK_EQ(invoke<int32_t>(*code, 2, bound, mask), computeResult(2));
 }
 
 void testWasmAddress()
@@ -17745,10 +17758,15 @@ void run(const char* filter)
     RUN(testDepend32());
     RUN(testDepend64());
 
-    RUN(testWasmBoundsCheck(0));
-    RUN(testWasmBoundsCheck(100));
-    RUN(testWasmBoundsCheck(10000));
-    RUN(testWasmBoundsCheck(std::numeric_limits<unsigned>::max() - 5));
+    RUN(testWasmBoundsCheck(0, UseIndexingMaskGPR::No));
+    RUN(testWasmBoundsCheck(100, UseIndexingMaskGPR::No));
+    RUN(testWasmBoundsCheck(10000, UseIndexingMaskGPR::No));
+    RUN(testWasmBoundsCheck(std::numeric_limits<unsigned>::max() - 5, UseIndexingMaskGPR::No));
+    RUN(testWasmBoundsCheck(0, UseIndexingMaskGPR::Yes));
+    RUN(testWasmBoundsCheck(100, UseIndexingMaskGPR::Yes));
+    RUN(testWasmBoundsCheck(10000, UseIndexingMaskGPR::Yes));
+    RUN(testWasmBoundsCheck(std::numeric_limits<unsigned>::max() - 5, UseIndexingMaskGPR::Yes));
+
     RUN(testWasmAddress());
     
     RUN(testFastTLSLoad());
index 92a5e74..fcc191e 100644 (file)
@@ -130,7 +130,7 @@ void SpeculativeJIT::emitAllocateRawObject(GPRReg resultGPR, RegisteredStructure
     MarkedAllocator* allocatorPtr = subspaceFor<JSFinalObject>(*m_jit.vm())->allocatorForNonVirtual(allocationSize, AllocatorForMode::AllocatorIfExists);
     if (allocatorPtr) {
         m_jit.move(TrustedImmPtr(allocatorPtr), scratchGPR);
-        uint32_t mask = Butterfly::computeIndexingMaskForVectorLength(vectorLength);
+        uint32_t mask = WTF::computeIndexingMask(vectorLength);
         emitAllocateJSObject(resultGPR, allocatorPtr, scratchGPR, TrustedImmPtr(structure), storageGPR, TrustedImm32(mask), scratch2GPR, slowCases);
         m_jit.emitInitializeInlineStorage(resultGPR, structure->inlineCapacity());
     } else
@@ -2854,8 +2854,9 @@ JITCompiler::Jump SpeculativeJIT::jumpForTypedArrayIsNeuteredIfOutOfBounds(Node*
     return done;
 }
 
-void SpeculativeJIT::loadFromIntTypedArray(GPRReg storageReg, GPRReg propertyReg, GPRReg resultReg, TypedArrayType type)
+void SpeculativeJIT::loadFromIntTypedArray(GPRReg baseReg, GPRReg storageReg, GPRReg propertyReg, GPRReg resultReg, TypedArrayType type)
 {
+    m_jit.and32(MacroAssembler::Address(baseReg, JSObject::butterflyIndexingMaskOffset()), propertyReg);
     switch (elementSize(type)) {
     case 1:
         if (isSigned(type))
@@ -2925,7 +2926,7 @@ void SpeculativeJIT::compileGetByValOnIntTypedArray(Node* node, TypedArrayType t
     ASSERT(node->arrayMode().alreadyChecked(m_jit.graph(), node, m_state.forNode(node->child1())));
 
     emitTypedArrayBoundsCheck(node, baseReg, propertyReg);
-    loadFromIntTypedArray(storageReg, propertyReg, resultReg, type);
+    loadFromIntTypedArray(baseReg, storageReg, propertyReg, resultReg, type);
     bool canSpeculate = true;
     setIntTypedArrayLoadResult(node, resultReg, type, canSpeculate);
 }
@@ -3157,6 +3158,7 @@ void SpeculativeJIT::compileGetByValOnFloatTypedArray(Node* node, TypedArrayType
     FPRTemporary result(this);
     FPRReg resultReg = result.fpr();
     emitTypedArrayBoundsCheck(node, baseReg, propertyReg);
+    m_jit.and32(MacroAssembler::Address(baseReg, JSObject::butterflyIndexingMaskOffset()), propertyReg);
     switch (elementSize(type)) {
     case 4:
         m_jit.loadFloat(MacroAssembler::BaseIndex(storageReg, propertyReg, MacroAssembler::TimesFour), resultReg);
@@ -8989,10 +8991,12 @@ void SpeculativeJIT::compileNewTypedArrayWithSize(Node* node)
     
     GPRTemporary result(this);
     GPRTemporary storage(this);
+    GPRTemporary indexingMask(this);
     GPRTemporary scratch(this);
     GPRTemporary scratch2(this);
     GPRReg resultGPR = result.gpr();
     GPRReg storageGPR = storage.gpr();
+    GPRReg indexingMaskGPR = indexingMask.gpr();
     GPRReg scratchGPR = scratch.gpr();
     GPRReg scratchGPR2 = scratch2.gpr();
     
@@ -9035,11 +9039,11 @@ void SpeculativeJIT::compileNewTypedArrayWithSize(Node* node)
     done.link(&m_jit);
 
     auto butterfly = TrustedImmPtr(nullptr);
-    auto mask = TrustedImm32(0);
+    m_jit.emitComputeButterflyIndexingMask(sizeGPR, scratchGPR, indexingMaskGPR);
     emitAllocateJSObject<JSArrayBufferView>(
-        resultGPR, TrustedImmPtr(structure), butterfly, mask, scratchGPR, scratchGPR2,
+        resultGPR, TrustedImmPtr(structure), butterfly, indexingMaskGPR, scratchGPR, scratchGPR2,
         slowCases);
-    
+
     m_jit.storePtr(
         storageGPR,
         MacroAssembler::Address(resultGPR, JSArrayBufferView::offsetOfVector()));
index 94f66c9..7d4fb79 100644 (file)
@@ -3042,7 +3042,7 @@ public:
         GPRTemporary& valueTag,
 #endif
         Edge valueUse, JITCompiler::JumpList& slowPathCases, bool isClamped = false);
-    void loadFromIntTypedArray(GPRReg storageReg, GPRReg propertyReg, GPRReg resultReg, TypedArrayType);
+    void loadFromIntTypedArray(GPRReg baseReg, GPRReg storageReg, GPRReg propertyReg, GPRReg resultReg, TypedArrayType);
     void setIntTypedArrayLoadResult(Node*, GPRReg resultReg, TypedArrayType, bool canSpeculate = false);
     template <typename ClassType> void compileNewFunctionCommon(GPRReg, RegisteredStructure, GPRReg, GPRReg, GPRReg, MacroAssembler::JumpList&, size_t, FunctionExecutable*, ptrdiff_t, ptrdiff_t, ptrdiff_t);
     void compileNewFunction(Node*);
index 04e16e4..a152bf1 100644 (file)
@@ -3302,7 +3302,7 @@ void SpeculativeJIT::compile(Node* node)
         
         JITCompiler::Label loop = m_jit.label();
         
-        loadFromIntTypedArray(storageGPR, indexGPR, oldValueGPR, type);
+        loadFromIntTypedArray(baseGPR, storageGPR, indexGPR, oldValueGPR, type);
         m_jit.move(oldValueGPR, newValueGPR);
         m_jit.move(oldValueGPR, resultGPR);
         
index 496a246..89f03b9 100644 (file)
@@ -3153,9 +3153,10 @@ private:
         LValue args[2];
         for (unsigned i = numExtraArgs; i--;)
             args[i] = getIntTypedArrayStoreOperand(argEdges[i]);
+        LValue base = lowCell(baseEdge);
         LValue storage = lowStorage(storageEdge);
-        
-        TypedPointer pointer = pointerIntoTypedArray(storage, index, type);
+
+        TypedPointer pointer = pointerIntoTypedArray(base, storage, index, type);
         Width width = widthForBytes(elementSize(type));
         
         LValue atomicValue;
@@ -3900,13 +3901,14 @@ private:
         }
             
         default: {
+            LValue base = lowCell(m_node->child1());
             LValue index = lowInt32(m_node->child2());
             LValue storage = lowStorage(m_node->child3());
             
             TypedArrayType type = m_node->arrayMode().typedArrayType();
             
             if (isTypedView(type)) {
-                TypedPointer pointer = pointerIntoTypedArray(storage, index, type);
+                TypedPointer pointer = pointerIntoTypedArray(base, storage, index, type);
                 
                 if (isInt(type)) {
                     LValue result = loadFromIntTypedArray(pointer, type);
@@ -5580,8 +5582,9 @@ private:
 
             ValueFromBlock haveStorage = m_out.anchor(storage);
 
+            LValue indexingMask = computeButterflyIndexingMask(size);
             LValue fastResultValue =
-                allocateObject<JSArrayBufferView>(structure, m_out.intPtrZero, m_out.int32Zero, slowCase);
+                allocateObject<JSArrayBufferView>(structure, m_out.intPtrZero, indexingMask, slowCase);
 
             m_out.storePtr(storage, fastResultValue, m_heaps.JSArrayBufferView_vector);
             m_out.store32(size, fastResultValue, m_heaps.JSArrayBufferView_length);
@@ -13141,9 +13144,10 @@ private:
         functor(TypeofType::Undefined);
     }
     
-    TypedPointer pointerIntoTypedArray(LValue storage, LValue index, TypedArrayType type)
+    TypedPointer pointerIntoTypedArray(LValue base, LValue storage, LValue index, TypedArrayType type)
     {
-        LValue offset = m_out.shl(m_out.zeroExtPtr(index), m_out.constIntPtr(logElementSize(type)));
+        LValue mask = m_out.load32(base, m_heaps.JSObject_butterflyMask);
+        LValue offset = m_out.shl(m_out.zeroExtPtr(m_out.bitAnd(mask, index)), m_out.constIntPtr(logElementSize(type)));
         return TypedPointer(
             m_heaps.typedArrayProperties,
             m_out.add(
index 938dfd9..c349bc0 100644 (file)
@@ -1478,7 +1478,7 @@ JIT::JumpList JIT::emitIntTypedArrayGetByVal(Instruction*, PatchableJump& badTyp
     slowCases.append(branch32(AboveOrEqual, property, Address(base, JSArrayBufferView::offsetOfLength())));
     loadPtr(Address(base, JSArrayBufferView::offsetOfVector()), scratch);
     cageConditionally(Gigacage::Primitive, scratch, scratch2);
-    
+
     switch (elementSize(type)) {
     case 1:
         if (JSC::isSigned(type))
index 7300ca7..d1a2ad5 100644 (file)
@@ -123,8 +123,7 @@ public:
     void setPublicLength(uint32_t value) { indexingHeader()->setPublicLength(value); }
     void setVectorLength(uint32_t value) { indexingHeader()->setVectorLength(value); }
 
-    static uint32_t computeIndexingMaskForVectorLength(uint32_t vectorLength) { return static_cast<uint64_t>(static_cast<uint32_t>(-1)) >> std::clz(vectorLength); }
-    uint32_t computeIndexingMask() const { return computeIndexingMaskForVectorLength(vectorLength()); }
+    uint32_t computeIndexingMask() const { return WTF::computeIndexingMask(vectorLength()); }
 
     template<typename T>
     T* indexingPayload() { return reinterpret_cast_ptr<T*>(this); }
index d139770..97f688e 100644 (file)
@@ -131,9 +131,7 @@ JSArrayBufferView::JSArrayBufferView(VM& vm, ConstructionContext& context)
     , m_length(context.length())
     , m_mode(context.mode())
 {
-    // Typed Arrays don't participate in indexing masks due to performance concerns. This isn't too big of a deal because
-    // they are gigacaged.
-    setButterflyWithIndexingMask(vm, context.butterfly(), 0);
+    setButterflyWithIndexingMask(vm, context.butterfly(), WTF::computeIndexingMask(length()));
     m_vector.setWithoutBarrier(context.vector());
 }
 
index fc5d110..62c71ce 100644 (file)
@@ -560,10 +560,10 @@ ArrayBuffer* JSGenericTypedArrayView<Adaptor>::slowDownAndWasteMemory(JSArrayBuf
     DeferGCForAWhile deferGC(*heap);
     
     RELEASE_ASSERT(!thisObject->hasIndexingHeader());
-    ASSERT(!thisObject->butterflyIndexingMask());
     thisObject->setButterflyWithIndexingMask(vm, Butterfly::createOrGrowArrayRight(
         thisObject->butterfly(), vm, thisObject, thisObject->structure(),
-        thisObject->structure()->outOfLineCapacity(), false, 0, 0), 0);
+        thisObject->structure()->outOfLineCapacity(), false, 0, 0),
+        WTF::computeIndexingMask(thisObject->length()));
 
     RefPtr<ArrayBuffer> buffer;
     
index a2b813f..17789f0 100644 (file)
@@ -233,7 +233,8 @@ private:
 
     void emitTierUpCheck(uint32_t decrementCount, Origin);
 
-    ExpressionType emitCheckAndPreparePointer(ExpressionType pointer, uint32_t offset, uint32_t sizeOfOp);
+    enum class ShouldMask { Yes, No };
+    ExpressionType emitCheckAndPreparePointer(ExpressionType pointer, uint32_t offset, uint32_t sizeOfOp, ShouldMask);
     B3::Kind memoryKind(B3::Opcode memoryOp);
     ExpressionType emitLoadOp(LoadOpType, ExpressionType pointer, uint32_t offset);
     void emitStoreOp(StoreOpType, ExpressionType pointer, ExpressionType value, uint32_t offset);
@@ -266,6 +267,7 @@ private:
     InsertionSet m_constantInsertionValues;
     GPRReg m_memoryBaseGPR { InvalidGPRReg };
     GPRReg m_memorySizeGPR { InvalidGPRReg };
+    GPRReg m_indexingMaskGPR { InvalidGPRReg };
     GPRReg m_wasmContextInstanceGPR { InvalidGPRReg };
     bool m_makesCalls { false };
 
@@ -344,6 +346,8 @@ B3IRGenerator::B3IRGenerator(const ModuleInformation& info, Procedure& procedure
 
     if (mode != MemoryMode::Signaling) {
         ASSERT(!pinnedRegs.sizeRegisters[0].sizeOffset);
+        m_indexingMaskGPR = pinnedRegs.indexingMask;
+        m_proc.pinRegister(pinnedRegs.indexingMask);
         m_memorySizeGPR = pinnedRegs.sizeRegisters[0].sizeRegister;
         for (const PinnedSizeRegisterInfo& regInfo : pinnedRegs.sizeRegisters)
             m_proc.pinRegister(regInfo.sizeRegister);
@@ -466,6 +470,7 @@ void B3IRGenerator::restoreWebAssemblyGlobalState(RestoreCachedStackLimit restor
         const PinnedRegisterInfo* pinnedRegs = &PinnedRegisterInfo::get();
         RegisterSet clobbers;
         clobbers.set(pinnedRegs->baseMemoryPointer);
+        clobbers.set(pinnedRegs->indexingMask);
         for (auto info : pinnedRegs->sizeRegisters)
             clobbers.set(info.sizeRegister);
 
@@ -485,6 +490,7 @@ void B3IRGenerator::restoreWebAssemblyGlobalState(RestoreCachedStackLimit restor
             ASSERT(sizeRegs.size() >= 1);
             ASSERT(!sizeRegs[0].sizeOffset); // The following code assumes we start at 0, and calculates subsequent size registers relative to 0.
             jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfSize()), sizeRegs[0].sizeRegister);
+            jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfIndexingMask()), pinnedRegs->indexingMask);
             jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfMemory()), baseMemory);
             for (unsigned i = 1; i < sizeRegs.size(); ++i)
                 jit.add64(CCallHelpers::TrustedImm32(-sizeRegs[i].sizeOffset), sizeRegs[0].sizeRegister, sizeRegs[i].sizeRegister);
@@ -635,19 +641,21 @@ auto B3IRGenerator::setGlobal(uint32_t index, ExpressionType value) -> PartialRe
     return { };
 }
 
-inline Value* B3IRGenerator::emitCheckAndPreparePointer(ExpressionType pointer, uint32_t offset, uint32_t sizeOfOperation)
+inline Value* B3IRGenerator::emitCheckAndPreparePointer(ExpressionType pointer, uint32_t offset, uint32_t sizeOfOperation, ShouldMask shouldMask)
 {
     ASSERT(m_memoryBaseGPR);
 
     switch (m_mode) {
-    case MemoryMode::BoundsChecking:
+    case MemoryMode::BoundsChecking: {
         // We're not using signal handling at all, we must therefore check that no memory access exceeds the current memory size.
         ASSERT(m_memorySizeGPR);
         ASSERT(sizeOfOperation + offset > offset);
-        m_currentBlock->appendNew<WasmBoundsCheckValue>(m_proc, origin(), m_memorySizeGPR, pointer, sizeOfOperation + offset - 1);
+        GPRReg indexingMask = shouldMask == ShouldMask::Yes ? m_indexingMaskGPR : InvalidGPRReg;
+        m_currentBlock->appendNew<WasmBoundsCheckValue>(m_proc, origin(), m_memorySizeGPR, indexingMask, pointer, sizeOfOperation + offset - 1);
         break;
+    }
 
-    case MemoryMode::Signaling:
+    case MemoryMode::Signaling: {
         // We've virtually mapped 4GiB+redzone for this memory. Only the user-allocated pages are addressable, contiguously in range [0, current],
         // and everything above is mapped PROT_NONE. We don't need to perform any explicit bounds check in the 4GiB range because WebAssembly register
         // memory accesses are 32-bit. However WebAssembly register + offset accesses perform the addition in 64-bit which can push an access above
@@ -664,6 +672,8 @@ inline Value* B3IRGenerator::emitCheckAndPreparePointer(ExpressionType pointer,
         }
         break;
     }
+    }
+
     pointer = m_currentBlock->appendNew<Value>(m_proc, ZExt32, origin(), pointer);
     return m_currentBlock->appendNew<WasmAddressValue>(m_proc, origin(), pointer, m_memoryBaseGPR);
 }
@@ -813,7 +823,7 @@ auto B3IRGenerator::load(LoadOpType op, ExpressionType pointer, ExpressionType&
         }
 
     } else
-        result = emitLoadOp(op, emitCheckAndPreparePointer(pointer, offset, sizeOfLoadOp(op)), offset);
+        result = emitLoadOp(op, emitCheckAndPreparePointer(pointer, offset, sizeOfLoadOp(op), ShouldMask::Yes), offset);
 
     return { };
 }
@@ -886,7 +896,7 @@ auto B3IRGenerator::store(StoreOpType op, ExpressionType pointer, ExpressionType
             this->emitExceptionCheck(jit, ExceptionType::OutOfBoundsMemoryAccess);
         });
     } else
-        emitStoreOp(op, emitCheckAndPreparePointer(pointer, offset, sizeOfStoreOp(op)), value, offset);
+        emitStoreOp(op, emitCheckAndPreparePointer(pointer, offset, sizeOfStoreOp(op), ShouldMask::No), value, offset);
 
     return { };
 }
@@ -1284,6 +1294,7 @@ auto B3IRGenerator::addCallIndirect(const Signature& signature, Vector<Expressio
             ASSERT(sizeRegs[0].sizeRegister != baseMemory);
             ASSERT(sizeRegs[0].sizeRegister != newContextInstance);
             ASSERT(!sizeRegs[0].sizeOffset);
+            jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfIndexingMask()), pinnedRegs.indexingMask); // Indexing mask.
             jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfSize()), sizeRegs[0].sizeRegister); // Memory size.
             jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfMemory()), baseMemory); // Memory::void*.
         });
index 62ea3af..759b268 100644 (file)
@@ -66,6 +66,7 @@ Expected<MacroAssemblerCodeRef, BindingFailure> wasmToWasm(unsigned importIndex)
     // Set up the callee's baseMemory register as well as the memory size registers.
     jit.loadPtr(JIT::Address(baseMemory, Instance::offsetOfMemory()), baseMemory); // Wasm::Memory*.
     ASSERT(!sizeRegs[0].sizeOffset); // The following code assumes we start at 0, and calculates subsequent size registers relative to 0.
+    jit.loadPtr(CCallHelpers::Address(baseMemory, Memory::offsetOfIndexingMask()), pinnedRegs.indexingMask); // Indexing mask.
     jit.loadPtr(JIT::Address(baseMemory, Wasm::Memory::offsetOfSize()), sizeRegs[0].sizeRegister); // Memory size.
     jit.loadPtr(JIT::Address(baseMemory, Wasm::Memory::offsetOfMemory()), baseMemory); // Wasm::Memory::void*.
     for (unsigned i = 1; i < sizeRegs.size(); ++i) {
index 1c1cf59..c120da7 100644 (file)
@@ -257,6 +257,7 @@ Memory::Memory(PageCount initial, PageCount maximum, Function<void(NotifyPressur
 Memory::Memory(void* memory, PageCount initial, PageCount maximum, size_t mappedCapacity, MemoryMode mode, Function<void(NotifyPressure)>&& notifyMemoryPressure, Function<void(SyncTryToReclaim)>&& syncTryToReclaimMemory, WTF::Function<void(GrowSuccess, PageCount, PageCount)>&& growSuccessCallback)
     : m_memory(memory)
     , m_size(initial.bytes())
+    , m_indexingMask(WTF::computeIndexingMask(initial.bytes()))
     , m_initial(initial)
     , m_maximum(maximum)
     , m_mappedCapacity(mappedCapacity)
@@ -432,6 +433,7 @@ Expected<PageCount, Memory::GrowFailReason> Memory::grow(PageCount delta)
         m_memory = newMemory;
         m_mappedCapacity = desiredSize;
         m_size = desiredSize;
+        m_indexingMask = WTF::computeIndexingMask(desiredSize);
         return success();
     }
     case MemoryMode::Signaling: {
@@ -446,6 +448,7 @@ Expected<PageCount, Memory::GrowFailReason> Memory::grow(PageCount delta)
         }
         commitZeroPages(startAddress, extraBytes);
         m_size = desiredSize;
+        m_indexingMask = WTF::computeIndexingMask(desiredSize);
         return success();
     }
     }
index d86de32..fff94eb 100644 (file)
@@ -85,6 +85,7 @@ public:
 
     static ptrdiff_t offsetOfMemory() { return OBJECT_OFFSETOF(Memory, m_memory); }
     static ptrdiff_t offsetOfSize() { return OBJECT_OFFSETOF(Memory, m_size); }
+    static ptrdiff_t offsetOfIndexingMask() { return OBJECT_OFFSETOF(Memory, m_indexingMask); }
 
 private:
     Memory();
@@ -94,6 +95,7 @@ private:
     // FIXME: we cache these on the instances to avoid a load on instance->instance calls. This will require updating all the instances when grow is called. https://bugs.webkit.org/show_bug.cgi?id=177305
     void* m_memory { nullptr };
     size_t m_size { 0 };
+    size_t m_indexingMask { 0 };
     PageCount m_initial;
     PageCount m_maximum;
     size_t m_mappedCapacity { 0 };
index b5c34bd..e2acfb4 100644 (file)
@@ -58,33 +58,36 @@ const PinnedRegisterInfo& PinnedRegisterInfo::get()
         Vector<PinnedSizeRegisterInfo> sizeRegisters;
         GPRReg baseMemoryPointer = InvalidGPRReg;
         GPRReg wasmContextInstancePointer = InvalidGPRReg;
+        GPRReg indexingMask = InvalidGPRReg;
 
         // FIXME: We should support more than one memory size register, and we should allow different
         //        WebAssembly.Instance to have different pins. Right now we take a vector with only one entry.
         //        If we have more than one size register, we can have one for each load size class.
         //        see: https://bugs.webkit.org/show_bug.cgi?id=162952
         Vector<unsigned> pinnedSizes = { 0 };
-        unsigned numberOfPinnedRegisters = pinnedSizes.size() + 1;
+        unsigned numberOfPinnedRegisters = pinnedSizes.size() + 2;
         if (!Context::useFastTLS())
             ++numberOfPinnedRegisters;
         Vector<GPRReg> pinnedRegs = getPinnedRegisters(numberOfPinnedRegisters);
 
         baseMemoryPointer = pinnedRegs.takeLast();
+        indexingMask = pinnedRegs.takeLast();
         if (!Context::useFastTLS())
             wasmContextInstancePointer = pinnedRegs.takeLast();
 
         ASSERT(pinnedSizes.size() == pinnedRegs.size());
         for (unsigned i = 0; i < pinnedSizes.size(); ++i)
             sizeRegisters.append({ pinnedRegs[i], pinnedSizes[i] });
-        staticPinnedRegisterInfo.construct(WTFMove(sizeRegisters), baseMemoryPointer, wasmContextInstancePointer);
+        staticPinnedRegisterInfo.construct(WTFMove(sizeRegisters), baseMemoryPointer, indexingMask, wasmContextInstancePointer);
     });
 
     return staticPinnedRegisterInfo.get();
 }
 
-PinnedRegisterInfo::PinnedRegisterInfo(Vector<PinnedSizeRegisterInfo>&& sizeRegisters, GPRReg baseMemoryPointer, GPRReg wasmContextInstancePointer)
+PinnedRegisterInfo::PinnedRegisterInfo(Vector<PinnedSizeRegisterInfo>&& sizeRegisters, GPRReg baseMemoryPointer, GPRReg indexingMask, GPRReg wasmContextInstancePointer)
     : sizeRegisters(WTFMove(sizeRegisters))
     , baseMemoryPointer(baseMemoryPointer)
+    , indexingMask(indexingMask)
     , wasmContextInstancePointer(wasmContextInstancePointer)
 {
 }
index 23329eb..fb10946 100644 (file)
@@ -31,6 +31,8 @@
 #include "RegisterSet.h"
 #include "WasmMemory.h"
 #include "WasmPageCount.h"
+
+#include <wtf/Forward.h>
 #include <wtf/Ref.h>
 #include <wtf/Vector.h>
 
@@ -41,12 +43,11 @@ struct PinnedSizeRegisterInfo {
     unsigned sizeOffset;
 };
 
-struct PinnedRegisterInfo {
-    Vector<PinnedSizeRegisterInfo> sizeRegisters;
-    GPRReg baseMemoryPointer;
-    GPRReg wasmContextInstancePointer;
+class PinnedRegisterInfo {
+public:
+    PinnedRegisterInfo(Vector<PinnedSizeRegisterInfo>&&, GPRReg, GPRReg, GPRReg);
+
     static const PinnedRegisterInfo& get();
-    PinnedRegisterInfo(Vector<PinnedSizeRegisterInfo>&&, GPRReg, GPRReg);
 
     RegisterSet toSave(MemoryMode mode) const
     {
@@ -55,11 +56,17 @@ struct PinnedRegisterInfo {
         if (wasmContextInstancePointer != InvalidGPRReg)
             result.set(wasmContextInstancePointer);
         if (mode != MemoryMode::Signaling) {
+            result.set(indexingMask);
             for (const auto& info : sizeRegisters)
                 result.set(info.sizeRegister);
         }
         return result;
     }
+
+    Vector<PinnedSizeRegisterInfo> sizeRegisters;
+    GPRReg baseMemoryPointer;
+    GPRReg indexingMask;
+    GPRReg wasmContextInstancePointer;
 };
 
 class MemoryInformation {
index 56e7801..31c9c2b 100644 (file)
@@ -181,6 +181,7 @@ std::unique_ptr<InternalFunction> createJSToWasmWrapper(CompilationContext& comp
         }
 
         if (mode != MemoryMode::Signaling) {
+            jit.loadPtr(CCallHelpers::Address(baseMemory, Wasm::Memory::offsetOfIndexingMask()), pinnedRegs.indexingMask);
             const auto& sizeRegs = pinnedRegs.sizeRegisters;
             ASSERT(sizeRegs.size() >= 1);
             ASSERT(!sizeRegs[0].sizeOffset); // The following code assumes we start at 0, and calculates subsequent size registers relative to 0.
index 13f5c20..dca55ca 100644 (file)
@@ -1,3 +1,13 @@
+2018-01-04  Keith Miller  <keith_miller@apple.com>
+
+        TypedArrays and Wasm should use index masking.
+        https://bugs.webkit.org/show_bug.cgi?id=181313
+
+        Reviewed by Michael Saboff.
+
+        * wtf/MathExtras.h:
+        (WTF::computeIndexingMask):
+
 2018-01-03  Michael Saboff  <msaboff@apple.com>
 
         Disable SharedArrayBuffers from Web API
index 08d458a..2db2b7c 100644 (file)
@@ -473,6 +473,13 @@ inline bool rangesOverlap(T leftMin, T leftMax, T rightMin, T rightMax)
     return nonEmptyRangesOverlap(leftMin, leftMax, rightMin, rightMax);
 }
 
+// This mask is not necessarily the minimal mask, specifically if size is
+// a power of 2. It has the advantage that it's fast to compute, however.
+inline uint32_t computeIndexingMask(uint32_t size)
+{
+    return static_cast<uint64_t>(static_cast<uint32_t>(-1)) >> std::clz(size);
+}
+
 } // namespace WTF
 
 #endif // #ifndef WTF_MathExtras_h