LGTM. Den 26. maj 2011 11.21 skrev <[email protected]>:
> Reviewers: fschneider, > > Description: > Do not allow inlining functions with direct arguments access. > > Our implementations of arguments without materializing the arguments > object (based on inspecting the stack frame) does not work for inlined > functions. Guard all attempts by disallowing them if possible or else > bailing out of the optimizing compiler. > > [email protected] > BUG= > TEST= > > > Please review this at http://codereview.chromium.org/6976022/ > > SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge > > Affected files: > M src/ast.cc > M src/hydrogen.cc > > > Index: src/ast.cc > diff --git a/src/ast.cc b/src/ast.cc > index > 50045568c931fcadfe3bdf9771b0ce5422674d31..b4abf5416e9fd6692783a4f4875e35a5a0ab7e72 > 100644 > --- a/src/ast.cc > +++ b/src/ast.cc > @@ -544,6 +544,17 @@ bool CallNew::IsInlineable() const { > > > bool CallRuntime::IsInlineable() const { > + // Don't try to inline JS runtime calls because we don't (currently) > even > + // optimize them. > + if (is_jsruntime()) return false; > + // Don't inline the %_ArgumentsLength or %_Arguments because their > + // implementation will not work. There is no stack frame to get them > + // from. > + if (function()->intrinsic_type == Runtime::INLINE && > + (name()->IsEqualTo(CStrVector("_ArgumentsLength")) || > + name()->IsEqualTo(CStrVector("_Arguments")))) { > + return false; > + } > const int count = arguments()->length(); > for (int i = 0; i < count; ++i) { > if (!arguments()->at(i)->IsInlineable()) return false; > Index: src/hydrogen.cc > diff --git a/src/hydrogen.cc b/src/hydrogen.cc > index > 6e520bd85acf933872d52855e3cfe7ec4ef45f45..bd370e4eede7460cc5032b28456dacae1e2efcd1 > 100644 > --- a/src/hydrogen.cc > +++ b/src/hydrogen.cc > @@ -3848,6 +3848,13 @@ bool HGraphBuilder::TryArgumentsAccess(Property* > expr) { > return false; > } > > + // Our implementation of arguments (based on this stack frame or an > + // adapter below it) does not work for inlined functions. > + if (function_state()->outer() != NULL) { > + Bailout("arguments access in inlined function"); > + return true; > + } > + > HInstruction* result = NULL; > if (expr->key()->IsPropertyName()) { > Handle<String> name = expr->key()->AsLiteral()->AsPropertyName(); > @@ -4396,6 +4403,13 @@ bool HGraphBuilder::TryCallApply(Call* expr) { > if (!expr->IsMonomorphic() || > expr->check_type() != RECEIVER_MAP_CHECK) return false; > > + // Our implementation of arguments (based on this stack frame or an > + // adapter below it) does not work for inlined functions. > + if (function_state()->outer() != NULL) { > + Bailout("Function.prototype.apply optimization in inlined function"); > + return true; > + } > + > // Found pattern f.apply(receiver, arguments). > VisitForValue(prop->obj()); > if (HasStackOverflow() || current_block() == NULL) return true; > @@ -5422,6 +5436,10 @@ void > HGraphBuilder::GenerateIsConstructCall(CallRuntime* call) { > > // Support for arguments.length and arguments[?]. > void HGraphBuilder::GenerateArgumentsLength(CallRuntime* call) { > + // Our implementation of arguments (based on this stack frame or an > + // adapter below it) does not work for inlined functions. This runtime > + // function is blacklisted by AstNode::IsInlineable. > + ASSERT(function_state()->outer() == NULL); > ASSERT(call->arguments()->length() == 0); > HInstruction* elements = AddInstruction(new(zone()) HArgumentsElements); > HArgumentsLength* result = new(zone()) HArgumentsLength(elements); > @@ -5430,6 +5448,10 @@ void > HGraphBuilder::GenerateArgumentsLength(CallRuntime* call) { > > > void HGraphBuilder::GenerateArguments(CallRuntime* call) { > + // Our implementation of arguments (based on this stack frame or an > + // adapter below it) does not work for inlined functions. This runtime > + // function is blacklisted by AstNode::IsInlineable. > + ASSERT(function_state()->outer() == NULL); > ASSERT(call->arguments()->length() == 1); > CHECK_ALIVE(VisitForValue(call->arguments()->at(0))); > HValue* index = Pop(); > > > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
