Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (200700 => 200701)
--- trunk/Source/_javascript_Core/ChangeLog 2016-05-11 20:09:37 UTC (rev 200700)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-05-11 20:54:09 UTC (rev 200701)
@@ -1,3 +1,58 @@
+2016-05-11 Saam barati <sbar...@apple.com>
+
+ Air may decide to put the result register of an arithmetic snippet in the tag register
+ https://bugs.webkit.org/show_bug.cgi?id=157548
+
+ Reviewed by Filip Pizlo.
+
+ This patch adds a new ValueRep to B3 called LateRegister. The semantics
+ are similar to Register in that it can be used to pin an argument to
+ a particular register. It differs from ValueRep::Register in that the semantics of
+ LateRegister are that it is used after the result of the node its an argument to
+ is computed. This means that a LateRegister argument will interfere with the result
+ of a node. LateRegister is not a valid result ValueRep.
+
+ This was needed because there was a bug where B3/Air would assign the
+ result of a patchpoint to the TagTypeNumber register. This broke our
+ code when we would box a double into a JSValue in a snippet when the
+ result is the same as the TagTypeNumber register. To fix the issue,
+ we pass TagMaskRegister and TagTypeNumberRegister as ValueRep::LateRegister
+ arguments to various patchpoints.
+
+ * b3/B3LowerToAir.cpp:
+ (JSC::B3::Air::LowerToAir::fillStackmap):
+ * b3/B3PatchpointSpecial.cpp:
+ (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):
+ (JSC::B3::ValueRep::emitRestore):
+ (JSC::B3::ValueRep::recoveryForJSValue):
+ (WTF::printInternal):
+ * b3/B3ValueRep.h:
+ (JSC::B3::ValueRep::reg):
+ (JSC::B3::ValueRep::lateReg):
+ (JSC::B3::ValueRep::stack):
+ (JSC::B3::ValueRep::operator==):
+ (JSC::B3::ValueRep::isSomeRegister):
+ (JSC::B3::ValueRep::isReg):
+ * b3/testb3.cpp:
+ (JSC::B3::testSpillUseLargerThanDef):
+ (JSC::B3::testLateRegister):
+ (JSC::B3::zero):
+ (JSC::B3::run):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::lower):
+ (JSC::FTL::DFG::LowerDFGToB3::compileIn):
+ (JSC::FTL::DFG::LowerDFGToB3::getById):
+ (JSC::FTL::DFG::LowerDFGToB3::emitBinarySnippet):
+ (JSC::FTL::DFG::LowerDFGToB3::emitBinaryBitOpSnippet):
+ (JSC::FTL::DFG::LowerDFGToB3::emitRightShiftSnippet):
+
2016-05-11 Joseph Pecoraro <pecor...@apple.com>
Improve error messages for accessing arguments.callee and similar getters in strict mode
Modified: trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp (200700 => 200701)
--- trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2016-05-11 20:09:37 UTC (rev 200700)
+++ trunk/Source/_javascript_Core/b3/B3LowerToAir.cpp 2016-05-11 20:54:09 UTC (rev 200701)
@@ -1005,6 +1005,7 @@
case ValueRep::SomeRegister:
arg = tmp(value.value());
break;
+ case ValueRep::LateRegister:
case ValueRep::Register:
stackmap->earlyClobbered().clear(value.rep().reg());
arg = Tmp(value.rep().reg());
Modified: trunk/Source/_javascript_Core/b3/B3PatchpointSpecial.cpp (200700 => 200701)
--- trunk/Source/_javascript_Core/b3/B3PatchpointSpecial.cpp 2016-05-11 20:09:37 UTC (rev 200700)
+++ trunk/Source/_javascript_Core/b3/B3PatchpointSpecial.cpp 2016-05-11 20:54:09 UTC (rev 200701)
@@ -110,6 +110,7 @@
return true;
case ValueRep::SomeRegister:
case ValueRep::Register:
+ case ValueRep::LateRegister:
return false;
default:
RELEASE_ASSERT_NOT_REACHED();
Modified: trunk/Source/_javascript_Core/b3/B3StackmapSpecial.cpp (200700 => 200701)
--- trunk/Source/_javascript_Core/b3/B3StackmapSpecial.cpp 2016-05-11 20:09:37 UTC (rev 200700)
+++ trunk/Source/_javascript_Core/b3/B3StackmapSpecial.cpp 2016-05-11 20:54:09 UTC (rev 200701)
@@ -107,6 +107,9 @@
case ValueRep::Constant:
role = Arg::Use;
break;
+ case ValueRep::LateRegister:
+ role = Arg::LateUse;
+ break;
case ValueRep::ColdAny:
role = Arg::ColdUse;
break;
@@ -228,6 +231,7 @@
return true;
case ValueRep::SomeRegister:
return arg.isTmp();
+ case ValueRep::LateRegister:
case ValueRep::Register:
return arg == Tmp(rep.reg());
case ValueRep::StackArgument:
Modified: trunk/Source/_javascript_Core/b3/B3Validate.cpp (200700 => 200701)
--- trunk/Source/_javascript_Core/b3/B3Validate.cpp 2016-05-11 20:09:37 UTC (rev 200700)
+++ trunk/Source/_javascript_Core/b3/B3Validate.cpp 2016-05-11 20:54:09 UTC (rev 200701)
@@ -411,6 +411,7 @@
case ValueRep::StackArgument:
break;
case ValueRep::Register:
+ case ValueRep::LateRegister:
if (value.rep().reg().isGPR())
VALIDATE(isInt(value.value()->type()), ("At ", *context, ": ", value));
else
Modified: trunk/Source/_javascript_Core/b3/B3ValueRep.cpp (200700 => 200701)
--- trunk/Source/_javascript_Core/b3/B3ValueRep.cpp 2016-05-11 20:09:37 UTC (rev 200700)
+++ trunk/Source/_javascript_Core/b3/B3ValueRep.cpp 2016-05-11 20:54:09 UTC (rev 200701)
@@ -42,6 +42,7 @@
case SomeRegister:
case Constant:
return;
+ case LateRegister:
case Register:
set.set(reg());
return;
@@ -70,6 +71,7 @@
case LateColdAny:
case SomeRegister:
return;
+ case LateRegister:
case Register:
out.print("(", reg(), ")");
return;
@@ -90,6 +92,7 @@
{
if (reg.isGPR()) {
switch (kind()) {
+ case LateRegister:
case Register:
if (isGPR())
jit.move(gpr(), reg.gpr());
@@ -110,6 +113,7 @@
}
switch (kind()) {
+ case LateRegister:
case Register:
if (isGPR())
jit.move64ToDouble(gpr(), reg.fpr());
@@ -132,6 +136,7 @@
ValueRecovery ValueRep::recoveryForJSValue() const
{
switch (kind()) {
+ case LateRegister:
case Register:
return ValueRecovery::inGPR(gpr(), DataFormatJS);
case Stack:
@@ -171,6 +176,9 @@
case ValueRep::Register:
out.print("Register");
return;
+ case ValueRep::LateRegister:
+ out.print("LateRegister");
+ return;
case ValueRep::Stack:
out.print("Stack");
return;
Modified: trunk/Source/_javascript_Core/b3/B3ValueRep.h (200700 => 200701)
--- trunk/Source/_javascript_Core/b3/B3ValueRep.h 2016-05-11 20:09:37 UTC (rev 200700)
+++ trunk/Source/_javascript_Core/b3/B3ValueRep.h 2016-05-11 20:54:09 UTC (rev 200701)
@@ -71,6 +71,12 @@
// representation, this tells us what register B3 picked.
Register,
+ // As an input representation, this forces a particular register and states that
+ // the register is used late. This means that the register is used after the result
+ // is defined (i.e, the result will interfere with this as an input).
+ // It's not valid for this to be used as a result kind.
+ LateRegister,
+
// As an output representation, this tells us what stack slot B3 picked. It's not a valid
// input representation.
Stack,
@@ -105,6 +111,13 @@
return ValueRep(reg);
}
+ static ValueRep lateReg(Reg reg)
+ {
+ ValueRep result(reg);
+ result.m_kind = LateRegister;
+ return result;
+ }
+
static ValueRep stack(intptr_t offsetFromFP)
{
ValueRep result;
@@ -141,6 +154,7 @@
if (kind() != other.kind())
return false;
switch (kind()) {
+ case LateRegister:
case Register:
return u.reg == other.u.reg;
case Stack:
@@ -165,7 +179,7 @@
bool isSomeRegister() const { return kind() == SomeRegister; }
- bool isReg() const { return kind() == Register; }
+ bool isReg() const { return kind() == Register || kind() == LateRegister; }
Reg reg() const
{
Modified: trunk/Source/_javascript_Core/b3/testb3.cpp (200700 => 200701)
--- trunk/Source/_javascript_Core/b3/testb3.cpp 2016-05-11 20:09:37 UTC (rev 200700)
+++ trunk/Source/_javascript_Core/b3/testb3.cpp 2016-05-11 20:54:09 UTC (rev 200701)
@@ -11777,6 +11777,83 @@
}
+void testLateRegister()
+{
+ Procedure proc;
+ BasicBlock* root = proc.addBlock();
+
+ // This works by making all but 1 register be input to the first patchpoint as LateRegister.
+ // The other 1 register is just a regular Register input. We assert our result is the regular
+ // register input. There would be no other way for the register allocator to arrange things
+ // because LateRegister interferes with the result.
+ // Then, the second patchpoint takes the result of the first as an argument and asks for
+ // it in a register that was a LateRegister. This is to incentivize the register allocator
+ // to use that LateRegister as the result for the first patchpoint. But of course it can not do that.
+ // So it must issue a mov after the first patchpoint from the first's result into the second's input.
+
+ RegisterSet regs = RegisterSet::allGPRs();
+ regs.exclude(RegisterSet::stackRegisters());
+ regs.exclude(RegisterSet::reservedHardwareRegisters());
+ Vector<Value*> lateUseArgs;
+ unsigned result = 0;
+ for (GPRReg reg = CCallHelpers::firstRegister(); reg <= CCallHelpers::lastRegister(); reg = CCallHelpers::nextRegister(reg)) {
+ if (!regs.get(reg))
+ continue;
+ result++;
+ if (reg == GPRInfo::regT0)
+ continue;
+ Value* value = root->appendNew<Const64Value>(proc, Origin(), 1);
+ lateUseArgs.append(value);
+ }
+ Value* regularUse = root->appendNew<Const64Value>(proc, Origin(), 1);
+ PatchpointValue* firstPatchpoint = root->appendNew<PatchpointValue>(proc, Int64, Origin());
+ {
+ unsigned i = 0;
+ for (GPRReg reg = CCallHelpers::firstRegister(); reg <= CCallHelpers::lastRegister(); reg = CCallHelpers::nextRegister(reg)) {
+ if (!regs.get(reg))
+ continue;
+ if (reg == GPRInfo::regT0)
+ continue;
+ Value* value = lateUseArgs[i++];
+ firstPatchpoint->append(value, ValueRep::lateReg(reg));
+ }
+ firstPatchpoint->append(regularUse, ValueRep::reg(GPRInfo::regT0));
+ }
+
+ firstPatchpoint->setGenerator(
+ [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+ AllowMacroScratchRegisterUsage allowScratch(jit);
+ CHECK(params[0].gpr() == GPRInfo::regT0);
+ // Note that regT0 should also start off as 1, so we're implicitly starting our add with 1, which is also an argument.
+ unsigned skipped = 0;
+ for (unsigned i = 1; i < params.size(); i++) {
+ if (params[i].gpr() == params[0].gpr()) {
+ skipped = i;
+ continue;
+ }
+ jit.add64(params[i].gpr(), params[0].gpr());
+ }
+ CHECK(!!skipped);
+ });
+
+ PatchpointValue* secondPatchpoint = root->appendNew<PatchpointValue>(proc, Int64, Origin());
+ secondPatchpoint->append(firstPatchpoint, ValueRep::reg(GPRInfo::regT1));
+ secondPatchpoint->setGenerator(
+ [&] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+ AllowMacroScratchRegisterUsage allowScratch(jit);
+ CHECK(params[1].gpr() == GPRInfo::regT1);
+ jit.nop();
+ jit.nop();
+ jit.move(params[1].gpr(), params[0].gpr());
+ jit.nop();
+ jit.nop();
+ });
+ root->appendNew<ControlValue>(proc, Return, Origin(), secondPatchpoint);
+
+ auto code = compile(proc);
+ CHECK(invoke<uint64_t>(*code) == result);
+}
+
// Make sure the compiler does not try to optimize anything out.
NEVER_INLINE double zero()
{
@@ -13165,6 +13242,8 @@
RUN(testSpillDefSmallerThanUse());
RUN(testSpillUseLargerThanDef());
+ RUN(testLateRegister());
+
if (tasks.isEmpty())
usage();
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (200700 => 200701)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-05-11 20:09:37 UTC (rev 200700)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2016-05-11 20:54:09 UTC (rev 200701)
@@ -184,9 +184,6 @@
auto preOrder = m_graph.blocksInPreOrder();
- // We should not create any alloca's after this point, since they will cease to
- // be mem2reg candidates.
-
m_callFrame = m_out.framePointer();
m_tagTypeNumber = m_out.constInt64(TagTypeNumber);
m_tagMask = m_out.constInt64(TagMask);
@@ -6146,8 +6143,8 @@
UniquedStringImpl* str = bitwise_cast<UniquedStringImpl*>(string->tryGetValueImpl());
B3::PatchpointValue* patchpoint = m_out.patchpoint(Int64);
patchpoint->appendSomeRegister(cell);
- patchpoint->append(m_tagMask, ValueRep::reg(GPRInfo::tagMaskRegister));
- patchpoint->append(m_tagTypeNumber, ValueRep::reg(GPRInfo::tagTypeNumberRegister));
+ patchpoint->append(m_tagMask, ValueRep::lateReg(GPRInfo::tagMaskRegister));
+ patchpoint->append(m_tagTypeNumber, ValueRep::lateReg(GPRInfo::tagTypeNumberRegister));
patchpoint->clobber(RegisterSet::macroScratchRegisters());
RefPtr<PatchpointExceptionHandle> exceptionHandle = preparePatchpointForExceptions(patchpoint);
@@ -7619,8 +7616,8 @@
B3::PatchpointValue* patchpoint = m_out.patchpoint(Int64);
patchpoint->appendSomeRegister(base);
- patchpoint->append(m_tagMask, ValueRep::reg(GPRInfo::tagMaskRegister));
- patchpoint->append(m_tagTypeNumber, ValueRep::reg(GPRInfo::tagTypeNumberRegister));
+ patchpoint->append(m_tagMask, ValueRep::lateReg(GPRInfo::tagMaskRegister));
+ patchpoint->append(m_tagTypeNumber, ValueRep::lateReg(GPRInfo::tagTypeNumberRegister));
// FIXME: If this is a GetByIdFlush, we might get some performance boost if we claim that it
// clobbers volatile registers late. It's not necessary for correctness, though, since the
@@ -7972,8 +7969,8 @@
PatchpointValue* patchpoint = m_out.patchpoint(Int64);
patchpoint->appendSomeRegister(left);
patchpoint->appendSomeRegister(right);
- patchpoint->append(m_tagMask, ValueRep::reg(GPRInfo::tagMaskRegister));
- patchpoint->append(m_tagTypeNumber, ValueRep::reg(GPRInfo::tagTypeNumberRegister));
+ patchpoint->append(m_tagMask, ValueRep::lateReg(GPRInfo::tagMaskRegister));
+ patchpoint->append(m_tagTypeNumber, ValueRep::lateReg(GPRInfo::tagTypeNumberRegister));
RefPtr<PatchpointExceptionHandle> exceptionHandle =
preparePatchpointForExceptions(patchpoint);
patchpoint->numGPScratchRegisters = 1;
@@ -8028,9 +8025,6 @@
{
Node* node = m_node;
- // FIXME: Make this do exceptions.
- // https://bugs.webkit.org/show_bug.cgi?id=151686
-
LValue left = lowJSValue(node->child1());
LValue right = lowJSValue(node->child2());
@@ -8040,8 +8034,8 @@
PatchpointValue* patchpoint = m_out.patchpoint(Int64);
patchpoint->appendSomeRegister(left);
patchpoint->appendSomeRegister(right);
- patchpoint->append(m_tagMask, ValueRep::reg(GPRInfo::tagMaskRegister));
- patchpoint->append(m_tagTypeNumber, ValueRep::reg(GPRInfo::tagTypeNumberRegister));
+ patchpoint->append(m_tagMask, ValueRep::lateReg(GPRInfo::tagMaskRegister));
+ patchpoint->append(m_tagTypeNumber, ValueRep::lateReg(GPRInfo::tagTypeNumberRegister));
RefPtr<PatchpointExceptionHandle> exceptionHandle =
preparePatchpointForExceptions(patchpoint);
patchpoint->numGPScratchRegisters = 1;
@@ -8094,8 +8088,8 @@
PatchpointValue* patchpoint = m_out.patchpoint(Int64);
patchpoint->appendSomeRegister(left);
patchpoint->appendSomeRegister(right);
- patchpoint->append(m_tagMask, ValueRep::reg(GPRInfo::tagMaskRegister));
- patchpoint->append(m_tagTypeNumber, ValueRep::reg(GPRInfo::tagTypeNumberRegister));
+ patchpoint->append(m_tagMask, ValueRep::lateReg(GPRInfo::tagMaskRegister));
+ patchpoint->append(m_tagTypeNumber, ValueRep::lateReg(GPRInfo::tagTypeNumberRegister));
RefPtr<PatchpointExceptionHandle> exceptionHandle =
preparePatchpointForExceptions(patchpoint);
patchpoint->numGPScratchRegisters = 1;
