Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (183614 => 183615)
--- trunk/Source/_javascript_Core/ChangeLog 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-04-30 04:40:55 UTC (rev 183615)
@@ -1,3 +1,57 @@
+2015-04-29 Benjamin Poulain <[email protected]>
+
+ [JSC] Remove RageConvert array conversion
+ https://bugs.webkit.org/show_bug.cgi?id=144433
+
+ 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.
+
2015-04-29 Joseph Pecoraro <[email protected]>
Gracefully handle missing auto pause key on remote inspector setup
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h (183614 => 183615)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractInterpreterInlines.h 2015-04-30 04:40:55 UTC (rev 183615)
@@ -1686,8 +1686,7 @@
m_state.setFoundConstants(true);
break;
}
- ASSERT(node->arrayMode().conversion() == Array::Convert
- || node->arrayMode().conversion() == Array::RageConvert);
+ ASSERT(node->arrayMode().conversion() == Array::Convert);
clobberStructures(clobberLimit);
filterArrayModes(node->child1(), node->arrayMode().arrayModesThatPassFiltering());
break;
Modified: trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp (183614 => 183615)
--- trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/dfg/DFGArrayMode.cpp 2015-04-30 04:40:55 UTC (rev 183615)
@@ -151,8 +151,7 @@
ArrayMode ArrayMode::refine(
Graph& graph, Node* node,
- SpeculatedType base, SpeculatedType index, SpeculatedType value,
- NodeFlags flags) const
+ SpeculatedType base, SpeculatedType index, SpeculatedType value) const
{
if (!base || !index) {
// It can be that we had a legitimate arrayMode but no incoming predictions. That'll
@@ -197,15 +196,11 @@
return withTypeAndConversion(Array::Contiguous, Array::Convert);
case Array::Double:
- if (flags & NodeBytecodeUsesAsInt)
- return withTypeAndConversion(Array::Contiguous, Array::RageConvert);
if (!value || isFullNumberSpeculation(value))
return *this;
return withTypeAndConversion(Array::Contiguous, Array::Convert);
case Array::Contiguous:
- if (doesConversion() && (flags & NodeBytecodeUsesAsInt))
- return withConversion(Array::RageConvert);
return *this;
case Array::Int8Array:
@@ -579,8 +574,6 @@
return "AsIs";
case Array::Convert:
return "Convert";
- case Array::RageConvert:
- return "RageConvert";
default:
return "Unknown!";
}
Modified: trunk/Source/_javascript_Core/dfg/DFGArrayMode.h (183614 => 183615)
--- trunk/Source/_javascript_Core/dfg/DFGArrayMode.h 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/dfg/DFGArrayMode.h 2015-04-30 04:40:55 UTC (rev 183615)
@@ -95,8 +95,7 @@
};
enum Conversion {
AsIs,
- Convert,
- RageConvert
+ Convert
};
} // namespace Array
@@ -222,7 +221,7 @@
return ArrayMode(type, arrayClass(), speculation(), conversion);
}
- ArrayMode refine(Graph&, Node*, SpeculatedType base, SpeculatedType index, SpeculatedType value = SpecNone, NodeFlags = 0) const;
+ ArrayMode refine(Graph&, Node*, SpeculatedType base, SpeculatedType index, SpeculatedType value = SpecNone) const;
bool alreadyChecked(Graph&, Node*, AbstractValue&) const;
Modified: trunk/Source/_javascript_Core/dfg/DFGArrayifySlowPathGenerator.h (183614 => 183615)
--- trunk/Source/_javascript_Core/dfg/DFGArrayifySlowPathGenerator.h 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/dfg/DFGArrayifySlowPathGenerator.h 2015-04-30 04:40:55 UTC (rev 183615)
@@ -101,10 +101,7 @@
jit->callOperation(operationEnsureDouble, m_tempGPR, m_baseGPR);
break;
case Array::Contiguous:
- if (m_arrayMode.conversion() == Array::RageConvert)
- jit->callOperation(operationRageEnsureContiguous, m_tempGPR, m_baseGPR);
- else
- jit->callOperation(operationEnsureContiguous, m_tempGPR, m_baseGPR);
+ jit->callOperation(operationEnsureContiguous, m_tempGPR, m_baseGPR);
break;
case Array::ArrayStorage:
case Array::SlowPutArrayStorage:
Modified: trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp (183614 => 183615)
--- trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/dfg/DFGFixupPhase.cpp 2015-04-30 04:40:55 UTC (rev 183615)
@@ -517,7 +517,7 @@
m_graph, node,
node->child1()->prediction(),
node->child2()->prediction(),
- SpecNone, node->flags()));
+ SpecNone));
blessArrayOperation(node->child1(), node->child2(), node->child3());
@@ -1105,7 +1105,7 @@
m_graph, node,
node->child1()->prediction(),
node->child2()->prediction(),
- SpecNone, node->flags()));
+ SpecNone));
blessArrayOperation(node->child1(), node->child2(), node->child3());
fixEdge<CellUse>(node->child1());
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.cpp (183614 => 183615)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.cpp 2015-04-30 04:40:55 UTC (rev 183615)
@@ -951,17 +951,6 @@
return reinterpret_cast<char*>(asObject(cell)->ensureContiguous(vm).data());
}
-char* JIT_OPERATION operationRageEnsureContiguous(ExecState* exec, JSCell* cell)
-{
- VM& vm = exec->vm();
- NativeCallFrameTracer tracer(&vm, exec);
-
- if (!cell->isObject())
- return 0;
-
- return reinterpret_cast<char*>(asObject(cell)->rageEnsureContiguous(vm).data());
-}
-
char* JIT_OPERATION operationEnsureArrayStorage(ExecState* exec, JSCell* cell)
{
VM& vm = exec->vm();
Modified: trunk/Source/_javascript_Core/dfg/DFGOperations.h (183614 => 183615)
--- trunk/Source/_javascript_Core/dfg/DFGOperations.h 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/dfg/DFGOperations.h 2015-04-30 04:40:55 UTC (rev 183615)
@@ -113,7 +113,6 @@
char* JIT_OPERATION operationEnsureInt32(ExecState*, JSCell*);
char* JIT_OPERATION operationEnsureDouble(ExecState*, JSCell*);
char* JIT_OPERATION operationEnsureContiguous(ExecState*, JSCell*);
-char* JIT_OPERATION operationRageEnsureContiguous(ExecState*, JSCell*);
char* JIT_OPERATION operationEnsureArrayStorage(ExecState*, JSCell*);
StringImpl* JIT_OPERATION operationResolveRope(ExecState*, JSString*);
JSString* JIT_OPERATION operationSingleCharacterString(ExecState*, int32_t);
Modified: trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp (183614 => 183615)
--- trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/dfg/DFGPredictionPropagationPhase.cpp 2015-04-30 04:40:55 UTC (rev 183615)
@@ -391,7 +391,7 @@
m_graph, node,
node->child1()->prediction(),
node->child2()->prediction(),
- SpecNone, node->flags());
+ SpecNone);
switch (arrayMode.type()) {
case Array::Double:
Modified: trunk/Source/_javascript_Core/dfg/DFGTypeCheckHoistingPhase.cpp (183614 => 183615)
--- trunk/Source/_javascript_Core/dfg/DFGTypeCheckHoistingPhase.cpp 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/dfg/DFGTypeCheckHoistingPhase.cpp 2015-04-30 04:40:55 UTC (rev 183615)
@@ -228,7 +228,9 @@
noticeStructureCheck(variable, node->structureSet());
break;
}
-
+
+ case ArrayifyToStructure:
+ case Arrayify:
case GetByOffset:
case PutByOffset:
case PutStructure:
@@ -250,22 +252,6 @@
// Don't count these uses.
break;
- case ArrayifyToStructure:
- case Arrayify:
- if (node->arrayMode().conversion() == Array::RageConvert) {
- // Rage conversion changes structures. We should avoid tying to do
- // any kind of hoisting when rage conversion is in play.
- Node* child = node->child1().node();
- if (child->op() != GetLocal)
- break;
- VariableAccessData* variable = child->variableAccessData();
- variable->vote(VoteOther);
- if (!shouldConsiderForHoisting<StructureTypeCheck>(variable))
- break;
- noticeStructureCheck(variable, 0);
- }
- break;
-
case SetLocal: {
// Find all uses of the source of the SetLocal. If any of them are a
// kind of CheckStructure, then we should notice them to ensure that
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (183614 => 183615)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-04-30 04:40:55 UTC (rev 183615)
@@ -1943,10 +1943,7 @@
vmCall(m_out.operation(operationEnsureDouble), m_callFrame, cell);
break;
case Array::Contiguous:
- if (m_node->arrayMode().conversion() == Array::RageConvert)
- vmCall(m_out.operation(operationRageEnsureContiguous), m_callFrame, cell);
- else
- vmCall(m_out.operation(operationEnsureContiguous), m_callFrame, cell);
+ vmCall(m_out.operation(operationEnsureContiguous), m_callFrame, cell);
break;
case Array::ArrayStorage:
case Array::SlowPutArrayStorage:
Modified: trunk/Source/_javascript_Core/runtime/JSObject.cpp (183614 => 183615)
--- trunk/Source/_javascript_Core/runtime/JSObject.cpp 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/runtime/JSObject.cpp 2015-04-30 04:40:55 UTC (rev 183615)
@@ -836,8 +836,7 @@
return convertInt32ToArrayStorage(vm, structure(vm)->suggestedArrayStorageTransition());
}
-template<JSObject::DoubleToContiguousMode mode>
-ContiguousJSValues JSObject::genericConvertDoubleToContiguous(VM& vm)
+ContiguousJSValues JSObject::convertDoubleToContiguous(VM& vm)
{
ASSERT(hasDouble(indexingType()));
@@ -849,20 +848,7 @@
currentAsValue->clear();
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);
}
@@ -870,16 +856,6 @@
return m_butterfly->contiguous();
}
-ContiguousJSValues JSObject::convertDoubleToContiguous(VM& vm)
-{
- return genericConvertDoubleToContiguous<EncodeValueAsDouble>(vm);
-}
-
-ContiguousJSValues JSObject::rageConvertDoubleToContiguous(VM& vm)
-{
- return genericConvertDoubleToContiguous<RageConvertDoubleToValue>(vm);
-}
-
ArrayStorage* JSObject::convertDoubleToArrayStorage(VM& vm, NonPropertyTransition transition)
{
DeferGC deferGC(vm.heap);
@@ -1049,7 +1025,7 @@
}
}
-ContiguousJSValues JSObject::ensureContiguousSlow(VM& vm, DoubleToContiguousMode mode)
+ContiguousJSValues JSObject::ensureContiguousSlow(VM& vm)
{
ASSERT(inherits(info()));
@@ -1066,8 +1042,6 @@
return convertInt32ToContiguous(vm);
case ALL_DOUBLE_INDEXING_TYPES:
- if (mode == RageConvertDoubleToValue)
- return rageConvertDoubleToContiguous(vm);
return convertDoubleToContiguous(vm);
case ALL_ARRAY_STORAGE_INDEXING_TYPES:
@@ -1079,16 +1053,6 @@
}
}
-ContiguousJSValues JSObject::ensureContiguousSlow(VM& vm)
-{
- return ensureContiguousSlow(vm, EncodeValueAsDouble);
-}
-
-ContiguousJSValues JSObject::rageEnsureContiguousSlow(VM& vm)
-{
- return ensureContiguousSlow(vm, RageConvertDoubleToValue);
-}
-
ArrayStorage* JSObject::ensureArrayStorageSlow(VM& vm)
{
ASSERT(inherits(info()));
Modified: trunk/Source/_javascript_Core/runtime/JSObject.h (183614 => 183615)
--- trunk/Source/_javascript_Core/runtime/JSObject.h 2015-04-30 04:15:11 UTC (rev 183614)
+++ trunk/Source/_javascript_Core/runtime/JSObject.h 2015-04-30 04:40:55 UTC (rev 183615)
@@ -689,18 +689,7 @@
return ensureContiguousSlow(vm);
}
-
- // Same as ensureContiguous(), except that if the indexed storage is in
- // double mode, then it does a rage conversion to contiguous: it
- // attempts to convert each double to an int32.
- ContiguousJSValues rageEnsureContiguous(VM& vm)
- {
- if (LIKELY(hasContiguous(indexingType())))
- return m_butterfly->contiguous();
-
- return rageEnsureContiguousSlow(vm);
- }
-
+
// Ensure that the object is in a mode where it has array storage. Use
// this if you're about to perform actions that would have required the
// object to be converted to have array storage, if it didn't have it
@@ -797,7 +786,6 @@
ArrayStorage* convertInt32ToArrayStorage(VM&);
ContiguousJSValues convertDoubleToContiguous(VM&);
- ContiguousJSValues rageConvertDoubleToContiguous(VM&);
ArrayStorage* convertDoubleToArrayStorage(VM&, NonPropertyTransition);
ArrayStorage* convertDoubleToArrayStorage(VM&);
@@ -983,14 +971,8 @@
ContiguousJSValues ensureInt32Slow(VM&);
ContiguousDoubles ensureDoubleSlow(VM&);
ContiguousJSValues ensureContiguousSlow(VM&);
- ContiguousJSValues rageEnsureContiguousSlow(VM&);
JS_EXPORT_PRIVATE ArrayStorage* ensureArrayStorageSlow(VM&);
-
- enum DoubleToContiguousMode { EncodeValueAsDouble, RageConvertDoubleToValue };
- template<DoubleToContiguousMode mode>
- ContiguousJSValues genericConvertDoubleToContiguous(VM&);
- ContiguousJSValues ensureContiguousSlow(VM&, DoubleToContiguousMode);
-
+
protected:
CopyWriteBarrier<Butterfly> m_butterfly;
#if USE(JSVALUE32_64)
