please take another look.

http://codereview.chromium.org/9141016/diff/16001/src/hydrogen.cc
File src/hydrogen.cc (right):

http://codereview.chromium.org/9141016/diff/16001/src/hydrogen.cc#newcode1478
src/hydrogen.cc:1478: // the current block contain all of the loop's
dependencies.
On 2012/02/01 10:54:14, fschneider wrote:
I don't think this works in general: e.g. in a simple while-loop:

P->H
H->A
H->X
A->B
A->C
B->D
C->D
D-->H

If I understand the code correctly, the outstanding count would be > 0
for all
blocks A, B, C, D.

Done.

http://codereview.chromium.org/9141016/diff/16001/test/mjsunit/elements-transition-hoisting.js
File test/mjsunit/elements-transition-hoisting.js (right):

http://codereview.chromium.org/9141016/diff/16001/test/mjsunit/elements-transition-hoisting.js#newcode50
test/mjsunit/elements-transition-hoisting.js:50: // point, but when the
code gets optimized, the double transition should
On 2012/02/01 10:54:14, fschneider wrote:
s/double//

Done.

http://codereview.chromium.org/9141016/diff/16001/test/mjsunit/elements-transition-hoisting.js#newcode56
test/mjsunit/elements-transition-hoisting.js:56: } while (false);
On 2012/02/01 10:54:14, fschneider wrote:
do { } while(false) may be optimized away by the compiler. Not sure if
we
currently do it, but in the future it will definitely be.

Done.

http://codereview.chromium.org/9141016/diff/16001/test/mjsunit/elements-transition-hoisting.js#newcode57
test/mjsunit/elements-transition-hoisting.js:57: // Check outside of
loop to prevent call in loop
On 2012/02/01 10:54:14, fschneider wrote:
This comment does not make sense: there is already a call to
assertTrue,
%HasFastElements, etc. inside the loop.

Done.

http://codereview.chromium.org/9141016/diff/16001/test/mjsunit/elements-transition-hoisting.js#newcode76
test/mjsunit/elements-transition-hoisting.js:76: // point, but when the
code gets optimized, the double transition should
On 2012/02/01 10:54:14, fschneider wrote:
s/double//

Done.

http://codereview.chromium.org/9141016/diff/16001/test/mjsunit/elements-transition-hoisting.js#newcode109
test/mjsunit/elements-transition-hoisting.js:109: a.foo = object; //
This map check should be hoistable
On 2012/02/01 10:54:14, fschneider wrote:
There are calls inside the loop, so this map check won't be hoisted.

Done.

http://codereview.chromium.org/9141016/

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

Reply via email to