Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1388)

Unified Diff: runtime/vm/flow_graph_inliner.cc

Issue 11269040: More inlining flags and tuned heuristics. (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
Patch Set: Style and phrasing. Created 8 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: runtime/vm/flow_graph_inliner.cc
diff --git a/runtime/vm/flow_graph_inliner.cc b/runtime/vm/flow_graph_inliner.cc
index 6aee0ad7b70bbec3b5f99911a8243d0b2bb4afac..6ecb77b43bc0aa593a0225ba6d6209741e3a0340 100644
--- a/runtime/vm/flow_graph_inliner.cc
+++ b/runtime/vm/flow_graph_inliner.cc
@@ -20,11 +20,23 @@ namespace dart {
DEFINE_FLAG(bool, trace_inlining, false, "Trace inlining");
DEFINE_FLAG(charp, inlining_filter, NULL, "Inline only in named function");
-DEFINE_FLAG(int, inlining_size_threshold, 50,
- "Inline only functions with up to threshold instructions (default 50)");
-// TODO(srdjan): set to 3 once crash in apidoc.dart is resolved.
+
+// Flags for inlining heuristics.
DEFINE_FLAG(int, inlining_depth_threshold, 3,
- "Inline recursively up to threshold depth (default 3)");
+ "Inline function calls up to threshold nesting depth");
+DEFINE_FLAG(int, inlining_size_threshold, 20,
+ "Always inline functions that have threshold or fewer instructions");
+DEFINE_FLAG(int, inlining_in_loop_size_threshold, 60,
+ "Inline functions in loops that have threshold or fewer instructions");
+DEFINE_FLAG(int, inlining_callee_call_sites_threshold, 1,
+ "Always inline functions containing threshold or fewer calls.");
+DEFINE_FLAG(int, inlining_constant_arguments_count, 1,
+ "Inline function calls with sufficient constant arguments "
+ "and up to the increased threshold on instructions");
+DEFINE_FLAG(int, inlining_constant_arguments_size_threshold, 40,
+ "Inline function calls with sufficient constant arguments "
+ "and up to the increased threshold on instructions");
srdjan 2012/10/25 20:38:50 Indent 4 characters (continuation of a 'statement
zerny-google 2012/10/29 16:44:25 Done.
+
DECLARE_FLAG(bool, print_flow_graph);
DECLARE_FLAG(int, deoptimization_counter_threshold);
DECLARE_FLAG(bool, verify_compiler);
@@ -128,6 +140,41 @@ struct NamedArgument : ValueObject {
};
+// Helper to collect information about a callee graph.
srdjan 2012/10/25 20:38:50 about inlineable callee graphs
zerny-google 2012/10/29 16:44:25 Done.
+class GraphInfoCollector : public ValueObject {
+ public:
+ explicit GraphInfoCollector()
srdjan 2012/10/25 20:38:50 remove explicit
zerny-google 2012/10/29 16:44:25 Done.
+ : call_site_count_(0),
+ instruction_count_(0) { }
+
+ void Collect(const FlowGraph& graph) {
+ call_site_count_ = 0;
+ instruction_count_ = 0;
+ for (BlockIterator block_it = graph.postorder_iterator();
+ !block_it.Done();
+ block_it.Advance()) {
+ for (ForwardInstructionIterator it(block_it.Current());
+ !it.Done();
+ it.Advance()) {
+ ++instruction_count_;
+ if (it.Current()->IsStaticCall() ||
+ it.Current()->IsClosureCall() ||
+ it.Current()->IsPolymorphicInstanceCall()) {
+ ++call_site_count_;
+ }
+ }
+ }
+ }
+
+ intptr_t call_site_count() const { return call_site_count_; }
+ intptr_t instruction_count() const { return instruction_count_; }
+
+ private:
+ intptr_t call_site_count_;
+ intptr_t instruction_count_;
+};
+
+
// A collection of call sites to consider for inlining.
class CallSites : public FlowGraphVisitor {
public:
@@ -207,6 +254,27 @@ class CallSiteInliner : public ValueObject {
inlining_call_sites_(NULL),
function_cache_() { }
+ // Inlining heuristics based on Cooper et al. 2008.
+ bool ShouldWeInline(intptr_t loop_depth,
+ intptr_t size,
srdjan 2012/10/25 20:38:50 Size of what? Please name better
zerny-google 2012/10/29 16:44:25 s/size/instr_count
+ intptr_t call_sites,
srdjan 2012/10/25 20:38:50 num_call_sites?
zerny-google 2012/10/29 16:44:25 s/call_sites/call_site_count
+ intptr_t constant_args) {
+ if (size <= FLAG_inlining_size_threshold) {
+ return true;
+ }
+ if (call_sites <= FLAG_inlining_callee_call_sites_threshold) {
+ return true;
+ }
+ if ((loop_depth > 0) && (size <= FLAG_inlining_in_loop_size_threshold)) {
+ return true;
+ }
+ if ((constant_args >= FLAG_inlining_constant_arguments_count) &&
+ (size <= FLAG_inlining_constant_arguments_size_threshold)) {
+ return true;
+ }
+ return false;
+ }
+
void InlineCalls() {
// If inlining depth is less then one abort.
if (FLAG_inlining_depth_threshold < 1) return;
@@ -320,6 +388,7 @@ class CallSiteInliner : public ValueObject {
}
// Build the callee graph.
+ const intptr_t loop_depth = call->GetBlock()->loop_depth();
FlowGraphBuilder builder(*parsed_function);
builder.SetInitialBlockId(caller_graph_->max_block_id());
FlowGraph* callee_graph;
@@ -327,7 +396,8 @@ class CallSiteInliner : public ValueObject {
TimerScope timer(FLAG_compiler_stats,
&CompilerStats::graphinliner_build_timer,
isolate);
- callee_graph = builder.BuildGraph(FlowGraphBuilder::kValueContext);
+ callee_graph =
+ builder.BuildGraph(FlowGraphBuilder::kValueContext, loop_depth);
}
// The parameter stubs are a copy of the actual arguments providing
@@ -392,15 +462,39 @@ class CallSiteInliner : public ValueObject {
printer.PrintBlocks();
}
- // If result is more than size threshold then abort.
+ // Collect information about the call site and caller graph.
// TODO(zerny): Do this after CP and dead code elimination.
- intptr_t size = callee_graph->InstructionCount();
- if (size > FLAG_inlining_size_threshold) {
- function.set_is_inlinable(false);
+ intptr_t constants_count = 0;
+ for (intptr_t i = 0; i < param_stubs.length(); ++i) {
+ if (param_stubs[i]->IsConstant()) ++constants_count;
+ }
+ GraphInfoCollector info;
+ info.Collect(*callee_graph);
+ const intptr_t size = info.instruction_count();
+ // Use heuristics do decide if this call should be inlined.
+ if (!ShouldWeInline(loop_depth,
+ size,
+ info.call_site_count(),
+ constants_count)) {
+ // If size is larger than all thresholds, don't consider it again.
+ if ((size > FLAG_inlining_size_threshold) &&
+ (size > FLAG_inlining_in_loop_size_threshold) &&
+ (size > FLAG_inlining_callee_call_sites_threshold) &&
+ (size > FLAG_inlining_constant_arguments_size_threshold)) {
+ function.set_is_inlinable(false);
+ }
isolate->set_long_jump_base(base);
isolate->set_deopt_id(prev_deopt_id);
isolate->set_ic_data_array(prev_ic_data.raw());
- TRACE_INLINING(OS::Print(" Bailout: graph size %"Pd"\n", size));
+ TRACE_INLINING(OS::Print(" Bailout: heuristics with "
+ "loop depth: %"Pd", "
+ "code size: %"Pd", "
+ "call sites: %"Pd", "
+ "const args: %"Pd"\n",
+ loop_depth,
+ size,
+ info.call_site_count(),
+ constants_count));
return false;
}

Powered by Google App Engine
This is Rietveld 408576698