I finally got around to rebasing this CL onto current trunk and addressing
Kevin's comments. It turned out that executing HLoadElements on any array no
longer seems to be an issue (at least I've been unable to reproduce any
problem); so I could change that part of the CL to allow hoisting of
HLoadElements (rather than preventing it by different means than before).
Florian: as I changed quite a bit, and Kevin is on vacation, could you take
a
quick look to see if this is all sane?
http://codereview.chromium.org/7298003/diff/3001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):
http://codereview.chromium.org/7298003/diff/3001/src/hydrogen-instructions.h#newcode2401
src/hydrogen-instructions.h:2401: explicit HJSArrayLength(HValue* value,
HValue* typecheck)
On 2011/07/11 15:03:54, Kevin Millikin wrote:
No need for explicit.
Done.
http://codereview.chromium.org/7298003/diff/3001/src/hydrogen-instructions.h#newcode3438
src/hydrogen-instructions.h:3438: explicit HLoadElements(HValue* value,
HValue* typecheck)
On 2011/07/11 15:03:54, Kevin Millikin wrote:
Same, no need for explicit.
Obsolete (I undid the changes to HLoadElements).
http://codereview.chromium.org/7298003/diff/3001/src/hydrogen.cc
File src/hydrogen.cc (right):
http://codereview.chromium.org/7298003/diff/3001/src/hydrogen.cc#newcode3267
src/hydrogen.cc:3267: elements = new(zone()) HLoadElements(literal,
literal);
On 2011/07/11 15:03:54, Kevin Millikin wrote:
The second occurrence of literal is used as a token to indicate that
you've
discharged your obligation to check this map ("trust me, I know what
I'm
doing"). I wonder if the undefined value or something is better to
use here?
Obsolete (I undid the changes to HLoadElements).
http://codereview.chromium.org/7298003/diff/3001/src/hydrogen.cc#newcode3983
src/hydrogen.cc:3983: new(zone()) HLoadElements(object,
elements_kind_branch));
On 2011/07/11 15:03:54, Kevin Millikin wrote:
Should we assert the check instruction is not NULL here?
Good catch! Depending on the JS code being executed, the check
instruction actually could be NULL at this point. While investigating, I
found that hoisting prevention for HLoadElements no longer seems to be
necessary, so I removed it.
http://codereview.chromium.org/7298003/
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
