- Revision
- 204065
- Author
- benja...@webkit.org
- Date
- 2016-08-02 20:45:07 -0700 (Tue, 02 Aug 2016)
Log Message
[JSC] Simplify the initialization of AbstractValue in the AbstractInterpreter
https://bugs.webkit.org/show_bug.cgi?id=160370
Reviewed by Saam Barati.
Source/_javascript_Core:
We use a ton of AbstractValue to run the Abstract Interpreter.
When we set up the initial values, the compiler sets
a zero on a first word, a one on a second word, and a zero
again on a third word.
Since no vector or double-store can deal with 3 words, unrolling
is done by repeating those instructions.
The reason for the one was TinyPtrSet. It needed a flag for
empty value to identify the set as thin. I flipped the flag to "fat"
to make sure TinyPtrSet is initialized to zero.
With that done, I just had to clean some places to make
the initialization shorter.
It makes the binary easier to follow but this does not help with
the bigger problem: the time spent per block on Abstract Interpreter.
* bytecode/Operands.h:
The traits were useless, no client code defines it.
(JSC::Operands::Operands):
(JSC::Operands::ensureLocals):
Because of the size of the function, llvm is not inlining it.
We were literally loading 3 registers from memory and storing
them in the vector.
Now that AbstractValue has a VectorTraits, we should just rely
on the memset of Vector when possible.
(JSC::Operands::getLocal):
(JSC::Operands::setArgumentFirstTime):
(JSC::Operands::setLocalFirstTime):
(JSC::Operands::clear):
(JSC::OperandValueTraits::defaultValue): Deleted.
(JSC::OperandValueTraits::isEmptyForDump): Deleted.
* bytecode/OperandsInlines.h:
(JSC::Operands<T>::dumpInContext):
(JSC::Operands<T>::dump):
(JSC::Traits>::dumpInContext): Deleted.
(JSC::Traits>::dump): Deleted.
* dfg/DFGAbstractValue.cpp:
* dfg/DFGAbstractValue.h:
(JSC::DFG::AbstractValue::AbstractValue):
Source/WTF:
* wtf/TinyPtrSet.h:
(WTF::TinyPtrSet::isThin):
(WTF::TinyPtrSet::set):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (204064 => 204065)
--- trunk/Source/_javascript_Core/ChangeLog 2016-08-03 02:56:22 UTC (rev 204064)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-08-03 03:45:07 UTC (rev 204065)
@@ -1,3 +1,53 @@
+2016-08-02 Benjamin Poulain <benja...@webkit.org>
+
+ [JSC] Simplify the initialization of AbstractValue in the AbstractInterpreter
+ https://bugs.webkit.org/show_bug.cgi?id=160370
+
+ Reviewed by Saam Barati.
+
+ We use a ton of AbstractValue to run the Abstract Interpreter.
+
+ When we set up the initial values, the compiler sets
+ a zero on a first word, a one on a second word, and a zero
+ again on a third word.
+ Since no vector or double-store can deal with 3 words, unrolling
+ is done by repeating those instructions.
+
+ The reason for the one was TinyPtrSet. It needed a flag for
+ empty value to identify the set as thin. I flipped the flag to "fat"
+ to make sure TinyPtrSet is initialized to zero.
+
+ With that done, I just had to clean some places to make
+ the initialization shorter.
+ It makes the binary easier to follow but this does not help with
+ the bigger problem: the time spent per block on Abstract Interpreter.
+
+ * bytecode/Operands.h:
+ The traits were useless, no client code defines it.
+
+ (JSC::Operands::Operands):
+ (JSC::Operands::ensureLocals):
+ Because of the size of the function, llvm is not inlining it.
+ We were literally loading 3 registers from memory and storing
+ them in the vector.
+ Now that AbstractValue has a VectorTraits, we should just rely
+ on the memset of Vector when possible.
+
+ (JSC::Operands::getLocal):
+ (JSC::Operands::setArgumentFirstTime):
+ (JSC::Operands::setLocalFirstTime):
+ (JSC::Operands::clear):
+ (JSC::OperandValueTraits::defaultValue): Deleted.
+ (JSC::OperandValueTraits::isEmptyForDump): Deleted.
+ * bytecode/OperandsInlines.h:
+ (JSC::Operands<T>::dumpInContext):
+ (JSC::Operands<T>::dump):
+ (JSC::Traits>::dumpInContext): Deleted.
+ (JSC::Traits>::dump): Deleted.
+ * dfg/DFGAbstractValue.cpp:
+ * dfg/DFGAbstractValue.h:
+ (JSC::DFG::AbstractValue::AbstractValue):
+
2016-08-02 Saam Barati <sbar...@apple.com>
update a class extending null w.r.t the ES7 spec
Modified: trunk/Source/_javascript_Core/bytecode/Operands.h (204064 => 204065)
--- trunk/Source/_javascript_Core/bytecode/Operands.h 2016-08-03 02:56:22 UTC (rev 204064)
+++ trunk/Source/_javascript_Core/bytecode/Operands.h 2016-08-03 03:45:07 UTC (rev 204065)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011, 2012, 2013, 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2011, 2012, 2013, 2015, 2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -37,32 +37,37 @@
template<typename T> struct OperandValueTraits;
-template<typename T>
-struct OperandValueTraits {
- static T defaultValue() { return T(); }
- static bool isEmptyForDump(const T& value) { return !value; }
-};
-
enum OperandKind { ArgumentOperand, LocalOperand };
enum OperandsLikeTag { OperandsLike };
-template<typename T, typename Traits = OperandValueTraits<T>>
+template<typename T>
class Operands {
public:
Operands() { }
- explicit Operands(size_t numArguments, size_t numLocals, const T& initialValue = Traits::defaultValue())
+ explicit Operands(size_t numArguments, size_t numLocals)
{
+ if (WTF::VectorTraits<T>::needsInitialization) {
+ m_arguments.resize(numArguments);
+ m_locals.resize(numLocals);
+ } else {
+ m_arguments.fill(T(), numArguments);
+ m_locals.fill(T(), numLocals);
+ }
+ }
+
+ explicit Operands(size_t numArguments, size_t numLocals, const T& initialValue)
+ {
m_arguments.fill(initialValue, numArguments);
m_locals.fill(initialValue, numLocals);
}
- template<typename U, typename OtherTraits>
- explicit Operands(OperandsLikeTag, const Operands<U, OtherTraits>& other)
+ template<typename U>
+ explicit Operands(OperandsLikeTag, const Operands<U>& other)
{
- m_arguments.fill(Traits::defaultValue(), other.numberOfArguments());
- m_locals.fill(Traits::defaultValue(), other.numberOfLocals());
+ m_arguments.fill(T(), other.numberOfArguments());
+ m_locals.fill(T(), other.numberOfLocals());
}
size_t numberOfArguments() const { return m_arguments.size(); }
@@ -96,7 +101,7 @@
return local(idx);
}
- void ensureLocals(size_t size, const T& ensuredValue = Traits::defaultValue())
+ void ensureLocals(size_t size)
{
if (size <= m_locals.size())
return;
@@ -103,6 +108,19 @@
size_t oldSize = m_locals.size();
m_locals.resize(size);
+ if (!WTF::VectorTraits<T>::needsInitialization) {
+ for (size_t i = oldSize; i < m_locals.size(); ++i)
+ m_locals[i] = T();
+ }
+ }
+
+ void ensureLocals(size_t size, const T& ensuredValue)
+ {
+ if (size <= m_locals.size())
+ return;
+
+ size_t oldSize = m_locals.size();
+ m_locals.resize(size);
for (size_t i = oldSize; i < m_locals.size(); ++i)
m_locals[i] = ensuredValue;
}
@@ -117,19 +135,19 @@
T getLocal(size_t idx)
{
if (idx >= m_locals.size())
- return Traits::defaultValue();
+ return T();
return m_locals[idx];
}
void setArgumentFirstTime(size_t idx, const T& value)
{
- ASSERT(m_arguments[idx] == Traits::defaultValue());
+ ASSERT(m_arguments[idx] == T());
argument(idx) = value;
}
void setLocalFirstTime(size_t idx, const T& value)
{
- ASSERT(idx >= m_locals.size() || m_locals[idx] == Traits::defaultValue());
+ ASSERT(idx >= m_locals.size() || m_locals[idx] == T());
setLocal(idx, value);
}
@@ -245,7 +263,7 @@
void clear()
{
- fill(Traits::defaultValue());
+ fill(T());
}
bool operator==(const Operands& other) const
Modified: trunk/Source/_javascript_Core/bytecode/OperandsInlines.h (204064 => 204065)
--- trunk/Source/_javascript_Core/bytecode/OperandsInlines.h 2016-08-03 02:56:22 UTC (rev 204064)
+++ trunk/Source/_javascript_Core/bytecode/OperandsInlines.h 2016-08-03 03:45:07 UTC (rev 204065)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -31,33 +31,33 @@
namespace JSC {
-template<typename T, typename Traits>
-void Operands<T, Traits>::dumpInContext(PrintStream& out, DumpContext* context) const
+template<typename T>
+void Operands<T>::dumpInContext(PrintStream& out, DumpContext* context) const
{
CommaPrinter comma(" ");
for (size_t argumentIndex = numberOfArguments(); argumentIndex--;) {
- if (Traits::isEmptyForDump(argument(argumentIndex)))
+ if (!argument(argumentIndex))
continue;
out.print(comma, "arg", argumentIndex, ":", inContext(argument(argumentIndex), context));
}
for (size_t localIndex = 0; localIndex < numberOfLocals(); ++localIndex) {
- if (Traits::isEmptyForDump(local(localIndex)))
+ if (!local(localIndex))
continue;
out.print(comma, "loc", localIndex, ":", inContext(local(localIndex), context));
}
}
-template<typename T, typename Traits>
-void Operands<T, Traits>::dump(PrintStream& out) const
+template<typename T>
+void Operands<T>::dump(PrintStream& out) const
{
CommaPrinter comma(" ");
for (size_t argumentIndex = numberOfArguments(); argumentIndex--;) {
- if (Traits::isEmptyForDump(argument(argumentIndex)))
+ if (!argument(argumentIndex))
continue;
out.print(comma, "arg", argumentIndex, ":", argument(argumentIndex));
}
for (size_t localIndex = 0; localIndex < numberOfLocals(); ++localIndex) {
- if (Traits::isEmptyForDump(local(localIndex)))
+ if (!local(localIndex))
continue;
out.print(comma, "loc", localIndex, ":", local(localIndex));
}
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp (204064 => 204065)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp 2016-08-03 02:56:22 UTC (rev 204064)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractValue.cpp 2016-08-03 03:45:07 UTC (rev 204065)
@@ -34,6 +34,8 @@
namespace JSC { namespace DFG {
+static_assert(sizeof(AbstractValue) == (sizeof(void*) + sizeof(unsigned) * 2 + sizeof(JSValue)), "AbstractValue should be as small as possible.");
+
void AbstractValue::observeTransitions(const TransitionVector& vector)
{
if (m_type & SpecCell) {
Modified: trunk/Source/_javascript_Core/dfg/DFGAbstractValue.h (204064 => 204065)
--- trunk/Source/_javascript_Core/dfg/DFGAbstractValue.h 2016-08-03 02:56:22 UTC (rev 204064)
+++ trunk/Source/_javascript_Core/dfg/DFGAbstractValue.h 2016-08-03 03:45:07 UTC (rev 204065)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011-2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -55,6 +55,17 @@
: m_type(SpecNone)
, m_arrayModes(0)
{
+#ifndef NDEBUG
+ // The WTF Traits for AbstractValue allow the initialization of values with bzero().
+ // We verify the correctness of this assumption here.
+ static bool needsDefaultConstructorCheck = true;
+ if (needsDefaultConstructorCheck) {
+ needsDefaultConstructorCheck = false;
+
+ for (unsigned i = 0; i < sizeof(AbstractValue); ++i)
+ ASSERT(!(reinterpret_cast<char*>(this)[i]));
+ }
+#endif
}
void clear()
@@ -456,6 +467,18 @@
} } // namespace JSC::DFG
+namespace WTF {
+template <>
+struct VectorTraits<JSC::DFG::AbstractValue> : VectorTraitsBase<false, JSC::DFG::AbstractValue> {
+ static const bool canInitializeWithMemset = true;
+};
+
+template <>
+struct HashTraits<JSC::DFG::AbstractValue> : GenericHashTraits<JSC::DFG::AbstractValue> {
+ static const bool emptyValueIsZero = true;
+};
+};
+
#endif // ENABLE(DFG_JIT)
#endif // DFGAbstractValue_h
Modified: trunk/Source/WTF/ChangeLog (204064 => 204065)
--- trunk/Source/WTF/ChangeLog 2016-08-03 02:56:22 UTC (rev 204064)
+++ trunk/Source/WTF/ChangeLog 2016-08-03 03:45:07 UTC (rev 204065)
@@ -1,3 +1,14 @@
+2016-08-02 Benjamin Poulain <benja...@webkit.org>
+
+ [JSC] Simplify the initialization of AbstractValue in the AbstractInterpreter
+ https://bugs.webkit.org/show_bug.cgi?id=160370
+
+ Reviewed by Saam Barati.
+
+ * wtf/TinyPtrSet.h:
+ (WTF::TinyPtrSet::isThin):
+ (WTF::TinyPtrSet::set):
+
2016-08-02 Benjamin Poulain <bpoul...@apple.com>
Clean up some useless AtomicString atoms
Modified: trunk/Source/WTF/wtf/TinyPtrSet.h (204064 => 204065)
--- trunk/Source/WTF/wtf/TinyPtrSet.h 2016-08-03 02:56:22 UTC (rev 204064)
+++ trunk/Source/WTF/wtf/TinyPtrSet.h 2016-08-03 03:45:07 UTC (rev 204065)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2015 Apple Inc. All rights reserved.
+ * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -371,9 +371,9 @@
private:
friend class JSC::DFG::StructureAbstractValue;
- static const uintptr_t thinFlag = 1;
+ static const uintptr_t fatFlag = 1;
static const uintptr_t reservedFlag = 2;
- static const uintptr_t flags = thinFlag | reservedFlag;
+ static const uintptr_t flags = fatFlag | reservedFlag;
static const uintptr_t reservedValue = 4;
static const unsigned defaultStartingSize = 4;
@@ -463,7 +463,7 @@
OutOfLineList::destroy(list());
}
- bool isThin() const { return m_pointer & thinFlag; }
+ bool isThin() const { return !(m_pointer & fatFlag); }
void* pointer() const
{
@@ -496,7 +496,7 @@
}
void set(uintptr_t pointer, bool singleEntry)
{
- m_pointer = pointer | (singleEntry ? thinFlag : 0) | (m_pointer & reservedFlag);
+ m_pointer = pointer | (singleEntry ? 0 : fatFlag) | (m_pointer & reservedFlag);
}
bool getReservedFlag() const { return m_pointer & reservedFlag; }
void setReservedFlag(bool value)
