From 7784ac141ad5150401d11882b8b93c88f67abe4e Mon Sep 17 00:00:00 2001 From: Hannes Verschore Date: Tue, 29 Jan 2013 16:26:11 +0100 Subject: [PATCH] Bug 835178: IonMonkey: Enable inlining of functions containing JSOP_SETARG, r=nbp --- js/src/ion/IonBuilder.cpp | 1 - js/src/ion/IonFrameIterator-inl.h | 31 ++++++++++++++++---------- js/src/ion/IonFrameIterator.h | 2 +- js/src/jit-test/tests/ion/bug835178.js | 30 +++++++++++++++++++++++++ js/src/jsanalyze.cpp | 2 +- 5 files changed, 51 insertions(+), 15 deletions(-) create mode 100644 js/src/jit-test/tests/ion/bug835178.js diff --git a/js/src/ion/IonBuilder.cpp b/js/src/ion/IonBuilder.cpp index e550a6a64499..966d046272ac 100644 --- a/js/src/ion/IonBuilder.cpp +++ b/js/src/ion/IonBuilder.cpp @@ -870,7 +870,6 @@ IonBuilder::inspectOpcode(JSOp op) return true; case JSOP_SETARG: - JS_ASSERT(inliningDepth == 0); // To handle this case, we should spill the arguments to the space where // actual arguments are stored. The tricky part is that if we add a MIR // to wrap the spilling action, we don't want the spilling to be diff --git a/js/src/ion/IonFrameIterator-inl.h b/js/src/ion/IonFrameIterator-inl.h index 8ba8a5ba4b82..18c0d984bb41 100644 --- a/js/src/ion/IonFrameIterator-inl.h +++ b/js/src/ion/IonFrameIterator-inl.h @@ -15,7 +15,7 @@ namespace ion { template inline void -SnapshotIterator::readFrameArgs(Op op, const Value *argv, Value *scopeChain, Value *thisv, +SnapshotIterator::readFrameArgs(Op &op, const Value *argv, Value *scopeChain, Value *thisv, unsigned start, unsigned formalEnd, unsigned iterEnd) { if (scopeChain) @@ -62,28 +62,35 @@ InlineFrameIterator::forEachCanonicalActualArg(JSContext *cx, Op op, unsigned st if (more()) { // There is still a parent frame of this inlined frame. - // Take arguments of the caller (parent inlined frame) it holds all actual - // arguments, needed in case of overflow, and because the analyze phase - // disable Ion inlining if the function redefine its arguments with JSOP_SETARG. + // The not overflown arguments are taken from the inlined frame, + // because it will have the updated value when JSOP_SETARG is done. + // All arguments (also the overflown) are the last pushed values in the parent frame. + // To get the overflown arguments, we need to take them from there. + // Get the non overflown arguments + unsigned formal_end = (end < nformal) ? end : nformal; + SnapshotIterator s(si_); + s.readFrameArgs(op, NULL, NULL, NULL, start, nformal, formal_end); + + // The overflown arguments are not available in current frame. + // They are the last pushed arguments in the parent frame of this inlined frame. InlineFrameIterator it(cx, this); - ++it; - SnapshotIterator s(it.snapshotIterator()); + SnapshotIterator parent_s((++it).snapshotIterator()); - // Skip over all slots untill we get to the arguments slots + // Skip over all slots untill we get to the last slots (= arguments slots of callee) // the +2 is for [this] and [scopechain] - JS_ASSERT(s.slots() >= nactual + 2); - unsigned skip = s.slots() - nactual - 2; + JS_ASSERT(parent_s.slots() >= nactual + 2); + unsigned skip = parent_s.slots() - nactual - 2; for (unsigned j = 0; j < skip; j++) - s.skip(); + parent_s.skip(); - s.readFrameArgs(op, NULL, NULL, NULL, start, nactual, end); + // Get the overflown arguments + parent_s.readFrameArgs(op, NULL, NULL, NULL, nformal, nactual, end); } else { SnapshotIterator s(si_); Value *argv = frame_->actualArgs(); s.readFrameArgs(op, argv, NULL, NULL, start, nformal, end); } - } } // namespace ion diff --git a/js/src/ion/IonFrameIterator.h b/js/src/ion/IonFrameIterator.h index 5b458b45761a..6665fe0d7bd1 100644 --- a/js/src/ion/IonFrameIterator.h +++ b/js/src/ion/IonFrameIterator.h @@ -238,7 +238,7 @@ class SnapshotIterator : public SnapshotReader } template - inline void readFrameArgs(Op op, const Value *argv, Value *scopeChain, Value *thisv, + inline void readFrameArgs(Op &op, const Value *argv, Value *scopeChain, Value *thisv, unsigned start, unsigned formalEnd, unsigned iterEnd); Value maybeReadSlotByIndex(size_t index) { diff --git a/js/src/jit-test/tests/ion/bug835178.js b/js/src/jit-test/tests/ion/bug835178.js new file mode 100644 index 000000000000..ec2395192400 --- /dev/null +++ b/js/src/jit-test/tests/ion/bug835178.js @@ -0,0 +1,30 @@ +function boo() { return foo.arguments[0] } +function foo(a,b,c) { if (a == 0) {a = 2; return boo();} return a } +function inlined() { return foo.apply({}, arguments); } +function test(a,b,c) { return inlined(a,b,c) } + +assertEq(test(1,2,3), 1); +assertEq(test(0,2,3), 2); + +function g(a) { if (g.arguments[1]) return true; return false; }; +function f() { return g(false, true); }; +function h() { return f(false, false); } + +assertEq(h(false, false), true); +assertEq(h(false, false), true); + +function g2(a) { if (a) { if (g2.arguments[1]) return true; return false; } return true; }; +function f2(a) { return g2(a, true); }; +function h2(a) { return f2(a, false); } + +assertEq(h2(false, false), true); +assertEq(h2(true, false), true); + +// Currently disabled for now, but in testsuite to be sure +function g3(a) { return a }; +function f3(a) { a = 3; return g3.apply({}, arguments); }; +function h3(a) { return f3(a); } + +assertEq(h3(0), 3); +assertEq(h3(0), 3); + diff --git a/js/src/jsanalyze.cpp b/js/src/jsanalyze.cpp index 01b4868905ba..a842551fcba2 100644 --- a/js/src/jsanalyze.cpp +++ b/js/src/jsanalyze.cpp @@ -447,7 +447,7 @@ ScriptAnalysis::analyzeBytecode(JSContext *cx) case JSOP_SETARG: modifiesArguments_ = true; - isIonInlineable = isJaegerInlineable = false; + isJaegerInlineable = false; break; case JSOP_GETPROP: