Log Message
FTL B3 should account for localsOffset https://bugs.webkit.org/show_bug.cgi?id=152288
Reviewed by Saam Barati.
The DFG will build up some data structures that expect to know about offsets from FP. Those data
structures may slide by some offset when the low-level compiler (either LLVM or B3) does stack
allocation. So, the LLVM FTL modifies those data structures based on the real offset that it gets
from LLVM's stackmaps. The B3 code needs to do the same.
I had previously vowed to never put more stuff into FTLB3Compile.cpp, because I didn't want it to
look like FTLCompile.cpp. Up until now, I was successful because I used lambdas installed by
FTLLower. But in this case, I actually think that having code that just does this explicitly in
FTLB3Compile.cpp is least confusing. There is no particular place in FTLLower that would want to
care about this, and we need to ensure that we do this fixup before we run any of the stackmap
generators. In other words, it needs to happen before we call B3::generate(). The ordering
constraints seem like a good reason to have this done explicitly rather than through lambdas.
I wrote a test. The test was failing in trunk because the B3 meaning of anchor().value() is
different from the LLVM meaning. This caused breakage when we used this idiom:
ValueFromBlock foo = m_out.anchor(things);
...(foo.value()) // we were expecting that foo.value() == things
I never liked this idiom to begin with, so instead of trying to change B3's anchor(), I changed
the idiom to:
LValue fooValue = things;
ValueFromBlock foo = m_out.anchor(fooValue);
...(fooValue)
This is probably a good idea, since eventually we want B3's anchor() to just return the
UpsilonValue*. To get there, we want to eliminate any situations where code assumes that
ValueFromBlock is an actual object and not just a typedef for a pointer.
* ftl/FTLB3Compile.cpp:
(JSC::FTL::compile):
* ftl/FTLB3Output.cpp:
(JSC::FTL::Output::appendTo):
(JSC::FTL::Output::lockedStackSlot):
* ftl/FTLB3Output.h:
(JSC::FTL::Output::framePointer):
(JSC::FTL::Output::constBool):
(JSC::FTL::Output::constInt32):
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::lower):
(JSC::FTL::DFG::LowerDFGToLLVM::compileGetIndexedPropertyStorage):
(JSC::FTL::DFG::LowerDFGToLLVM::compileGetByVal):
(JSC::FTL::DFG::LowerDFGToLLVM::compileCreateDirectArguments):
(JSC::FTL::DFG::LowerDFGToLLVM::compileStringCharAt):
(JSC::FTL::DFG::LowerDFGToLLVM::compileForwardVarargs):
(JSC::FTL::DFG::LowerDFGToLLVM::compileHasIndexedProperty):
(JSC::FTL::DFG::LowerDFGToLLVM::allocateJSArray):
(JSC::FTL::DFG::LowerDFGToLLVM::sensibleDoubleToInt32):
* ftl/FTLState.h:
(JSC::FTL::verboseCompilationEnabled):
* tests/stress/ftl-function-dot-arguments-with-callee-saves.js: Added.
Modified Paths
- trunk/Source/_javascript_Core/ChangeLog
- trunk/Source/_javascript_Core/ftl/FTLB3Compile.cpp
- trunk/Source/_javascript_Core/ftl/FTLB3Output.cpp
- trunk/Source/_javascript_Core/ftl/FTLB3Output.h
- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp
- trunk/Source/_javascript_Core/ftl/FTLState.h
Added Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (194091 => 194092)
--- trunk/Source/_javascript_Core/ChangeLog 2015-12-15 04:43:03 UTC (rev 194091)
+++ trunk/Source/_javascript_Core/ChangeLog 2015-12-15 04:52:55 UTC (rev 194092)
@@ -1,3 +1,63 @@
+2015-12-14 Filip Pizlo <[email protected]>
+
+ FTL B3 should account for localsOffset
+ https://bugs.webkit.org/show_bug.cgi?id=152288
+
+ Reviewed by Saam Barati.
+
+ The DFG will build up some data structures that expect to know about offsets from FP. Those data
+ structures may slide by some offset when the low-level compiler (either LLVM or B3) does stack
+ allocation. So, the LLVM FTL modifies those data structures based on the real offset that it gets
+ from LLVM's stackmaps. The B3 code needs to do the same.
+
+ I had previously vowed to never put more stuff into FTLB3Compile.cpp, because I didn't want it to
+ look like FTLCompile.cpp. Up until now, I was successful because I used lambdas installed by
+ FTLLower. But in this case, I actually think that having code that just does this explicitly in
+ FTLB3Compile.cpp is least confusing. There is no particular place in FTLLower that would want to
+ care about this, and we need to ensure that we do this fixup before we run any of the stackmap
+ generators. In other words, it needs to happen before we call B3::generate(). The ordering
+ constraints seem like a good reason to have this done explicitly rather than through lambdas.
+
+ I wrote a test. The test was failing in trunk because the B3 meaning of anchor().value() is
+ different from the LLVM meaning. This caused breakage when we used this idiom:
+
+ ValueFromBlock foo = m_out.anchor(things);
+ ...(foo.value()) // we were expecting that foo.value() == things
+
+ I never liked this idiom to begin with, so instead of trying to change B3's anchor(), I changed
+ the idiom to:
+
+ LValue fooValue = things;
+ ValueFromBlock foo = m_out.anchor(fooValue);
+ ...(fooValue)
+
+ This is probably a good idea, since eventually we want B3's anchor() to just return the
+ UpsilonValue*. To get there, we want to eliminate any situations where code assumes that
+ ValueFromBlock is an actual object and not just a typedef for a pointer.
+
+ * ftl/FTLB3Compile.cpp:
+ (JSC::FTL::compile):
+ * ftl/FTLB3Output.cpp:
+ (JSC::FTL::Output::appendTo):
+ (JSC::FTL::Output::lockedStackSlot):
+ * ftl/FTLB3Output.h:
+ (JSC::FTL::Output::framePointer):
+ (JSC::FTL::Output::constBool):
+ (JSC::FTL::Output::constInt32):
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::DFG::LowerDFGToLLVM::lower):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileGetIndexedPropertyStorage):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileGetByVal):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileCreateDirectArguments):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileStringCharAt):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileForwardVarargs):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileHasIndexedProperty):
+ (JSC::FTL::DFG::LowerDFGToLLVM::allocateJSArray):
+ (JSC::FTL::DFG::LowerDFGToLLVM::sensibleDoubleToInt32):
+ * ftl/FTLState.h:
+ (JSC::FTL::verboseCompilationEnabled):
+ * tests/stress/ftl-function-dot-arguments-with-callee-saves.js: Added.
+
2015-12-14 Yusuke Suzuki <[email protected]>
Math.random should have an intrinsic thunk and it should be later handled as a DFG Node
Modified: trunk/Source/_javascript_Core/ftl/FTLB3Compile.cpp (194091 => 194092)
--- trunk/Source/_javascript_Core/ftl/FTLB3Compile.cpp 2015-12-15 04:43:03 UTC (rev 194091)
+++ trunk/Source/_javascript_Core/ftl/FTLB3Compile.cpp 2015-12-15 04:52:55 UTC (rev 194092)
@@ -30,6 +30,7 @@
#include "B3Generate.h"
#include "B3ProcedureInlines.h"
+#include "B3StackSlotValue.h"
#include "CodeBlockWithJITType.h"
#include "CCallHelpers.h"
#include "DFGCommon.h"
@@ -80,6 +81,34 @@
ASSERT(!(state.proc->frameSize() % sizeof(EncodedJSValue)));
state.jitCode->common.frameRegisterCount = state.proc->frameSize() / sizeof(EncodedJSValue);
+ int localsOffset =
+ state.capturedValue->offsetFromFP() / sizeof(EncodedJSValue) + graph.m_nextMachineLocal;
+ for (unsigned i = graph.m_inlineVariableData.size(); i--;) {
+ InlineCallFrame* inlineCallFrame = graph.m_inlineVariableData[i].inlineCallFrame;
+
+ if (inlineCallFrame->argumentCountRegister.isValid())
+ inlineCallFrame->argumentCountRegister += localsOffset;
+
+ for (unsigned argument = inlineCallFrame->arguments.size(); argument-- > 1;) {
+ inlineCallFrame->arguments[argument] =
+ inlineCallFrame->arguments[argument].withLocalsOffset(localsOffset);
+ }
+
+ if (inlineCallFrame->isClosureCall) {
+ inlineCallFrame->calleeRecovery =
+ inlineCallFrame->calleeRecovery.withLocalsOffset(localsOffset);
+ }
+
+ if (graph.hasDebuggerEnabled())
+ codeBlock->setScopeRegister(codeBlock->scopeRegister() + localsOffset);
+ }
+ for (OSRExitDescriptor& descriptor : state.jitCode->osrExitDescriptors) {
+ for (unsigned i = descriptor.m_values.size(); i--;)
+ descriptor.m_values[i] = descriptor.m_values[i].withLocalsOffset(localsOffset);
+ for (ExitTimeObjectMaterialization* materialization : descriptor.m_materializations)
+ materialization->accountForLocalsOffset(localsOffset);
+ }
+
CCallHelpers jit(&vm, codeBlock);
B3::generate(*state.proc, jit);
Modified: trunk/Source/_javascript_Core/ftl/FTLB3Output.cpp (194091 => 194092)
--- trunk/Source/_javascript_Core/ftl/FTLB3Output.cpp 2015-12-15 04:43:03 UTC (rev 194091)
+++ trunk/Source/_javascript_Core/ftl/FTLB3Output.cpp 2015-12-15 04:52:55 UTC (rev 194092)
@@ -59,7 +59,7 @@
m_block = block;
}
-LValue Output::lockedStackSlot(size_t bytes)
+B3::StackSlotValue* Output::lockedStackSlot(size_t bytes)
{
return m_block->appendNew<B3::StackSlotValue>(m_proc, origin(), bytes, B3::StackSlotKind::Locked);
}
Modified: trunk/Source/_javascript_Core/ftl/FTLB3Output.h (194091 => 194092)
--- trunk/Source/_javascript_Core/ftl/FTLB3Output.h 2015-12-15 04:43:03 UTC (rev 194091)
+++ trunk/Source/_javascript_Core/ftl/FTLB3Output.h 2015-12-15 04:52:55 UTC (rev 194092)
@@ -103,7 +103,7 @@
LValue framePointer() { return m_block->appendNew<B3::Value>(m_proc, B3::FramePointer, origin()); }
- LValue lockedStackSlot(size_t bytes);
+ B3::StackSlotValue* lockedStackSlot(size_t bytes);
LValue constBool(bool value) { return m_block->appendNew<B3::Const32Value>(m_proc, origin(), value); }
LValue constInt32(int32_t value) { return m_block->appendNew<B3::Const32Value>(m_proc, origin(), value); }
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp (194091 => 194092)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-12-15 04:43:03 UTC (rev 194091)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToLLVM.cpp 2015-12-15 04:52:55 UTC (rev 194092)
@@ -187,9 +187,9 @@
#if FTL_USES_B3
size_t sizeOfCaptured = sizeof(JSValue) * m_graph.m_nextMachineLocal;
- LValue capturedBase = m_out.lockedStackSlot(sizeOfCaptured);
+ B3::StackSlotValue* capturedBase = m_out.lockedStackSlot(sizeOfCaptured);
m_captured = m_out.add(capturedBase, m_out.constIntPtr(sizeOfCaptured));
- m_ftlState.capturedValue = m_captured;
+ m_ftlState.capturedValue = capturedBase;
#else // FTL_USES_B3
LValue capturedAlloca = m_out.alloca(arrayType(m_out.int64, m_graph.m_nextMachineLocal));
@@ -2633,12 +2633,12 @@
if (m_node->arrayMode().type() == Array::String) {
LBasicBlock slowPath = FTL_NEW_BLOCK(m_out, ("GetIndexedPropertyStorage String slow case"));
LBasicBlock continuation = FTL_NEW_BLOCK(m_out, ("GetIndexedPropertyStorage String continuation"));
+
+ LValue fastResultValue = m_out.loadPtr(cell, m_heaps.JSString_value);
+ ValueFromBlock fastResult = m_out.anchor(fastResultValue);
- ValueFromBlock fastResult = m_out.anchor(
- m_out.loadPtr(cell, m_heaps.JSString_value));
-
m_out.branch(
- m_out.notNull(fastResult.value()), usually(continuation), rarely(slowPath));
+ m_out.notNull(fastResultValue), usually(continuation), rarely(slowPath));
LBasicBlock lastNext = m_out.appendTo(slowPath, continuation);
@@ -2793,11 +2793,11 @@
rarely(slowCase), usually(fastCase));
LBasicBlock lastNext = m_out.appendTo(fastCase, slowCase);
-
- ValueFromBlock fastResult = m_out.anchor(
- m_out.load64(baseIndex(heap, storage, index, m_node->child2())));
+
+ LValue fastResultValue = m_out.load64(baseIndex(heap, storage, index, m_node->child2()));
+ ValueFromBlock fastResult = m_out.anchor(fastResultValue);
m_out.branch(
- m_out.isZero64(fastResult.value()), rarely(slowCase), usually(continuation));
+ m_out.isZero64(fastResultValue), rarely(slowCase), usually(continuation));
m_out.appendTo(slowCase, continuation);
ValueFromBlock slowResult = m_out.anchor(
@@ -3677,18 +3677,20 @@
LBasicBlock loop = FTL_NEW_BLOCK(m_out, ("CreateDirectArguments loop body"));
LBasicBlock end = FTL_NEW_BLOCK(m_out, ("CreateDirectArguments loop end"));
-
+
ValueFromBlock originalLength;
if (minCapacity) {
LValue capacity = m_out.select(
m_out.aboveOrEqual(length.value, m_out.constInt32(minCapacity)),
length.value,
m_out.constInt32(minCapacity));
- originalLength = m_out.anchor(m_out.zeroExtPtr(capacity));
+ LValue originalLengthValue = m_out.zeroExtPtr(capacity);
+ originalLength = m_out.anchor(originalLengthValue);
m_out.jump(loop);
} else {
- originalLength = m_out.anchor(m_out.zeroExtPtr(length.value));
- m_out.branch(m_out.isNull(originalLength.value()), unsure(end), unsure(loop));
+ LValue originalLengthValue = m_out.zeroExtPtr(length.value);
+ originalLength = m_out.anchor(originalLengthValue);
+ m_out.branch(m_out.isNull(originalLengthValue), unsure(end), unsure(loop));
}
lastNext = m_out.appendTo(loop, end);
@@ -4256,13 +4258,14 @@
m_out.jump(bitsContinuation);
m_out.appendTo(is16Bit, bigCharacter);
-
- ValueFromBlock char16Bit = m_out.anchor(
- m_out.load16ZeroExt32(m_out.baseIndex(
+
+ LValue char16BitValue = m_out.load16ZeroExt32(
+ m_out.baseIndex(
m_heaps.characters16, storage, m_out.zeroExtPtr(index),
- provenValue(m_node->child2()))));
+ provenValue(m_node->child2())));
+ ValueFromBlock char16Bit = m_out.anchor(char16BitValue);
m_out.branch(
- m_out.aboveOrEqual(char16Bit.value(), m_out.constInt32(0x100)),
+ m_out.aboveOrEqual(char16BitValue, m_out.constInt32(0x100)),
rarely(bigCharacter), usually(bitsContinuation));
m_out.appendTo(bigCharacter, bitsContinuation);
@@ -4270,7 +4273,7 @@
Vector<ValueFromBlock, 4> results;
results.append(m_out.anchor(vmCall(
m_out.int64, m_out.operation(operationSingleCharacterString),
- m_callFrame, char16Bit.value())));
+ m_callFrame, char16BitValue)));
m_out.jump(continuation);
m_out.appendTo(bitsContinuation, slowPath);
@@ -5141,9 +5144,10 @@
LBasicBlock continuation = FTL_NEW_BLOCK(m_out, ("ForwardVarargs continuation"));
LValue lengthAsPtr = m_out.zeroExtPtr(length);
- ValueFromBlock loopBound = m_out.anchor(m_out.constIntPtr(data->mandatoryMinimum));
+ LValue loopBoundValue = m_out.constIntPtr(data->mandatoryMinimum);
+ ValueFromBlock loopBound = m_out.anchor(loopBoundValue);
m_out.branch(
- m_out.above(loopBound.value(), lengthAsPtr), unsure(undefinedLoop), unsure(mainLoopEntry));
+ m_out.above(loopBoundValue, lengthAsPtr), unsure(undefinedLoop), unsure(mainLoopEntry));
LBasicBlock lastNext = m_out.appendTo(undefinedLoop, mainLoopEntry);
LValue previousIndex = m_out.phi(m_out.intPtr, loopBound);
@@ -5158,7 +5162,7 @@
m_out.appendTo(mainLoopEntry, mainLoop);
loopBound = m_out.anchor(lengthAsPtr);
- m_out.branch(m_out.notNull(loopBound.value()), unsure(mainLoop), unsure(continuation));
+ m_out.branch(m_out.notNull(lengthAsPtr), unsure(mainLoop), unsure(continuation));
m_out.appendTo(mainLoop, continuation);
previousIndex = m_out.phi(m_out.intPtr, loopBound);
@@ -5800,10 +5804,11 @@
} else
m_out.jump(checkHole);
- LBasicBlock lastNext = m_out.appendTo(checkHole, slowCase);
- ValueFromBlock checkHoleResult = m_out.anchor(
- m_out.notZero64(m_out.load64(baseIndex(heap, storage, index, m_node->child2()))));
- m_out.branch(checkHoleResult.value(), usually(continuation), rarely(slowCase));
+ LBasicBlock lastNext = m_out.appendTo(checkHole, slowCase);
+ LValue checkHoleResultValue =
+ m_out.notZero64(m_out.load64(baseIndex(heap, storage, index, m_node->child2())));
+ ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
+ m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
m_out.appendTo(slowCase, continuation);
ValueFromBlock slowResult = m_out.anchor(m_out.equal(
@@ -5836,8 +5841,9 @@
LBasicBlock lastNext = m_out.appendTo(checkHole, slowCase);
LValue doubleValue = m_out.loadDouble(baseIndex(heap, storage, index, m_node->child2()));
- ValueFromBlock checkHoleResult = m_out.anchor(m_out.doubleEqual(doubleValue, doubleValue));
- m_out.branch(checkHoleResult.value(), usually(continuation), rarely(slowCase));
+ LValue checkHoleResultValue = m_out.doubleEqual(doubleValue, doubleValue);
+ ValueFromBlock checkHoleResult = m_out.anchor(checkHoleResultValue);
+ m_out.branch(checkHoleResultValue, usually(continuation), rarely(slowCase));
m_out.appendTo(slowCase, continuation);
ValueFromBlock slowResult = m_out.anchor(m_out.equal(
@@ -7201,7 +7207,7 @@
});
ValueFromBlock slowArray = m_out.anchor(slowArrayValue);
ValueFromBlock slowButterfly = m_out.anchor(
- m_out.loadPtr(slowArray.value(), m_heaps.JSObject_butterfly));
+ m_out.loadPtr(slowArrayValue, m_heaps.JSObject_butterfly));
m_out.jump(continuation);
@@ -7981,11 +7987,11 @@
{
LBasicBlock slowPath = FTL_NEW_BLOCK(m_out, ("sensible doubleToInt32 slow path"));
LBasicBlock continuation = FTL_NEW_BLOCK(m_out, ("sensible doubleToInt32 continuation"));
-
- ValueFromBlock fastResult = m_out.anchor(
- m_out.sensibleDoubleToInt(doubleValue));
+
+ LValue fastResultValue = m_out.sensibleDoubleToInt(doubleValue);
+ ValueFromBlock fastResult = m_out.anchor(fastResultValue);
m_out.branch(
- m_out.equal(fastResult.value(), m_out.constInt32(0x80000000)),
+ m_out.equal(fastResultValue, m_out.constInt32(0x80000000)),
rarely(slowPath), usually(continuation));
LBasicBlock lastNext = m_out.appendTo(slowPath, continuation);
Modified: trunk/Source/_javascript_Core/ftl/FTLState.h (194091 => 194092)
--- trunk/Source/_javascript_Core/ftl/FTLState.h 2015-12-15 04:43:03 UTC (rev 194091)
+++ trunk/Source/_javascript_Core/ftl/FTLState.h 2015-12-15 04:52:55 UTC (rev 194092)
@@ -42,8 +42,15 @@
#include "FTLStackMaps.h"
#include <wtf/Noncopyable.h>
-namespace JSC { namespace FTL {
+namespace JSC {
+namespace B3 {
+class PatchpointValue;
+class StackSlotValue;
+} // namespace B3
+
+namespace FTL {
+
inline bool verboseCompilationEnabled()
{
return DFG::verboseCompilationEnabled(DFG::FTLMode);
@@ -75,11 +82,11 @@
GeneratedFunction generatedFunction;
JITFinalizer* finalizer;
#if FTL_USES_B3
- LValue handleStackOverflowExceptionValue { nullptr };
- LValue handleExceptionValue { nullptr };
- LValue capturedValue { nullptr };
- LValue varargsSpillSlotsValue { nullptr };
- LValue exceptionHandlingSpillSlotValue { nullptr };
+ B3::PatchpointValue* handleStackOverflowExceptionValue { nullptr };
+ B3::PatchpointValue* handleExceptionValue { nullptr };
+ B3::StackSlotValue* capturedValue { nullptr };
+ B3::StackSlotValue* varargsSpillSlotsValue { nullptr };
+ B3::StackSlotValue* exceptionHandlingSpillSlotValue { nullptr };
#else // FTL_USES_B3
unsigned handleStackOverflowExceptionStackmapID { UINT_MAX };
unsigned handleExceptionStackmapID { UINT_MAX };
Added: trunk/Source/_javascript_Core/tests/stress/ftl-function-dot-arguments-with-callee-saves.js (0 => 194092)
--- trunk/Source/_javascript_Core/tests/stress/ftl-function-dot-arguments-with-callee-saves.js (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/ftl-function-dot-arguments-with-callee-saves.js 2015-12-15 04:52:55 UTC (rev 194092)
@@ -0,0 +1,23 @@
+function baz() {
+ return foo.arguments;
+}
+
+noInline(baz);
+
+function foo() {
+ return baz();
+}
+
+function bar(o, i) {
+ var x = o.f;
+ return [foo(1, 2, 3), x];
+}
+
+noInline(bar);
+
+for (var i = 0; i < 10000; ++i) {
+ var a = bar({f:42});
+ if (a.length != 2 || a[0].length != 3 || a[0][0] != 1 || a[0][1] != 2 || a[0][2] != 3 || a[1] != 42)
+ throw "Error: bad result: " + a;
+}
+
_______________________________________________ webkit-changes mailing list [email protected] https://lists.webkit.org/mailman/listinfo/webkit-changes
