Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (195306 => 195307)
--- trunk/Source/_javascript_Core/ChangeLog 2016-01-19 20:32:01 UTC (rev 195306)
+++ trunk/Source/_javascript_Core/ChangeLog 2016-01-19 20:55:34 UTC (rev 195307)
@@ -1,3 +1,44 @@
+2016-01-19 Filip Pizlo <fpi...@apple.com>
+
+ Reconsider B3's constant motion policy
+ https://bugs.webkit.org/show_bug.cgi?id=152202
+
+ Reviewed by Geoffrey Garen.
+
+ This changes moveConstants() to hoist constants. This is a speed-up on things like mandreel.
+ It has a generally positive impact on the Octane score, but it's within margin of error.
+
+ This also changes IRC to make it a bit more likely to spill constants. We don't want it to
+ spill them too much, because we can't rely on fixObviousSpills() to always replace a load of
+ a constant from the stack with the constant itself, especially in case of instructions that
+ need an extra register to materialize the immediate.
+
+ Also fixed DFG graph dumping to print a bit less things. It was trying to print the results of
+ constant property inference, and this sometimes caused crashes when you dumped the graph at an
+ inopportune time.
+
+ * _javascript_Core.xcodeproj/project.pbxproj:
+ * b3/B3MoveConstants.cpp:
+ * b3/air/AirArg.h:
+ * b3/air/AirArgInlines.h: Added.
+ (JSC::B3::Air::ArgThingHelper<Tmp>::is):
+ (JSC::B3::Air::ArgThingHelper<Tmp>::as):
+ (JSC::B3::Air::ArgThingHelper<Tmp>::forEachFast):
+ (JSC::B3::Air::ArgThingHelper<Tmp>::forEach):
+ (JSC::B3::Air::ArgThingHelper<Arg>::is):
+ (JSC::B3::Air::ArgThingHelper<Arg>::as):
+ (JSC::B3::Air::ArgThingHelper<Arg>::forEachFast):
+ (JSC::B3::Air::ArgThingHelper<Arg>::forEach):
+ (JSC::B3::Air::Arg::is):
+ (JSC::B3::Air::Arg::as):
+ (JSC::B3::Air::Arg::forEachFast):
+ (JSC::B3::Air::Arg::forEach):
+ * b3/air/AirIteratedRegisterCoalescing.cpp:
+ * b3/air/AirUseCounts.h:
+ (JSC::B3::Air::UseCounts::UseCounts):
+ * dfg/DFGGraph.cpp:
+ (JSC::DFG::Graph::dump):
+
2016-01-19 Enrica Casucci <enr...@apple.com>
Add support for DataDetectors in WK (iOS).
Modified: trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj (195306 => 195307)
--- trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj 2016-01-19 20:32:01 UTC (rev 195306)
+++ trunk/Source/_javascript_Core/_javascript_Core.xcodeproj/project.pbxproj 2016-01-19 20:55:34 UTC (rev 195307)
@@ -433,6 +433,7 @@
0F64B2721A784BAF006E4E66 /* BinarySwitch.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F64B2701A784BAF006E4E66 /* BinarySwitch.h */; settings = {ATTRIBUTES = (Private, ); }; };
0F64B2791A7957B2006E4E66 /* CallEdge.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F64B2771A7957B2006E4E66 /* CallEdge.cpp */; };
0F64B27A1A7957B2006E4E66 /* CallEdge.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F64B2781A7957B2006E4E66 /* CallEdge.h */; settings = {ATTRIBUTES = (Private, ); }; };
+ 0F64EAF31C4ECD0600621E9B /* AirArgInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F64EAF21C4ECD0600621E9B /* AirArgInlines.h */; };
0F666EC0183566F900D017F1 /* BytecodeLivenessAnalysisInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F666EBE183566F900D017F1 /* BytecodeLivenessAnalysisInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
0F666EC1183566F900D017F1 /* FullBytecodeLiveness.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F666EBF183566F900D017F1 /* FullBytecodeLiveness.h */; settings = {ATTRIBUTES = (Private, ); }; };
0F666EC61835672B00D017F1 /* DFGAvailability.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F666EC21835672B00D017F1 /* DFGAvailability.cpp */; };
@@ -2601,6 +2602,7 @@
0F64B2701A784BAF006E4E66 /* BinarySwitch.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BinarySwitch.h; sourceTree = "<group>"; };
0F64B2771A7957B2006E4E66 /* CallEdge.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CallEdge.cpp; sourceTree = "<group>"; };
0F64B2781A7957B2006E4E66 /* CallEdge.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CallEdge.h; sourceTree = "<group>"; };
+ 0F64EAF21C4ECD0600621E9B /* AirArgInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = AirArgInlines.h; path = b3/air/AirArgInlines.h; sourceTree = "<group>"; };
0F666EBE183566F900D017F1 /* BytecodeLivenessAnalysisInlines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = BytecodeLivenessAnalysisInlines.h; sourceTree = "<group>"; };
0F666EBF183566F900D017F1 /* FullBytecodeLiveness.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FullBytecodeLiveness.h; sourceTree = "<group>"; };
0F666EC21835672B00D017F1 /* DFGAvailability.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = DFGAvailability.cpp; path = dfg/DFGAvailability.cpp; sourceTree = "<group>"; };
@@ -4850,6 +4852,7 @@
0FEC85491BDACDC70080FF74 /* AirAllocateStack.h */,
0FEC854A1BDACDC70080FF74 /* AirArg.cpp */,
0FEC854B1BDACDC70080FF74 /* AirArg.h */,
+ 0F64EAF21C4ECD0600621E9B /* AirArgInlines.h */,
0FEC854C1BDACDC70080FF74 /* AirBasicBlock.cpp */,
0FEC854D1BDACDC70080FF74 /* AirBasicBlock.h */,
0FB3878B1BFBC44D00E3AB1E /* AirBlockWorklist.h */,
@@ -7506,6 +7509,7 @@
C4703CCE192844CC0013FBEA /* generate_js_backend_commands.py in Headers */,
A5EA710319F6DE6F0098F5EC /* generate_objc_backend_dispatcher_header.py in Headers */,
A5EA710419F6DE720098F5EC /* generate_objc_backend_dispatcher_implementation.py in Headers */,
+ 0F64EAF31C4ECD0600621E9B /* AirArgInlines.h in Headers */,
A5EA710519F6DE740098F5EC /* generate_objc_configuration_header.py in Headers */,
A5EA710619F6DE760098F5EC /* generate_objc_configuration_implementation.py in Headers */,
A5EA710719F6DE780098F5EC /* generate_objc_conversion_helpers.py in Headers */,
Modified: trunk/Source/_javascript_Core/b3/B3MoveConstants.cpp (195306 => 195307)
--- trunk/Source/_javascript_Core/b3/B3MoveConstants.cpp 2016-01-19 20:32:01 UTC (rev 195306)
+++ trunk/Source/_javascript_Core/b3/B3MoveConstants.cpp 2016-01-19 20:55:34 UTC (rev 195307)
@@ -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
@@ -53,27 +53,22 @@
void run()
{
- // This phase uses super simple heuristics to ensure that constants are in profitable places
- // and also lowers constant materialization code. Constants marked "fast" by the client are
- // hoisted to the lowest common dominator. A table is created for constants that need to be
- // loaded to be materialized, and all of their Values are turned into loads from that table.
- // Non-fast constants get materialized in the block that uses them. Constants that are
- // materialized by loading get special treatment when they get used in some kind of Any in a
- // StackmapValue. In that case, the Constants are sunk to the point of use, since that allows
- // the instruction selector to sink the constants into an Arg::imm64.
-
- // FIXME: Implement a better story for constants. For example, the table used to hold double
- // constants should have a pointer to it that is hoisted. If we wanted to be more aggressive,
- // we could make constant materialization be a feature of Air: we could label some Tmps as
- // being unmaterialized constants and have a late Air phase - post register allocation - that
- // creates materializations of those constant Tmps by scavenging leftover registers.
-
- hoistFastConstants();
- sinkSlowConstants();
+ hoistConstants(
+ [&] (const ValueKey& key) -> bool {
+ return key.opcode() == ConstFloat || key.opcode() == ConstDouble;
+ });
+
+ lowerFPConstants();
+
+ hoistConstants(
+ [&] (const ValueKey& key) -> bool {
+ return key.opcode() == Const32 || key.opcode() == Const64;
+ });
}
private:
- void hoistFastConstants()
+ template<typename Filter>
+ void hoistConstants(const Filter& filter)
{
Dominators& dominators = m_proc.dominators();
HashMap<ValueKey, Value*> valueForConstant;
@@ -84,7 +79,7 @@
for (Value* value : *block) {
for (Value*& child : value->children()) {
ValueKey key = child->key();
- if (!m_proc.isFastConstant(key))
+ if (!filter(key))
continue;
auto result = valueForConstant.add(key, child);
@@ -116,7 +111,7 @@
if (block->frequency() < value->owner->frequency())
value->owner = block;
}
- materializations[entry.value->owner].append(entry.value);
+ materializations[value->owner].append(value);
}
// Get rid of Value's that are fast constants but aren't canonical. Also remove the canonical
@@ -124,7 +119,7 @@
for (BasicBlock* block : m_proc) {
for (Value*& value : *block) {
ValueKey key = value->key();
- if (!m_proc.isFastConstant(key))
+ if (!filter(key))
continue;
if (valueForConstant.get(key) == value)
@@ -141,7 +136,7 @@
Value* value = block->at(valueIndex);
for (Value* child : value->children()) {
ValueKey key = child->key();
- if (!m_proc.isFastConstant(key))
+ if (!filter(key))
continue;
// If we encounter a fast constant, then it must be canonical, since we already
@@ -174,16 +169,11 @@
m_insertionSet.execute(block);
}
}
-
- void sinkSlowConstants()
+
+ void lowerFPConstants()
{
- // First we need to figure out which constants go into the data section. These are non-zero
- // double constants.
for (Value* value : m_proc.values()) {
ValueKey key = value->key();
- if (!needsMotion(key))
- continue;
- m_toRemove.append(value);
if (goesInTable(key))
m_constTable.add(key, m_constTable.size());
}
@@ -191,67 +181,52 @@
m_dataSection = static_cast<int64_t*>(m_proc.addDataSection(m_constTable.size() * sizeof(int64_t)));
for (auto& entry : m_constTable)
m_dataSection[entry.value] = entry.key.value();
-
+
+ IndexSet<Value> offLimits;
for (BasicBlock* block : m_proc) {
- m_constants.clear();
-
for (unsigned valueIndex = 0; valueIndex < block->size(); ++valueIndex) {
- Value* value = block->at(valueIndex);
- StackmapValue* stackmap = value->as<StackmapValue>();
+ StackmapValue* value = block->at(valueIndex)->as<StackmapValue>();
+ if (!value)
+ continue;
+
for (unsigned childIndex = 0; childIndex < value->numChildren(); ++childIndex) {
+ if (!value->constrainedChild(childIndex).rep().isAny())
+ continue;
+
Value*& child = value->child(childIndex);
ValueKey key = child->key();
- if (!needsMotion(key))
+ if (!goesInTable(key))
continue;
- if (stackmap
- && goesInTable(key)
- && stackmap->constrainedChild(childIndex).rep().isAny()) {
- // This is a weird special case. When we constant-fold an argument to a
- // stackmap, and that argument has the Any constraint, we want to just
- // tell the stackmap's generator that the argument is a constant rather
- // than materializing it in a register. For this to work, we need
- // lowerToAir to see this argument as a constant rather than as a load
- // from a table.
- child = m_insertionSet.insertValue(
- valueIndex, key.materialize(m_proc, value->origin()));
- continue;
- }
-
- child = materialize(valueIndex, key, value->origin());
+ child = m_insertionSet.insertValue(
+ valueIndex, key.materialize(m_proc, value->origin()));
+ offLimits.add(child);
}
}
-
+
m_insertionSet.execute(block);
}
- for (Value* toRemove : m_toRemove)
- toRemove->replaceWithNop();
- }
-
- Value* materialize(unsigned valueIndex, const ValueKey& key, const Origin& origin)
- {
- if (Value* result = m_constants.get(key))
- return result;
+ for (BasicBlock* block : m_proc) {
+ for (unsigned valueIndex = 0; valueIndex < block->size(); ++valueIndex) {
+ Value* value = block->at(valueIndex);
+ ValueKey key = value->key();
+ if (!goesInTable(key))
+ continue;
+ if (offLimits.contains(value))
+ continue;
- // Note that we deliberately don't do this in one add() because this is a recursive function
- // that may rehash the map.
+ Value* tableBase = m_insertionSet.insertIntConstant(
+ valueIndex, value->origin(), pointerType(),
+ bitwise_cast<intptr_t>(m_dataSection));
+ Value* result = m_insertionSet.insert<MemoryValue>(
+ valueIndex, Load, value->type(), value->origin(), tableBase,
+ sizeof(int64_t) * m_constTable.get(key));
+ value->replaceWithIdentity(result);
+ }
- Value* result;
- if (goesInTable(key)) {
- Value* tableBase = materialize(
- valueIndex,
- ValueKey(
- constPtrOpcode(), pointerType(),
- static_cast<int64_t>(bitwise_cast<intptr_t>(m_dataSection))),
- origin);
- result = m_insertionSet.insert<MemoryValue>(
- valueIndex, Load, key.type(), origin, tableBase,
- sizeof(int64_t) * m_constTable.get(key));
- } else
- result = m_insertionSet.insertValue(valueIndex, key.materialize(m_proc, origin));
- m_constants.add(key, result);
- return result;
+ m_insertionSet.execute(block);
+ }
}
bool goesInTable(const ValueKey& key)
@@ -260,11 +235,6 @@
|| (key.opcode() == ConstFloat && key != floatZero());
}
- bool needsMotion(const ValueKey& key)
- {
- return key.isConstant() && !m_proc.isFastConstant(key);
- }
-
static ValueKey doubleZero()
{
return ValueKey(ConstDouble, Double, 0.0);
Modified: trunk/Source/_javascript_Core/b3/air/AirArg.h (195306 => 195307)
--- trunk/Source/_javascript_Core/b3/air/AirArg.h 2016-01-19 20:32:01 UTC (rev 195306)
+++ trunk/Source/_javascript_Core/b3/air/AirArg.h 2016-01-19 20:55:34 UTC (rev 195307)
@@ -1078,6 +1078,18 @@
bool usesTmp(Air::Tmp tmp) const;
+ template<typename Thing>
+ bool is() const;
+
+ template<typename Thing>
+ Thing as() const;
+
+ template<typename Thing, typename Functor>
+ void forEachFast(const Functor&);
+
+ template<typename Thing, typename Functor>
+ void forEach(Role, Type, Width, const Functor&);
+
// This is smart enough to know that an address arg in a Def or UseDef rule will use its
// tmps and never def them. For example, this:
//
Added: trunk/Source/_javascript_Core/b3/air/AirArgInlines.h (0 => 195307)
--- trunk/Source/_javascript_Core/b3/air/AirArgInlines.h (rev 0)
+++ trunk/Source/_javascript_Core/b3/air/AirArgInlines.h 2016-01-19 20:55:34 UTC (rev 195307)
@@ -0,0 +1,116 @@
+/*
+ * Copyright (C) 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
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef AirArgInlines_h
+#define AirArgInlines_h
+
+#if ENABLE(B3_JIT)
+
+#include "AirArg.h"
+
+namespace JSC { namespace B3 { namespace Air {
+
+template<typename T> struct ArgThingHelper;
+
+template<> struct ArgThingHelper<Tmp> {
+ static bool is(const Arg& arg)
+ {
+ return arg.isTmp();
+ }
+
+ static Tmp as(const Arg& arg)
+ {
+ if (is(arg))
+ return arg.tmp();
+ return Tmp();
+ }
+
+ template<typename Functor>
+ static void forEachFast(Arg& arg, const Functor& functor)
+ {
+ arg.forEachTmpFast(functor);
+ }
+
+ template<typename Functor>
+ static void forEach(Arg& arg, Arg::Role role, Arg::Type type, Arg::Width width, const Functor& functor)
+ {
+ arg.forEachTmp(role, type, width, functor);
+ }
+};
+
+template<> struct ArgThingHelper<Arg> {
+ static bool is(const Arg&)
+ {
+ return true;
+ }
+
+ static Arg as(const Arg& arg)
+ {
+ return arg;
+ }
+
+ template<typename Functor>
+ static void forEachFast(Arg& arg, const Functor& functor)
+ {
+ functor(arg);
+ }
+
+ template<typename Functor>
+ static void forEach(Arg& arg, Arg::Role role, Arg::Type type, Arg::Width width, const Functor& functor)
+ {
+ functor(arg, role, type, width);
+ }
+};
+
+template<typename Thing>
+bool Arg::is() const
+{
+ return ArgThingHelper<Thing>::is(*this);
+}
+
+template<typename Thing>
+Thing Arg::as() const
+{
+ return ArgThingHelper<Thing>::as(*this);
+}
+
+template<typename Thing, typename Functor>
+void Arg::forEachFast(const Functor& functor)
+{
+ ArgThingHelper<Thing>::forEachFast(*this, functor);
+}
+
+template<typename Thing, typename Functor>
+void Arg::forEach(Role role, Type type, Width width, const Functor& functor)
+{
+ ArgThingHelper<Thing>::forEach(*this, role, type, width, functor);
+}
+
+} } } // namespace JSC::B3::Air
+
+#endif // ENABLE(B3_JIT)
+
+#endif // AirArgInlines_h
+
Modified: trunk/Source/_javascript_Core/b3/air/AirIteratedRegisterCoalescing.cpp (195306 => 195307)
--- trunk/Source/_javascript_Core/b3/air/AirIteratedRegisterCoalescing.cpp 2016-01-19 20:32:01 UTC (rev 195306)
+++ trunk/Source/_javascript_Core/b3/air/AirIteratedRegisterCoalescing.cpp 2016-01-19 20:55:34 UTC (rev 195307)
@@ -948,6 +948,11 @@
const UseCounts<Tmp>::Counts& counts = m_useCounts[tmp];
double uses = counts.numWarmUses + counts.numDefs;
+ // If it's a constant, then it's not as bad to spill. We can rematerialize it in many
+ // cases.
+ if (counts.numConstDefs == counts.numDefs)
+ uses /= 2;
+
return degree / uses;
};
Modified: trunk/Source/_javascript_Core/b3/air/AirUseCounts.h (195306 => 195307)
--- trunk/Source/_javascript_Core/b3/air/AirUseCounts.h 2016-01-19 20:32:01 UTC (rev 195306)
+++ trunk/Source/_javascript_Core/b3/air/AirUseCounts.h 2016-01-19 20:55:34 UTC (rev 195307)
@@ -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
@@ -28,7 +28,7 @@
#if ENABLE(B3_JIT)
-#include "AirArg.h"
+#include "AirArgInlines.h"
#include "AirBlockWorklist.h"
#include "AirCode.h"
#include "AirInstInlines.h"
@@ -57,6 +57,7 @@
double numWarmUses { 0 };
double numColdUses { 0 };
double numDefs { 0 };
+ double numConstDefs { 0 };
};
UseCounts(Code& code)
@@ -87,6 +88,11 @@
if (Arg::isAnyDef(role))
counts.numDefs += frequency;
});
+
+ if ((inst.opcode == Move || inst.opcode == Move32)
+ && inst.args[0].isSomeImm()
+ && inst.args[1].is<Thing>())
+ m_counts.add(inst.args[1].as<Thing>(), Counts()).iterator->value.numConstDefs++;
}
}
}
Modified: trunk/Source/_javascript_Core/dfg/DFGGraph.cpp (195306 => 195307)
--- trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2016-01-19 20:32:01 UTC (rev 195306)
+++ trunk/Source/_javascript_Core/dfg/DFGGraph.cpp 2016-01-19 20:55:34 UTC (rev 195307)
@@ -271,11 +271,8 @@
if (node->hasMultiGetByOffsetData()) {
MultiGetByOffsetData& data = ""
out.print(comma, "id", data.identifierNumber, "{", identifiers()[data.identifierNumber], "}");
- for (unsigned i = 0; i < data.cases.size(); ++i) {
+ for (unsigned i = 0; i < data.cases.size(); ++i)
out.print(comma, inContext(data.cases[i], context));
- if (data.cases[i].method().kind() == GetByOffsetMethod::Load)
- out.print(" (inferred value = ", inContext(inferredValueForProperty(data.cases[i].set(), identifiers()[data.identifierNumber]), context), ")");
- }
}
if (node->hasMultiPutByOffsetData()) {
MultiPutByOffsetData& data = ""
