Uploaded a new patch set that renames "heavy" to "non_primitive" and adds
comments.
I tried moving the counters to IsInlineable (see
http://codereview.chromium.org/8700001/) but don't really like it because
there
is no good place to call ComputeAstNodeCountAndInlineableFlags() from. This
function needs to be called after analyzing the scopes and before code
generation, because Jacob's ARM performance improvement CL is using the
"is_primitive" flag in the full-codegen.
http://codereview.chromium.org/8677008/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):
http://codereview.chromium.org/8677008/diff/1/src/hydrogen.cc#newcode4649
src/hydrogen.cc:4649: int ast_node_count =
target_shared->ast_node_count();
On 2011/11/24 13:15:56, Kevin Millikin wrote:
Move this inside the if where it's used.
Now I use to increment "inlined_count_".
http://codereview.chromium.org/8677008/diff/1/src/hydrogen.cc#newcode4652
src/hydrogen.cc:4652: if (ast_node_count > kMaxInlinedSize) {
On 2011/11/24 13:15:56, Kevin Millikin wrote:
All of these need some comment about what we're trying to achieve,
please.
Either here in the code, or at the constant declaration.
Otherwise, it's impossible to figure out how to tune it.
E.g.:
1. Never inline big functions (bigger than kMaxInlinedSize AST nodes
or
kMaxSourceSize source text characters).
2. After reaching kMaxInlinedNodesSoft of successfully inlined AST
nodes, only
inline small 'primitive' functions (smaller than
kMaxInlinedPrimitiveSize,
primitive according to a syntactic criterion).
3. After reaching kMaxInlinedNodesHard of successfully inlined AST
nodes, do not
inline anything.
Done.
http://codereview.chromium.org/8677008/diff/1/src/hydrogen.cc#newcode4657
src/hydrogen.cc:4657: if (target->shared()->SourceSize() >
kMaxSourceSize) {
On 2011/11/24 13:15:56, Kevin Millikin wrote:
I wonder if we can just get rid of this now?
Unfortunately, removing it regresses the delta blue benchmark.
http://codereview.chromium.org/8677008/diff/1/src/hydrogen.cc#newcode4662
src/hydrogen.cc:4662: if (inlined_count_ > kMaxInlinedNodesSoft) {
On 2011/11/24 13:15:56, Kevin Millikin wrote:
OK, but the logic is hard to follow because it doesn't match the way
the
conditions would be described (above). Try rewriting it into
something like:
// Do not inline after the hard limit is reached.
if (inlined_count_ > kMaxInlinedNodesHard) ... return false;
// Only inline small primitives after the soft limit is reached.
if (inlined_count_ > kMaxInlinedNodesSoft &&
!(target_shared_is_primitive() && ast_node_count <=
kMaxInlinedPrimitiveSize)) {
... return false;
}
Done.
http://codereview.chromium.org/8677008/diff/1/src/hydrogen.cc#newcode4712
src/hydrogen.cc:4712: int count_before = AstNode::Count();
On 2011/11/24 13:15:56, Kevin Millikin wrote:
Please remove this, because it's only used to compute nodes_added
which is now
unused. Please remove nodes_added. Please remove AstNode::Count(),
since these
are the only callers.
Done.
http://codereview.chromium.org/8677008/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
