Modified: trunk/Source/_javascript_Core/ChangeLog (195421 => 195422)
--- trunk/Source/_javascript_Core/ChangeLog 2016-01-21 21:51:39 UTC (rev 195421)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-01-21 22:56:21 UTC (rev 195422)
@@ -1,3 +1,28 @@
+2016-01-21 Benjamin Poulain <benja...@webkit.org>
+
+ [JSC] foldPathConstants() makes invalid assumptions with Switch
+ https://bugs.webkit.org/show_bug.cgi?id=153324
+
+ Reviewed by Filip Pizlo.
+
+ If a Switch() has two cases pointing to the same basic block, foldPathConstants()
+ was adding two override for that block with two different constants.
+ If the block with the Switch dominates the target, both override were equally valid
+ and we were assuming any of the constants as the value in the target block.
+
+ See testSwitchTargettingSameBlockFoldPathConstant() for an example that breaks.
+
+ This patch adds checks to ignore any block that is reached more than
+ once by the control value.
+
+ * b3/B3FoldPathConstants.cpp:
+ * b3/B3Generate.cpp:
+ (JSC::B3::generateToAir):
+ * b3/testb3.cpp:
+ (JSC::B3::testSwitchTargettingSameBlock):
+ (JSC::B3::testSwitchTargettingSameBlockFoldPathConstant):
+ (JSC::B3::run):
+
2016-01-21 Filip Pizlo <fpi...@apple.com>
Unreviewed, undo DFGCommon.h change that accidentally enabled the B3 JIT.
Modified: trunk/Source/_javascript_Core/b3/B3FoldPathConstants.cpp (195421 => 195422)
--- trunk/Source/_javascript_Core/b3/B3FoldPathConstants.cpp 2016-01-21 21:51:39 UTC (rev 195421)
+++ trunk/Source/_javascript_Core/b3/B3FoldPathConstants.cpp 2016-01-21 22:56:21 UTC (rev 195422)
@@ -90,6 +90,8 @@
ControlValue* branch = block->last()->as<ControlValue>();
switch (branch->opcode()) {
case Branch:
+ if (branch->successorBlock(0) == branch->successorBlock(1))
+ continue;
addOverride(
block, branch->child(0),
Override::nonZero(branch->successorBlock(0)));
@@ -97,13 +99,21 @@
block, branch->child(0),
Override::constant(branch->successorBlock(1), 0));
break;
- case Switch:
+ case Switch: {
+ HashMap<BasicBlock*, unsigned> targetUses;
+ for (const SwitchCase& switchCase : *branch->as<SwitchValue>())
+ targetUses.add(switchCase.targetBlock(), 0).iterator->value++;
+
for (const SwitchCase& switchCase : *branch->as<SwitchValue>()) {
+ if (targetUses.find(switchCase.targetBlock())->value != 1)
+ continue;
+
addOverride(
block, branch->child(0),
Override::constant(switchCase.targetBlock(), switchCase.caseValue()));
}
break;
+ }
default:
break;
}
Modified: trunk/Source/_javascript_Core/b3/B3Generate.cpp (195421 => 195422)
--- trunk/Source/_javascript_Core/b3/B3Generate.cpp 2016-01-21 21:51:39 UTC (rev 195421)
+++ trunk/Source/_javascript_Core/b3/B3Generate.cpp 2016-01-21 22:56:21 UTC (rev 195422)
@@ -91,7 +91,7 @@
if (optLevel >= 1) {
reduceStrength(procedure);
-
+
// FIXME: Add more optimizations here.
// https://bugs.webkit.org/show_bug.cgi?id=150507
}
Modified: trunk/Source/_javascript_Core/b3/testb3.cpp (195421 => 195422)
--- trunk/Source/_javascript_Core/b3/testb3.cpp 2016-01-21 21:51:39 UTC (rev 195421)
+++ trunk/Source/_javascript_Core/b3/testb3.cpp 2016-01-21 22:56:21 UTC (rev 195422)
@@ -8429,6 +8429,66 @@
CHECK(!invoke<int32_t>(*code, degree * gap + 1, 42, 11));
}
+void testSwitchTargettingSameBlock()
+{
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+
+ BasicBlock* terminate = proc.addBlock();
+ terminate->appendNew<ControlValue>(
+ proc, Return, Origin(),
+ terminate->appendNew<Const32Value>(proc, Origin(), 5));
+
+ SwitchValue* switchValue = root->appendNew<SwitchValue>(
+ proc, Origin(),
+ root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0),
+ FrequentedBlock(terminate));
+
+ BasicBlock* otherTarget = proc.addBlock();
+ otherTarget->appendNew<ControlValue>(
+ proc, Return, Origin(),
+ otherTarget->appendNew<Const32Value>(proc, Origin(), 42));
+ switchValue->appendCase(SwitchCase(3, FrequentedBlock(otherTarget)));
+ switchValue->appendCase(SwitchCase(13, FrequentedBlock(otherTarget)));
+
+ auto code = compile(proc);
+
+ for (unsigned i = 0; i < 20; ++i) {
+ int32_t expected = (i == 3 || i == 13) ? 42 : 5;
+ CHECK(invoke<int32_t>(*code, i) == expected);
+ }
+}
+
+void testSwitchTargettingSameBlockFoldPathConstant()
+{
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+
+ BasicBlock* terminate = proc.addBlock();
+ terminate->appendNew<ControlValue>(
+ proc, Return, Origin(),
+ terminate->appendNew<Const32Value>(proc, Origin(), 42));
+
+ Value* argument = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::argumentGPR0);
+ SwitchValue* switchValue = root->appendNew<SwitchValue>(
+ proc, Origin(),
+ argument,
+ FrequentedBlock(terminate));
+
+ BasicBlock* otherTarget = proc.addBlock();
+ otherTarget->appendNew<ControlValue>(
+ proc, Return, Origin(), argument);
+ switchValue->appendCase(SwitchCase(3, FrequentedBlock(otherTarget)));
+ switchValue->appendCase(SwitchCase(13, FrequentedBlock(otherTarget)));
+
+ auto code = compile(proc);
+
+ for (unsigned i = 0; i < 20; ++i) {
+ int32_t expected = (i == 3 || i == 13) ? i : 42;
+ CHECK(invoke<int32_t>(*code, i) == expected);
+ }
+}
+
void testTruncFold(int64_t value)
{
Procedure proc;
@@ -10385,6 +10445,9 @@
RUN(testSwitchChillDiv(100, 1));
RUN(testSwitchChillDiv(100, 100));
+ RUN(testSwitchTargettingSameBlock());
+ RUN(testSwitchTargettingSameBlockFoldPathConstant());
+
RUN(testTrunc(0));
RUN(testTrunc(1));
RUN(testTrunc(-1));
