Changeset 183615 in webkit for trunk/Source/JavaScriptCore/runtime/JSObject.cpp

Timestamp:

Apr 29, 2015, 9:40:55 PM (10 years ago)

Author:

[email protected]

Message:

[JSC] Remove RageConvert array conversion
​https://bugs.webkit.org/show_bug.cgi?id=144433

Patch by Benjamin Poulain <​[email protected]> on 2015-04-29
Reviewed by Filip Pizlo.

RageConvert was causing a subtle bug that was hitting the Kraken crypto tests
pretty hard:
-The indexing types shows that the array access varies between Int32 and DoubleArray.
-ArrayMode::fromObserved() decided to use the most generic type: DoubleArray.

An Arrayify node would convert the Int32 to that type.

-Somewhere, a GetByVal or PutByVal would have the flag NodeBytecodeUsesAsInt. That

node would use RageConvert instead of Convert.

-The Arrayify for that GetByVal with RageConvert would not convert the array to

Contiguous.

-All the following array access that do not have the flag NodeBytecodeUsesAsInt would

now expect a DoubleArray and always get a Contiguous Array. The CheckStructure
fail systematically and we never get to run the later code.

Getting rid of RageConvert fixes the problem and does not seems to have any
negative side effect on other benchmarks.

The improvments on Kraken are:

-stanford-crypto-aes: definitely 1.0915x faster.
-stanford-crypto-pbkdf2: definitely 1.2446x faster.
-stanford-crypto-sha256-iterative: definitely 1.0544x faster.

• dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

• dfg/DFGArrayMode.cpp:

(JSC::DFG::ArrayMode::refine):
(JSC::DFG::arrayConversionToString):

• dfg/DFGArrayMode.h:
• dfg/DFGArrayifySlowPathGenerator.h:
• dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):

• dfg/DFGOperations.cpp:
• dfg/DFGOperations.h:
• dfg/DFGPredictionPropagationPhase.cpp:

(JSC::DFG::PredictionPropagationPhase::propagate):

• dfg/DFGTypeCheckHoistingPhase.cpp:

(JSC::DFG::TypeCheckHoistingPhase::identifyRedundantStructureChecks):

• ftl/FTLLowerDFGToLLVM.cpp:

(JSC::FTL::LowerDFGToLLVM::compileArrayifyToStructure):

• runtime/JSObject.cpp:

(JSC::JSObject::convertDoubleToContiguous):
(JSC::JSObject::ensureContiguousSlow):
(JSC::JSObject::genericConvertDoubleToContiguous): Deleted.
(JSC::JSObject::rageConvertDoubleToContiguous): Deleted.
(JSC::JSObject::rageEnsureContiguousSlow): Deleted.

• runtime/JSObject.h:

(JSC::JSObject::rageEnsureContiguous): Deleted.

File:

• trunk/Source/JavaScriptCore/runtime/JSObject.cpp (modified) (6 diffs)

trunk/Source/JavaScriptCore/runtime/JSObject.cpp

@@ -837 +837 @@
 }
 
-template<JSObject::DoubleToContiguousMode mode>
-ContiguousJSValues JSObject::genericConvertDoubleToContiguous(VM& vm)
+ContiguousJSValues JSObject::convertDoubleToContiguous(VM& vm)
 {
     ASSERT(hasDouble(indexingType()));
@@ -850 +849 @@
             continue;
         }
-        JSValue v;
-        switch (mode) {
-        case EncodeValueAsDouble:
-            v = JSValue(JSValue::EncodeAsDouble, value);
-            break;
-        case RageConvertDoubleToValue:
-            v = jsNumber(value);
-            break;
-        default:
-            v = JSValue();
-            RELEASE_ASSERT_NOT_REACHED();
-            break;
-        }
-        ASSERT(v.isNumber());
+        JSValue v = JSValue(JSValue::EncodeAsDouble, value);
         currentAsValue->setWithoutWriteBarrier(v);
     }
@@ -869 +855 @@
     setStructure(vm, Structure::nonPropertyTransition(vm, structure(vm), AllocateContiguous));
     return m_butterfly->contiguous();
-}
-
-ContiguousJSValues JSObject::convertDoubleToContiguous(VM& vm)
-{
-    return genericConvertDoubleToContiguous<EncodeValueAsDouble>(vm);
-}
-
-ContiguousJSValues JSObject::rageConvertDoubleToContiguous(VM& vm)
-{
-    return genericConvertDoubleToContiguous<RageConvertDoubleToValue>(vm);
 }
 
@@ -1050 +1026 @@
 }
 
-ContiguousJSValues JSObject::ensureContiguousSlow(VM& vm, DoubleToContiguousMode mode)
+ContiguousJSValues JSObject::ensureContiguousSlow(VM& vm)
 {
     ASSERT(inherits(info()));
@@ -1067 +1043 @@
         
     case ALL_DOUBLE_INDEXING_TYPES:
-        if (mode == RageConvertDoubleToValue)
-            return rageConvertDoubleToContiguous(vm);
         return convertDoubleToContiguous(vm);
         
@@ -1078 +1052 @@
         return ContiguousJSValues();
     }
-}
-
-ContiguousJSValues JSObject::ensureContiguousSlow(VM& vm)
-{
-    return ensureContiguousSlow(vm, EncodeValueAsDouble);
-}
-
-ContiguousJSValues JSObject::rageEnsureContiguousSlow(VM& vm)
-{
-    return ensureContiguousSlow(vm, RageConvertDoubleToValue);
 }