Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (262561 => 262562)
--- trunk/Source/_javascript_Core/ChangeLog 2020-06-04 19:52:52 UTC (rev 262561)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-06-04 19:54:17 UTC (rev 262562)
@@ -1,3 +1,59 @@
+2020-06-04 Mark Lam <[email protected]>
+
+ Reduce DFGDoesGCCheck to only storing a uint32_t.
+ https://bugs.webkit.org/show_bug.cgi?id=212734
+
+ Reviewed by Saam Barati and Caio Lima.
+
+ This patch changes the encoding of DoesGCCheck so that it will fit better in a
+ uint32_t. This has the following benefits:
+ 1. speed improvement for debug builds because it now takes less instructions
+ (especially in JITted code) to store to DoesGCCheck::m_value.
+ 2. enables this check for 32-bit platforms as well.
+
+ Fun fact: we currently have 373 DFG::NodeTypes. Hence, 9 bits for nodeOp.
+
+ The new encoding provides 21 bis for the nodeIndex. This gives us up to 2097152
+ node indexes. In my experience, I've never seen more than 3 decimal digits for
+ the nodeIndex so far. If we ever find that we need more than 21 bits of nodeIndex,
+ we have 2 options to deal with it:
+
+ 1. We can just ignore the high bits. After all, it is the nodeOp that is the
+ most interesting piece of data we need to debug doesGC issues.
+
+ 2. We can make DoesGCCheck use uint64_t for storage. This encoding automatically
+ scales to 64-bit, while still allowing the more efficient form of storing a
+ 32-bit immediate to be used for the common cases.
+
+ This patch also makes ENABLE_DFG_DOES_GC_VALIDATION dependent on ENABLE(DFG_JIT).
+ DoesGC is only relevant for the DFG and FTL JITs.
+
+ * dfg/DFGDoesGCCheck.cpp:
+ (JSC::DFG::DoesGCCheck::verifyCanGC):
+ * dfg/DFGDoesGCCheck.h:
+ (JSC::DFG::DoesGCCheck::encode):
+ (JSC::DFG::DoesGCCheck::expectDoesGC const):
+ (JSC::DFG::DoesGCCheck::isSpecial const):
+ (JSC::DFG::DoesGCCheck::special):
+ (JSC::DFG::DoesGCCheck::nodeOp):
+ (JSC::DFG::DoesGCCheck::nodeIndex):
+ (JSC::DFG::DoesGCCheck::expectDoesGC): Deleted.
+ (JSC::DFG::DoesGCCheck::isSpecial): Deleted.
+ (JSC::DFG::DoesGCCheck::specialIndex): Deleted.
+ (JSC::DFG::DoesGCCheck::bits): Deleted.
+ * dfg/DFGNodeType.h:
+ * dfg/DFGOSRExit.cpp:
+ (JSC::DFG::OSRExit::compileExit):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * ftl/FTLLowerDFGToB3.cpp:
+ (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+ * ftl/FTLOSRExitCompiler.cpp:
+ (JSC::FTL::compileStub):
+ * heap/Heap.h:
+
2020-06-04 Tim Horton <[email protected]>
Work around broken system version macro
Modified: trunk/Source/_javascript_Core/dfg/DFGDoesGCCheck.cpp (262561 => 262562)
--- trunk/Source/_javascript_Core/dfg/DFGDoesGCCheck.cpp 2020-06-04 19:52:52 UTC (rev 262561)
+++ trunk/Source/_javascript_Core/dfg/DFGDoesGCCheck.cpp 2020-06-04 19:54:17 UTC (rev 262562)
@@ -28,6 +28,7 @@
#include "CallFrame.h"
#include "CodeBlock.h"
+#include "DFGNodeType.h"
#include "Heap.h"
#include "VMInspector.h"
#include <wtf/DataLog.h>
@@ -41,6 +42,10 @@
void DoesGCCheck::verifyCanGC(VM& vm)
{
+ // We do this check here just so we don't have to #include DFGNodeType.h
+ // in the header file.
+ static_assert(numberOfNodeTypes <= (1 << nodeOpBits));
+
if (!expectDoesGC()) {
dataLog("Error: DoesGC failed");
if (isSpecial()) {
@@ -53,7 +58,6 @@
case Special::FTLOSRExit:
dataLog(" @ FTL osr exit");
break;
- case Special::ReservedUnused:
case Special::NumberOfSpecials:
RELEASE_ASSERT_NOT_REACHED();
}
Modified: trunk/Source/_javascript_Core/dfg/DFGDoesGCCheck.h (262561 => 262562)
--- trunk/Source/_javascript_Core/dfg/DFGDoesGCCheck.h 2020-06-04 19:52:52 UTC (rev 262561)
+++ trunk/Source/_javascript_Core/dfg/DFGDoesGCCheck.h 2020-06-04 19:54:17 UTC (rev 262562)
@@ -36,7 +36,6 @@
struct DoesGCCheck {
enum class Special {
- ReservedUnused, // so that specials start with 1. isSpecial() relies on this.
Uninitialized,
DFGOSRExit,
FTLOSRExit,
@@ -47,20 +46,16 @@
: m_value(encode(true, Special::Uninitialized))
{ }
- static uint64_t encode(bool expectDoesGC, unsigned nodeIndex, unsigned nodeOp)
+ static uint32_t encode(bool expectDoesGC, unsigned nodeIndex, unsigned nodeOp)
{
- uint64_t result = bits(nodeIndex) << nodeIndexShift;
- result |= bits(nodeOp) << nodeOpShift;
- result |= bits(expectDoesGC) << expectDoesGCShift;
- return result;
+ // We know nodeOp always fits because of the static_assert in DFGDoesGCCheck.cpp.
+ ASSERT((nodeIndex << nodeIndexShift) >> nodeIndexShift == nodeIndex);
+ return nodeIndex << nodeIndexShift | nodeOp << nodeOpShift | bits(expectDoesGC);
}
- static uint64_t encode(bool expectDoesGC, Special special)
+ static uint32_t encode(bool expectDoesGC, Special special)
{
- ASSERT(bits(special) < numberOfSpecials);
- uint64_t result = bits(special) << specialShift;
- result |= bits(expectDoesGC) << expectDoesGCShift;
- return result;
+ return bits(special) << specialShift | isSpecialBit | bits(expectDoesGC);
}
void set(bool expectDoesGC, unsigned nodeIndex, unsigned nodeOp)
@@ -73,32 +68,39 @@
m_value = encode(expectDoesGC, special);
}
- bool expectDoesGC() { return m_value & expectDoesGCMask; }
- Special special() { return static_cast<Special>(specialIndex()); }
+ bool expectDoesGC() const { return m_value & expectDoesGCBit; }
+ bool isSpecial() const { return m_value & isSpecialBit; }
+
+ Special special() { return static_cast<Special>(m_value >> specialShift); }
+
+ unsigned nodeOp() { return (m_value >> nodeOpShift) & nodeOpMask; }
unsigned nodeIndex() { return m_value >> nodeIndexShift; }
- unsigned nodeOp() { return (m_value >> nodeOpShift) & nodeOpMask; }
- bool isSpecial() { return specialIndex(); }
-
JS_EXPORT_PRIVATE void verifyCanGC(VM&);
private:
- unsigned specialIndex() { return (m_value >> specialShift) & specialMask; }
+ template<typename T> static uint32_t bits(T value) { return static_cast<uint32_t>(value); }
- template<typename T> static uint64_t bits(T value) { return static_cast<uint64_t>(value); }
+ // The value cannot be both a Special and contain node information at the
+ // time. Hence, the 2 can have separate encodings. The isSpecial bit
+ // determines which encoding is in use.
- static constexpr uint64_t numberOfSpecials = static_cast<uint64_t>(Special::NumberOfSpecials);
+ static constexpr unsigned expectDoesGCBit = 1 << 0;
+ static constexpr unsigned isSpecialBit = 1 << 1;
+ static constexpr unsigned commonBits = 2;
+ static_assert((expectDoesGCBit | isSpecialBit) == (1 << commonBits) - 1);
- static constexpr unsigned expectDoesGCShift = 0;
- static constexpr unsigned specialShift = 1;
- static constexpr unsigned nodeOpShift = 16;
- static constexpr unsigned nodeIndexShift = 32;
+ static constexpr unsigned specialShift = commonBits;
- static constexpr unsigned expectDoesGCMask = 0x1;
- static constexpr unsigned specialMask = 0x7fff;
- static constexpr unsigned nodeOpMask = 0xffff;
+ static constexpr unsigned nodeOpBits = 9;
+ static constexpr unsigned nodeOpMask = (1 << nodeOpBits) - 1;
+ static constexpr unsigned nodeOpShift = commonBits;
- uint64_t m_value { 0 };
+ static constexpr unsigned nodeIndexBits = 21;
+ static constexpr unsigned nodeIndexShift = nodeOpShift + nodeOpBits;
+ static_assert(nodeIndexShift + nodeIndexBits == 32);
+
+ uint32_t m_value { 0 };
};
} // namespace DFG
Modified: trunk/Source/_javascript_Core/dfg/DFGNodeType.h (262561 => 262562)
--- trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2020-06-04 19:52:52 UTC (rev 262561)
+++ trunk/Source/_javascript_Core/dfg/DFGNodeType.h 2020-06-04 19:54:17 UTC (rev 262562)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2020 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -552,6 +552,10 @@
LastNodeType
};
+#define DFG_OP_COUNT(opcode, flags) + 1
+constexpr unsigned numberOfNodeTypes = FOR_EACH_DFG_OP(DFG_OP_COUNT);
+#undef DFG_OP_COUNT
+
// Specifies the default flags for each node.
inline NodeFlags defaultFlags(NodeType op)
{
Modified: trunk/Source/_javascript_Core/dfg/DFGOSRExit.cpp (262561 => 262562)
--- trunk/Source/_javascript_Core/dfg/DFGOSRExit.cpp 2020-06-04 19:52:52 UTC (rev 262561)
+++ trunk/Source/_javascript_Core/dfg/DFGOSRExit.cpp 2020-06-04 19:54:17 UTC (rev 262562)
@@ -560,7 +560,7 @@
// to set it here because compileOSRExit() is only called on the first time
// we exit from this site, but all subsequent exits will take this compiled
// ramp without calling compileOSRExit() first.
- jit.store64(CCallHelpers::TrustedImm64(DoesGCCheck::encode(true, DoesGCCheck::Special::DFGOSRExit)), vm.heap.addressOfDoesGC());
+ jit.store32(CCallHelpers::TrustedImm32(DoesGCCheck::encode(true, DoesGCCheck::Special::DFGOSRExit)), vm.heap.addressOfDoesGC());
}
#endif
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp (262561 => 262562)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2020-06-04 19:52:52 UTC (rev 262561)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT32_64.cpp 2020-06-04 19:54:17 UTC (rev 262562)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2020 Apple Inc. All rights reserved.
* Copyright (C) 2011 Intel Corporation. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -33,6 +33,7 @@
#include "CallFrameShuffler.h"
#include "DFGAbstractInterpreterInlines.h"
#include "DFGCallArrayAllocatorSlowPathGenerator.h"
+#include "DFGDoesGC.h"
#include "DFGOperations.h"
#include "DFGSlowPathGenerator.h"
#include "DirectArguments.h"
@@ -1806,6 +1807,11 @@
{
NodeType op = node->op();
+ if (validateDFGDoesGC) {
+ bool expectDoesGC = doesGC(m_jit.graph(), node);
+ m_jit.store32(TrustedImm32(DoesGCCheck::encode(expectDoesGC, node->index(), node->op())), vm().heap.addressOfDoesGC());
+ }
+
#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
m_jit.clearRegisterAllocationOffsets();
#endif
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (262561 => 262562)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2020-06-04 19:52:52 UTC (rev 262561)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2020-06-04 19:54:17 UTC (rev 262562)
@@ -2138,7 +2138,7 @@
if (validateDFGDoesGC) {
bool expectDoesGC = doesGC(m_jit.graph(), node);
- m_jit.store64(TrustedImm64(DoesGCCheck::encode(expectDoesGC, node->index(), node->op())), vm().heap.addressOfDoesGC());
+ m_jit.store32(TrustedImm32(DoesGCCheck::encode(expectDoesGC, node->index(), node->op())), vm().heap.addressOfDoesGC());
}
#if ENABLE(DFG_REGISTER_ALLOCATION_VALIDATION)
Modified: trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp (262561 => 262562)
--- trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-06-04 19:52:52 UTC (rev 262561)
+++ trunk/Source/_javascript_Core/ftl/FTLLowerDFGToB3.cpp 2020-06-04 19:54:17 UTC (rev 262562)
@@ -704,7 +704,7 @@
if (validateDFGDoesGC) {
bool expectDoesGC = doesGC(m_graph, m_node);
- m_out.store(m_out.constInt64(DoesGCCheck::encode(expectDoesGC, m_node->index(), m_node->op())), m_out.absolute(vm().heap.addressOfDoesGC()));
+ m_out.store(m_out.constInt32(DoesGCCheck::encode(expectDoesGC, m_node->index(), m_node->op())), m_out.absolute(vm().heap.addressOfDoesGC()));
}
switch (m_node->op()) {
Modified: trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp (262561 => 262562)
--- trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp 2020-06-04 19:52:52 UTC (rev 262561)
+++ trunk/Source/_javascript_Core/ftl/FTLOSRExitCompiler.cpp 2020-06-04 19:54:17 UTC (rev 262562)
@@ -215,7 +215,7 @@
// to set it here because compileFTLOSRExit() is only called on the first time
// we exit from this site, but all subsequent exits will take this compiled
// ramp without calling compileFTLOSRExit() first.
- jit.store64(CCallHelpers::TrustedImm64(DoesGCCheck::encode(true, DoesGCCheck::Special::FTLOSRExit)), vm.heap.addressOfDoesGC());
+ jit.store32(CCallHelpers::TrustedImm32(DoesGCCheck::encode(true, DoesGCCheck::Special::FTLOSRExit)), vm.heap.addressOfDoesGC());
}
// Bring the stack back into a sane form and assert that it's sane.
Modified: trunk/Source/_javascript_Core/heap/Heap.h (262561 => 262562)
--- trunk/Source/_javascript_Core/heap/Heap.h 2020-06-04 19:52:52 UTC (rev 262561)
+++ trunk/Source/_javascript_Core/heap/Heap.h 2020-06-04 19:54:17 UTC (rev 262562)
@@ -98,7 +98,12 @@
class Worklist;
}
-#define ENABLE_DFG_DOES_GC_VALIDATION ASSERT_ENABLED
+#if ENABLE(DFG_JIT) && ASSERT_ENABLED
+#define ENABLE_DFG_DOES_GC_VALIDATION 1
+#else
+#define ENABLE_DFG_DOES_GC_VALIDATION 0
+#endif
+
constexpr bool validateDFGDoesGC = ENABLE_DFG_DOES_GC_VALIDATION;
typedef HashCountedSet<JSCell*> ProtectCountSet;
