Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (203487 => 203488)
--- trunk/Source/_javascript_Core/ChangeLog 2016-07-21 03:30:48 UTC (rev 203487)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-07-21 05:24:54 UTC (rev 203488)
@@ -1,3 +1,50 @@
+2016-07-20 Filip Pizlo <fpi...@apple.com>
+
+ FTL snippet generators should be able to request a different register for output and input
+ https://bugs.webkit.org/show_bug.cgi?id=160010
+ rdar://problem/27439330
+
+ Reviewed by Saam Barati.
+
+ The BitOr and BitXor snippet generators have problems if the register for the right input is
+ the same as the register for the result. We could fix those generators, but I'm not convinced
+ that the other snippet generators don't have this bug. So, the approach that this patch takes
+ is to teach the FTL to request that B3 to use a different register for the result than for
+ any input to the snippet patchpoint.
+
+ Air already has the ability to let any instruction do an EarlyDef, which means exactly this.
+ But B3 did not expose this via ValueRep. This patch exposes this in ValueRep as
+ SomeEarlyRegister. That's most of the change.
+
+ This adds a testb3 test for SomeEarlyRegister and a regression test for this particular
+ problem. The regression test failed on trunk JSC before this.
+
+ * b3/B3LowerToAir.cpp:
+ (JSC::B3::Air::LowerToAir::lower):
+ * b3/B3PatchpointSpecial.cpp:
+ (JSC::B3::PatchpointSpecial::forEachArg):
+ (JSC::B3::PatchpointSpecial::admitsStack):
+ * b3/B3StackmapSpecial.cpp:
+ (JSC::B3::StackmapSpecial::forEachArgImpl):
+ (JSC::B3::StackmapSpecial::isArgValidForRep):
+ * b3/B3Validate.cpp:
+ * b3/B3ValueRep.cpp:
+ (JSC::B3::ValueRep::addUsedRegistersTo):
+ (JSC::B3::ValueRep::dump):
+ (WTF::printInternal):
+ * b3/B3ValueRep.h:
+ (JSC::B3::ValueRep::ValueRep):
+ (JSC::B3::ValueRep::reg):
+ (JSC::B3::ValueRep::isAny):
+ (JSC::B3::ValueRep::isReg):
+ (JSC::B3::ValueRep::isSomeRegister): Deleted.
+ * b3/testb3.cpp:
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::emitBinarySnippet):
+ (JSC::FTL::DFG::LowerDFGToB3::emitBinaryBitOpSnippet):
+ (JSC::FTL::DFG::LowerDFGToB3::emitRightShiftSnippet):
+ * tests/stress/ftl-bit-xor-right-result-interference.js: Added.
+
2016-07-20 Michael Saboff <msab...@apple.com>
CrashOnOverflow in JSC::Yarr::YarrPatternConstructor::setupAlternativeOffsets
Modified: trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp (203487 => 203488)
--- trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2016-07-21 03:30:48 UTC (rev 203487)
+++ trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2016-07-21 05:24:54 UTC (rev 203488)
@@ -2193,6 +2193,7 @@
case ValueRep::ColdAny:
case ValueRep::LateColdAny:
case ValueRep::SomeRegister:
+ case ValueRep::SomeEarlyRegister:
inst.args.append(tmp(patchpointValue));
break;
case ValueRep::Register: {
Modified: trunk/Source/_javascript_Core/b3/B3PatchpointSpecial.cpp (203487 => 203488)
--- trunk/Source/_javascript_Core/b3/B3PatchpointSpecial.cpp 2016-07-21 03:30:48 UTC (rev 203487)
+++ trunk/Source/_javascript_Core/b3/B3PatchpointSpecial.cpp 2016-07-21 05:24:54 UTC (rev 203488)
@@ -46,17 +46,25 @@
void PatchpointSpecial::forEachArg(Inst& inst, const ScopedLambda<Inst::EachArgCallback>& callback)
{
+ PatchpointValue* patchpoint = inst.origin->as<PatchpointValue>();
unsigned argIndex = 1;
- if (inst.origin->type() != Void)
- callback(inst.args[argIndex++], Arg::Def, inst.origin->airType(), inst.origin->airWidth());
+ if (patchpoint->type() != Void) {
+ Arg::Role role;
+ if (patchpoint->resultConstraint.kind() == ValueRep::SomeEarlyRegister)
+ role = Arg::EarlyDef;
+ else
+ role = Arg::Def;
+
+ callback(inst.args[argIndex++], role, inst.origin->airType(), inst.origin->airWidth());
+ }
forEachArgImpl(0, argIndex, inst, SameAsRep, Nullopt, callback);
argIndex += inst.origin->numChildren();
- for (unsigned i = inst.origin->as<PatchpointValue>()->numGPScratchRegisters; i--;)
+ for (unsigned i = patchpoint->numGPScratchRegisters; i--;)
callback(inst.args[argIndex++], Arg::Scratch, Arg::GP, Arg::conservativeWidth(Arg::GP));
- for (unsigned i = inst.origin->as<PatchpointValue>()->numFPScratchRegisters; i--;)
+ for (unsigned i = patchpoint->numFPScratchRegisters; i--;)
callback(inst.args[argIndex++], Arg::Scratch, Arg::FP, Arg::conservativeWidth(Arg::FP));
}
@@ -109,6 +117,7 @@
case ValueRep::StackArgument:
return true;
case ValueRep::SomeRegister:
+ case ValueRep::SomeEarlyRegister:
case ValueRep::Register:
case ValueRep::LateRegister:
return false;
Modified: trunk/Source/_javascript_Core/b3/B3StackmapSpecial.cpp (203487 => 203488)
--- trunk/Source/_javascript_Core/b3/B3StackmapSpecial.cpp 2016-07-21 03:30:48 UTC (rev 203487)
+++ trunk/Source/_javascript_Core/b3/B3StackmapSpecial.cpp 2016-07-21 05:24:54 UTC (rev 203488)
@@ -116,6 +116,9 @@
case ValueRep::LateColdAny:
role = Arg::LateColdUse;
break;
+ default:
+ RELEASE_ASSERT_NOT_REACHED();
+ break;
}
break;
case ForceLateUse:
@@ -230,6 +233,7 @@
// We already verified by isArgValidForValue().
return true;
case ValueRep::SomeRegister:
+ case ValueRep::SomeEarlyRegister:
return arg.isTmp();
case ValueRep::LateRegister:
case ValueRep::Register:
Modified: trunk/Source/_javascript_Core/b3/B3Validate.cpp (203487 => 203488)
--- trunk/Source/_javascript_Core/b3/B3Validate.cpp 2016-07-21 03:30:48 UTC (rev 203487)
+++ trunk/Source/_javascript_Core/b3/B3Validate.cpp 2016-07-21 05:24:54 UTC (rev 203488)
@@ -337,6 +337,7 @@
switch (value->as<PatchpointValue>()->resultConstraint.kind()) {
case ValueRep::WarmAny:
case ValueRep::SomeRegister:
+ case ValueRep::SomeEarlyRegister:
case ValueRep::Register:
case ValueRep::StackArgument:
break;
@@ -345,7 +346,7 @@
break;
}
- validateStackmapConstraint(value, ConstrainedValue(value, value->as<PatchpointValue>()->resultConstraint));
+ validateStackmapConstraint(value, ConstrainedValue(value, value->as<PatchpointValue>()->resultConstraint), ConstraintRole::Def);
}
validateStackmap(value);
break;
@@ -432,7 +433,11 @@
validateStackmapConstraint(stackmap, child);
}
- void validateStackmapConstraint(Value* context, const ConstrainedValue& value)
+ enum class ConstraintRole {
+ Use,
+ Def
+ };
+ void validateStackmapConstraint(Value* context, const ConstrainedValue& value, ConstraintRole role = ConstraintRole::Use)
{
switch (value.rep().kind()) {
case ValueRep::WarmAny:
@@ -441,6 +446,9 @@
case ValueRep::SomeRegister:
case ValueRep::StackArgument:
break;
+ case ValueRep::SomeEarlyRegister:
+ VALIDATE(role == ConstraintRole::Def, ("At ", *context, ": ", value));
+ break;
case ValueRep::Register:
case ValueRep::LateRegister:
if (value.rep().reg().isGPR())
Modified: trunk/Source/_javascript_Core/b3/B3ValueRep.cpp (203487 => 203488)
--- trunk/Source/_javascript_Core/b3/B3ValueRep.cpp 2016-07-21 03:30:48 UTC (rev 203487)
+++ trunk/Source/_javascript_Core/b3/B3ValueRep.cpp 2016-07-21 05:24:54 UTC (rev 203488)
@@ -40,6 +40,7 @@
case ColdAny:
case LateColdAny:
case SomeRegister:
+ case SomeEarlyRegister:
case Constant:
return;
case LateRegister:
@@ -70,6 +71,7 @@
case ColdAny:
case LateColdAny:
case SomeRegister:
+ case SomeEarlyRegister:
return;
case LateRegister:
case Register:
@@ -173,6 +175,9 @@
case ValueRep::SomeRegister:
out.print("SomeRegister");
return;
+ case ValueRep::SomeEarlyRegister:
+ out.print("SomeEarlyRegister");
+ return;
case ValueRep::Register:
out.print("Register");
return;
Modified: trunk/Source/_javascript_Core/b3/B3ValueRep.h (203487 => 203488)
--- trunk/Source/_javascript_Core/b3/B3ValueRep.h 2016-07-21 03:30:48 UTC (rev 203487)
+++ trunk/Source/_javascript_Core/b3/B3ValueRep.h 2016-07-21 05:24:54 UTC (rev 203488)
@@ -67,6 +67,11 @@
// register that this claims to clobber!
SomeRegister,
+ // As an input representation, this tells us that B3 should pick some register, but implies
+ // that the def happens before any of the effects of the stackmap. This is only valid for
+ // the result constraint of a Patchpoint.
+ SomeEarlyRegister,
+
// As an input representation, this forces a particular register. As an output
// representation, this tells us what register B3 picked.
Register,
@@ -103,7 +108,7 @@
ValueRep(Kind kind)
: m_kind(kind)
{
- ASSERT(kind == WarmAny || kind == ColdAny || kind == LateColdAny || kind == SomeRegister);
+ ASSERT(kind == WarmAny || kind == ColdAny || kind == LateColdAny || kind == SomeRegister || kind == SomeEarlyRegister);
}
static ValueRep reg(Reg reg)
@@ -177,8 +182,6 @@
bool isAny() const { return kind() == WarmAny || kind() == ColdAny || kind() == LateColdAny; }
- bool isSomeRegister() const { return kind() == SomeRegister; }
-
bool isReg() const { return kind() == Register || kind() == LateRegister; }
Reg reg() const
Modified: trunk/Source/_javascript_Core/b3/testb3.cpp (203487 => 203488)
--- trunk/Source/_javascript_Core/b3/testb3.cpp 2016-07-21 03:30:48 UTC (rev 203487)
+++ trunk/Source/_javascript_Core/b3/testb3.cpp 2016-07-21 05:24:54 UTC (rev 203488)
@@ -12377,6 +12377,49 @@
validate(proc);
}
+void testSomeEarlyRegister()
+{
+ auto run = [&] (bool succeed) {
+ Procedure proc;
+
+ BasicBlock* root = proc.addBlock();
+
+ PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Int32, Origin());
+ patchpoint->resultConstraint = ValueRep::reg(GPRInfo::returnValueGPR);
+ bool ranFirstPatchpoint = false;
+ patchpoint->setGenerator(
+ [&] (CCallHelpers&, const StackmapGenerationParams& params) {
+ CHECK(params[0].gpr() == GPRInfo::returnValueGPR);
+ ranFirstPatchpoint = true;
+ });
+
+ Value* arg = patchpoint;
+
+ patchpoint = root->appendNew<PatchpointValue>(proc, Int32, Origin());
+ patchpoint->appendSomeRegister(arg);
+ if (succeed)
+ patchpoint->resultConstraint = ValueRep::SomeEarlyRegister;
+ bool ranSecondPatchpoint = false;
+ patchpoint->setGenerator(
+ [&] (CCallHelpers&, const StackmapGenerationParams& params) {
+ if (succeed)
+ CHECK(params[0].gpr() != params[1].gpr());
+ else
+ CHECK(params[0].gpr() == params[1].gpr());
+ ranSecondPatchpoint = true;
+ });
+
+ root->appendNew<Value>(proc, Return, Origin(), patchpoint);
+
+ compile(proc);
+ CHECK(ranFirstPatchpoint);
+ CHECK(ranSecondPatchpoint);
+ };
+
+ run(true);
+ run(false);
+}
+
// Make sure the compiler does not try to optimize anything out.
NEVER_INLINE double zero()
{
@@ -13783,6 +13826,7 @@
RUN(testInterpreter());
RUN(testReduceStrengthCheckBottomUseInAnotherBlock());
RUN(testResetReachabilityDanglingReference());
+ RUN(testSomeEarlyRegister());
if (tasks.isEmpty())
usage();
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (203487 => 203488)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-07-21 03:30:48 UTC (rev 203487)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-07-21 05:24:54 UTC (rev 203488)
@@ -8108,6 +8108,7 @@
if (scratchFPRUsage == NeedScratchFPR)
patchpoint->numFPScratchRegisters++;
patchpoint->clobber(RegisterSet::macroScratchRegisters());
+ patchpoint->resultConstraint = ValueRep::SomeEarlyRegister;
State* state = &m_ftlState;
patchpoint->setGenerator(
[=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
@@ -8170,6 +8171,7 @@
preparePatchpointForExceptions(patchpoint);
patchpoint->numGPScratchRegisters = 1;
patchpoint->clobber(RegisterSet::macroScratchRegisters());
+ patchpoint->resultConstraint = ValueRep::SomeEarlyRegister;
State* state = &m_ftlState;
patchpoint->setGenerator(
[=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
@@ -8225,6 +8227,7 @@
patchpoint->numGPScratchRegisters = 1;
patchpoint->numFPScratchRegisters = 1;
patchpoint->clobber(RegisterSet::macroScratchRegisters());
+ patchpoint->resultConstraint = ValueRep::SomeEarlyRegister;
State* state = &m_ftlState;
patchpoint->setGenerator(
[=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
Added: trunk/Source/_javascript_Core/tests/stress/ftl-bit-xor-right-result-interference.js (0 => 203488)
--- trunk/Source/_javascript_Core/tests/stress/ftl-bit-xor-right-result-interference.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/ftl-bit-xor-right-result-interference.js 2016-07-21 05:24:54 UTC (rev 203488)
@@ -0,0 +1,31 @@
+var toggle = 0;
+
+function bar()
+{
+ if (toggle ^= 1)
+ return 42;
+ else
+ return {valueOf: function() { return 42; }};
+}
+
+noInline(bar);
+
+function baz()
+{
+ return 7;
+}
+
+noInline(baz);
+
+function foo()
+{
+ return bar() ^ baz();
+}
+
+noInline(foo);
+
+for (var i = 0; i < 100000; ++i) {
+ var result = foo();
+ if (result != 45)
+ throw "Error: bad result: " + result;
+}
