Modified: trunk/Source/_javascript_Core/ChangeLog (209952 => 209953)
--- trunk/Source/_javascript_Core/ChangeLog 2016-12-17 01:06:49 UTC (rev 209952)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-12-17 01:07:38 UTC (rev 209953)
@@ -1,3 +1,25 @@
+2016-12-16 Saam Barati <sbar...@apple.com>
+
+ B3::DoubleToFloatReduction will accidentally convince itself it converted a Phi from Double to Float and then convert uses of that Phi into a use of FloatToDouble(@Phi)
+ https://bugs.webkit.org/show_bug.cgi?id=165946
+
+ Reviewed by Keith Miller.
+
+ This was happening because the phase will convert some Phi nodes
+ from Double to Float. However, one place that did this conversion
+ forgot to first check if the Phi was already a Float. If it's already
+ a Float, a later part of the phase will be buggy if the phase claims that it has
+ converted it from Double->Float. The reason is that at the end of the
+ phase, we'll look for all uses of former Double Phi nodes and make them
+ be a use of ConvertFloatToDouble on the Phi, instead of a use of the Phi itself.
+ This is clearly wrong if the Phi were Float to begin with (and
+ therefore, the uses were Float uses to begin with).
+
+ * b3/B3ReduceDoubleToFloat.cpp:
+ * b3/testb3.cpp:
+ (JSC::B3::testReduceFloatToDoubleValidates):
+ (JSC::B3::run):
+
2016-12-16 Mark Lam <mark....@apple.com>
De-duplicate finally blocks.
Modified: trunk/Source/_javascript_Core/b3/B3ReduceDoubleToFloat.cpp (209952 => 209953)
--- trunk/Source/_javascript_Core/b3/B3ReduceDoubleToFloat.cpp 2016-12-17 01:06:49 UTC (rev 209952)
+++ trunk/Source/_javascript_Core/b3/B3ReduceDoubleToFloat.cpp 2016-12-17 01:07:38 UTC (rev 209953)
@@ -231,7 +231,9 @@
return insertionSet.insert<ConstFloatValue>(valueIndex, value->origin(), static_cast<float>(value->asDouble()));
if (value->opcode() == Phi) {
- convertPhi(value);
+ ASSERT(value->type() == Double || value->type() == Float);
+ if (value->type() == Double)
+ convertPhi(value);
return value;
}
RELEASE_ASSERT_NOT_REACHED();
@@ -241,6 +243,7 @@
void convertPhi(Value* phi)
{
ASSERT(phi->opcode() == Phi);
+ ASSERT(phi->type() == Double);
phi->setType(Float);
m_convertedPhis.add(phi);
}
Modified: trunk/Source/_javascript_Core/b3/testb3.cpp (209952 => 209953)
--- trunk/Source/_javascript_Core/b3/testb3.cpp 2016-12-17 01:06:49 UTC (rev 209952)
+++ trunk/Source/_javascript_Core/b3/testb3.cpp 2016-12-17 01:07:38 UTC (rev 209953)
@@ -4535,6 +4535,61 @@
CHECK(isIdentical(invoke<float>(*code, 0, bitwise_cast<int32_t>(value)), static_cast<float>(M_PI)));
}
+void testReduceFloatToDoubleValidates()
+{
+ // Simple case of:
+ // f = DoubleToFloat(Bitcast(argGPR0))
+ // if (a) {
+ // x = FloatConst()
+ // else
+ // x = FloatConst()
+ // p = Phi(x)
+ // a = Mul(p, p)
+ // b = Add(a, f)
+ // c = Add(p, b)
+ // Return(c)
+ //
+ // This should not crash in the validator after ReduceFloatToDouble.
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+ BasicBlock* thenCase = proc.addBlock();
+ BasicBlock* elseCase = proc.addBlock();
+ BasicBlock* tail = proc.addBlock();
+
+ Value* condition = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+ Value* thingy = root->appendNew<Value>(proc, BitwiseCast, Origin(), condition);
+ thingy = root->appendNew<Value>(proc, DoubleToFloat, Origin(), thingy); // Make the phase think it has work to do.
+ root->appendNewControlValue(
+ proc, Branch, Origin(),
+ condition,
+ FrequentedBlock(thenCase), FrequentedBlock(elseCase));
+
+ UpsilonValue* thenValue = thenCase->appendNew<UpsilonValue>(proc, Origin(),
+ thenCase->appendNew<ConstFloatValue>(proc, Origin(), 11.5));
+ thenCase->appendNewControlValue(proc, Jump, Origin(), FrequentedBlock(tail));
+
+ UpsilonValue* elseValue = elseCase->appendNew<UpsilonValue>(proc, Origin(),
+ elseCase->appendNew<ConstFloatValue>(proc, Origin(), 10.5));
+ elseCase->appendNewControlValue(proc, Jump, Origin(), FrequentedBlock(tail));
+
+ Value* phi = tail->appendNew<Value>(proc, Phi, Float, Origin());
+ thenValue->setPhi(phi);
+ elseValue->setPhi(phi);
+ Value* result = tail->appendNew<Value>(proc, Mul, Origin(),
+ phi, phi);
+ result = tail->appendNew<Value>(proc, Add, Origin(),
+ result,
+ thingy);
+ result = tail->appendNew<Value>(proc, Add, Origin(),
+ phi,
+ result);
+ tail->appendNewControlValue(proc, Return, Origin(), result);
+
+ auto code = compile(proc);
+ CHECK(isIdentical(invoke<float>(*code, 1), 11.5f * 11.5f + static_cast<float>(bitwise_cast<double>(static_cast<uint64_t>(1))) + 11.5f));
+ CHECK(isIdentical(invoke<float>(*code, 0), 10.5f * 10.5f + static_cast<float>(bitwise_cast<double>(static_cast<uint64_t>(0))) + 10.5f));
+}
+
void testDoubleProducerPhiToFloatConversion(float value)
{
Procedure proc;
@@ -14628,6 +14683,7 @@
RUN_BINARY(testCompareOneFloatToDouble, floatingPointOperands<float>(), floatingPointOperands<double>());
RUN_BINARY(testCompareFloatToDoubleThroughPhi, floatingPointOperands<float>(), floatingPointOperands<float>());
RUN_UNARY(testDoubleToFloatThroughPhi, floatingPointOperands<float>());
+ RUN(testReduceFloatToDoubleValidates());
RUN_UNARY(testDoubleProducerPhiToFloatConversion, floatingPointOperands<float>());
RUN_UNARY(testDoubleProducerPhiToFloatConversionWithDoubleConsumer, floatingPointOperands<float>());
RUN_BINARY(testDoubleProducerPhiWithNonFloatConst, floatingPointOperands<float>(), floatingPointOperands<double>());
