Mainly I have suggestions for further additions, and the existing code
LGTM.


http://codereview.chromium.org/15037/diff/206/12
File src/codegen-ia32.cc (right):

http://codereview.chromium.org/15037/diff/206/12#newcode2672
Line 2672: frame_->Push(&temp);
I thought this was in the assembler's Set() function.  Once we have
constant folding, the attacker can put a 32-bit SMI in the code by x =
bad_word - 1 + 1.  Can this be put in Set() instead?

http://codereview.chromium.org/15037/diff/206/8
File src/register-allocator-ia32.cc (right):

http://codereview.chromium.org/15037/diff/206/8#newcode43
Line 43: cgen_->allocator()->Use(reg);
Do we want Result(no_reg) to be INVALID?
This way, with the assert, is good, because it
can always be changed if we need it.

http://codereview.chromium.org/15037/diff/206/8#newcode93
Line 93: Use(edi);
Pretty sad, that we use all of these. :(

http://codereview.chromium.org/15037/diff/206/11
File src/register-allocator-ia32.h (right):

http://codereview.chromium.org/15037/diff/206/11#newcode94
Line 94:
Do we also want ToRegister(Register target)?

http://codereview.chromium.org/15037/diff/206/9
File src/virtual-frame-ia32.cc (right):

http://codereview.chromium.org/15037/diff/206/9#newcode349
Line 349: frame_registers_.count(element.reg().code()) > 1)) {
Shouldn't we overload count() and is_used() to take
a Register, as well as a code?  We have so many occurrences
of is_used(x.reg().code()).

http://codereview.chromium.org/15037

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

Reply via email to