Incorporated most comments, I'll have to investigate a failed ASSERT before
writing more tests... :-P


https://chromiumcodereview.appspot.com/10836133/diff/4003/src/hydrogen.cc
File src/hydrogen.cc (right):

https://chromiumcodereview.appspot.com/10836133/diff/4003/src/hydrogen.cc#newcode6876
src/hydrogen.cc:6876: // Falling from the end of an inlined construct
call. In a test context
On 2012/08/08 13:05:33, Michael Starzinger wrote:
Hmm, how about "Falling off the end ..."?

Done.

https://chromiumcodereview.appspot.com/10836133/diff/4003/src/hydrogen.cc#newcode6888
src/hydrogen.cc:6888: // Falling from the end of an inlined setter call.
The returned value is
On 2012/08/08 13:05:33, Michael Starzinger wrote:
Likewise.

Done.

https://chromiumcodereview.appspot.com/10836133/diff/4003/src/hydrogen.cc#newcode6900
src/hydrogen.cc:6900: // Falling from the end of a normal inlined
function. This basically means
On 2012/08/08 13:05:33, Michael Starzinger wrote:
Likewise.

Done.

https://chromiumcodereview.appspot.com/10836133/diff/4003/src/hydrogen.cc#newcode9274
src/hydrogen.cc:9274: ReturnHandlingFlag return_handling) const {
On 2012/08/08 13:05:33, Michael Starzinger wrote:
Since this flag is not only used to distinguish the return handling,
but also
the entry-kind for inlining, I would suggest to rename it to
"InlininigKind" or
something like that.

Done.

https://chromiumcodereview.appspot.com/10836133/diff/4003/test/mjsunit/compiler/inline-accessors.js
File test/mjsunit/compiler/inline-accessors.js (right):

https://chromiumcodereview.appspot.com/10836133/diff/4003/test/mjsunit/compiler/inline-accessors.js#newcode33
test/mjsunit/compiler/inline-accessors.js:33: function Constr3() { }
On 2012/08/08 13:05:33, Michael Starzinger wrote:
Can we move the constructors down, right before they are actually
used?

Done.

https://chromiumcodereview.appspot.com/10836133/diff/4003/test/mjsunit/compiler/inline-accessors.js#newcode81
test/mjsunit/compiler/inline-accessors.js:81: return 1234;
On 2012/08/08 13:05:33, Michael Starzinger wrote:
You might want to consider returning a global "getterValue" here, and
combine it
with some addition that can be sued to cause deopts once you start
working on
the deoptimizer.

As discussed offline, this will be done via an explicit "expected"
argument plus a global variable for forcing deopts.

https://chromiumcodereview.appspot.com/10836133/diff/4003/test/mjsunit/compiler/inline-accessors.js#newcode86
test/mjsunit/compiler/inline-accessors.js:86:
TestGetterInAllContexts(Object.create(obj));
On 2012/08/08 13:05:33, Michael Starzinger wrote:
Also test a getter that falls off the function end without returning
anything.

Done.

https://chromiumcodereview.appspot.com/10836133/diff/4003/test/mjsunit/compiler/inline-accessors.js#newcode113
test/mjsunit/compiler/inline-accessors.js:113: return obj.setterProperty
= value + 1;
On 2012/08/08 13:05:33, Michael Starzinger wrote:
I wouldn't use an addition on the RHS, otherwise it will deopt here,
instead of
inside the setter.

Done.

https://chromiumcodereview.appspot.com/10836133/diff/4003/test/mjsunit/compiler/inline-accessors.js#newcode153
test/mjsunit/compiler/inline-accessors.js:153:
TestSetterInAllContexts(Object.create(obj));
On 2012/08/08 13:05:33, Michael Starzinger wrote:
We also need tests for arguments mismatch of both setters and getters.
For the
getter that is too many arguments. For the setter that is both, too
many and too
few arguments.

Still a TODO...

https://chromiumcodereview.appspot.com/10836133/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to