Log Message
FTL B3 should know that used registers are not the same thing as used registers. Rename the latter to unavailable registers to avoid future confusion. https://bugs.webkit.org/show_bug.cgi?id=152572
Reviewed by Saam Barati. Prior to this change, we used the term "used registers" in two different senses: - The set of registers that are live at some point in the current compilation unit. A register is live at some point if it is read after that point on some path through that point. - The set of registers that are not available for scratch register use at some point. A register may not be available if it is live or if it is a callee-save register but it is not being saved by the current compilation. In the old FTL LLVM code, we had some translations from the first sense into the second sense. We forgot to do those in FTL B3, and so we get crashes, for example in V8/splay. That benchmark highlighted this issue because it fired some lazy slow paths, and then used an unsaved callee-save for scratch. Curiously, we could merge these two definitions by observing that, in some sense, an unsaved callee save is live at every point in a compilation in the sense that it may contain a value that will be read when the compilation returns. That's pretty cool, but it feels strange to me. This isn't how we would normally define liveness of registers. It's not how the Air::TmpLiveness analysis would do it for any of its other clients. So, this changes B3 to have two different concepts: - Used registers. These are the registers that are live. - Unavailable registers. These are the registers that are not available for scratch. It's always a superset of used registers. This also changes FTLLower to use unavailableRegisters() pretty much everywhere that it previously used usedRegisters(). This makes it possible to run V8/splay. * b3/B3StackmapGenerationParams.cpp: (JSC::B3::StackmapGenerationParams::usedRegisters): (JSC::B3::StackmapGenerationParams::unavailableRegisters): (JSC::B3::StackmapGenerationParams::proc): * b3/B3StackmapGenerationParams.h: * ftl/FTLLowerDFGToLLVM.cpp: (JSC::FTL::DFG::LowerDFGToLLVM::compilePutById): (JSC::FTL::DFG::LowerDFGToLLVM::getById): (JSC::FTL::DFG::LowerDFGToLLVM::lazySlowPath):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (194430 => 194431)
--- trunk/Source/_javascript_Core/ChangeLog 2015-12-28 17:22:51 UTC (rev 194430)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-12-28 22:46:51 UTC (rev 194431)
@@ -1,3 +1,54 @@
+2015-12-27 Filip Pizlo <[email protected]>
+
+ FTL B3 should know that used registers are not the same thing as used registers. Rename the
+ latter to unavailable registers to avoid future confusion.
+ https://bugs.webkit.org/show_bug.cgi?id=152572
+
+ Reviewed by Saam Barati.
+
+ Prior to this change, we used the term "used registers" in two different senses:
+
+ - The set of registers that are live at some point in the current compilation unit. A
+ register is live at some point if it is read after that point on some path through that
+ point.
+
+ - The set of registers that are not available for scratch register use at some point. A
+ register may not be available if it is live or if it is a callee-save register but it is
+ not being saved by the current compilation.
+
+ In the old FTL LLVM code, we had some translations from the first sense into the second
+ sense. We forgot to do those in FTL B3, and so we get crashes, for example in V8/splay. That
+ benchmark highlighted this issue because it fired some lazy slow paths, and then used an
+ unsaved callee-save for scratch.
+
+ Curiously, we could merge these two definitions by observing that, in some sense, an unsaved
+ callee save is live at every point in a compilation in the sense that it may contain a value
+ that will be read when the compilation returns. That's pretty cool, but it feels strange to
+ me. This isn't how we would normally define liveness of registers. It's not how the
+ Air::TmpLiveness analysis would do it for any of its other clients.
+
+ So, this changes B3 to have two different concepts:
+
+ - Used registers. These are the registers that are live.
+
+ - Unavailable registers. These are the registers that are not available for scratch. It's
+ always a superset of used registers.
+
+ This also changes FTLLower to use unavailableRegisters() pretty much everywhere that it
+ previously used usedRegisters().
+
+ This makes it possible to run V8/splay.
+
+ * b3/B3StackmapGenerationParams.cpp:
+ (JSC::B3::StackmapGenerationParams::usedRegisters):
+ (JSC::B3::StackmapGenerationParams::unavailableRegisters):
+ (JSC::B3::StackmapGenerationParams::proc):
+ * b3/B3StackmapGenerationParams.h:
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::DFG::LowerDFGToLLVM::compilePutById):
+ (JSC::FTL::DFG::LowerDFGToLLVM::getById):
+ (JSC::FTL::DFG::LowerDFGToLLVM::lazySlowPath):
+
2015-12-25 Andy Estes <[email protected]>
Stop moving local objects in return statements
Modified: trunk/Source/_javascript_Core/b3/B3StackmapGenerationParams.cpp (194430 => 194431)
--- trunk/Source/_javascript_Core/b3/B3StackmapGenerationParams.cpp 2015-12-28 17:22:51 UTC (rev 194430)
+++ trunk/Source/_javascript_Core/b3/B3StackmapGenerationParams.cpp 2015-12-28 22:46:51 UTC (rev 194431)
@@ -41,6 +41,18 @@
return m_value->m_usedRegisters;
}
+RegisterSet StackmapGenerationParams::unavailableRegisters() const
+{
+ RegisterSet result = usedRegisters();
+
+ RegisterSet unsavedCalleeSaves = RegisterSet::vmCalleeSaveRegisters();
+ for (const RegisterAtOffset& regAtOffset : m_context.code->calleeSaveRegisters())
+ unsavedCalleeSaves.clear(regAtOffset.reg());
+
+ result.merge(unsavedCalleeSaves);
+ return result;
+}
+
Procedure& StackmapGenerationParams::proc() const
{
return m_context.code->proc();
Modified: trunk/Source/_javascript_Core/b3/B3StackmapGenerationParams.h (194430 => 194431)
--- trunk/Source/_javascript_Core/b3/B3StackmapGenerationParams.h 2015-12-28 17:22:51 UTC (rev 194430)
+++ trunk/Source/_javascript_Core/b3/B3StackmapGenerationParams.h 2015-12-28 22:46:51 UTC (rev 194431)
@@ -59,6 +59,13 @@
// This tells you the registers that were used.
const RegisterSet& usedRegisters() const;
+ // This is a useful helper if you want to do register allocation inside of a patchpoint. You
+ // can only use callee-save registers if they were saved in the prologue. This gives you the
+ // used register set that's useful for such settings by returning:
+ //
+ // usedRegisters() | (RegisterSet::calleeSaveRegisters() - proc.calleeSaveRegisters())
+ RegisterSet unavailableRegisters() const;
+
// This is provided for convenience; it means that you don't have to capture it if you don't want to.
Procedure& proc() const;
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (194430 => 194431)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-12-28 17:22:51 UTC (rev 194430)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-12-28 22:46:51 UTC (rev 194431)
@@ -2586,8 +2586,8 @@
auto generator = Box<JITPutByIdGenerator>::create(
jit.codeBlock(), node->origin.semantic,
state->jitCode->common.addUniqueCallSiteIndex(node->origin.semantic),
- params.usedRegisters(), JSValueRegs(params[0].gpr()), JSValueRegs(params[1].gpr()),
- GPRInfo::patchpointScratchRegister, ecmaMode,
+ params.unavailableRegisters(), JSValueRegs(params[0].gpr()),
+ JSValueRegs(params[1].gpr()), GPRInfo::patchpointScratchRegister, ecmaMode,
node->op() == PutByIdDirect ? Direct : NotDirect);
generator->generateFastPath(jit);
@@ -2603,8 +2603,8 @@
generator->slowPathJump().link(&jit);
CCallHelpers::Label slowPathBegin = jit.label();
CCallHelpers::Call slowPathCall = callOperation(
- *state, params.usedRegisters(), jit, node->origin.semantic, &exceptions,
- generator->slowPathFunction(), InvalidGPRReg,
+ *state, params.unavailableRegisters(), jit, node->origin.semantic,
+ &exceptions, generator->slowPathFunction(), InvalidGPRReg,
CCallHelpers::TrustedImmPtr(generator->stubInfo()), params[1].gpr(),
params[0].gpr(), CCallHelpers::TrustedImmPtr(uid)).call();
jit.jump().linkTo(done, &jit);
@@ -7091,7 +7091,8 @@
auto generator = Box<JITGetByIdGenerator>::create(
jit.codeBlock(), node->origin.semantic,
state->jitCode->common.addUniqueCallSiteIndex(node->origin.semantic),
- params.usedRegisters(), JSValueRegs(params[1].gpr()), JSValueRegs(params[0].gpr()));
+ params.unavailableRegisters(), JSValueRegs(params[1].gpr()),
+ JSValueRegs(params[0].gpr()));
generator->generateFastPath(jit);
CCallHelpers::Label done = jit.label();
@@ -7106,8 +7107,8 @@
generator->slowPathJump().link(&jit);
CCallHelpers::Label slowPathBegin = jit.label();
CCallHelpers::Call slowPathCall = callOperation(
- *state, params.usedRegisters(), jit, node->origin.semantic, &exceptions,
- operationGetByIdOptimize, params[0].gpr(),
+ *state, params.unavailableRegisters(), jit, node->origin.semantic,
+ &exceptions, operationGetByIdOptimize, params[0].gpr(),
CCallHelpers::TrustedImmPtr(generator->stubInfo()), params[1].gpr(),
CCallHelpers::TrustedImmPtr(uid)).call();
jit.jump().linkTo(done, &jit);
@@ -8516,7 +8517,7 @@
CCallHelpers::PatchableJump patchableJump = jit.patchableJump();
CCallHelpers::Label done = jit.label();
- RegisterSet usedRegisters = params.usedRegisters();
+ RegisterSet usedRegisters = params.unavailableRegisters();
// FIXME: As part of handling exceptions, we need to create a concrete OSRExit here.
// Doing so should automagically register late paths that emit exit thunks.
@@ -8544,7 +8545,7 @@
generatorJump, CodeLocationLabel(
vm->getCTIStub(
lazySlowPathGenerationThunkGenerator).code()));
-
+
CodeLocationJump linkedPatchableJump = CodeLocationJump(
linkBuffer.locationOf(patchableJump));
CodeLocationLabel linkedDone = linkBuffer.locationOf(done);
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
