LG. I hope we have good coverage of the generated code. You should try sprinkling __ stop(...) at various places and make sure you hit them - if not we need more tests. Does it lint?
http://codereview.chromium.org/67163/diff/1/14 File src/assembler.cc (right): http://codereview.chromium.org/67163/diff/1/14#newcode613 Line 613: break; How about a default clause that triggers an UNREACHABLE? http://codereview.chromium.org/67163/diff/1/10 File src/assembler.h (right): http://codereview.chromium.org/67163/diff/1/10#newcode420 Line 420: static ExternalReference double_fp_operation(char operation); Why is the operation a char? Why not a Token::Value or something like that? ... and it seems a dodgy to have this on IA32 where we don't need it at all. http://codereview.chromium.org/67163/diff/1/11 File src/codegen-arm.cc (right): http://codereview.chromium.org/67163/diff/1/11#newcode4516 Line 4516: char operation, Token::Value operation? http://codereview.chromium.org/67163/diff/1/11#newcode4543 Line 4543: // Get type of r1 into r3 Terminate comment with . http://codereview.chromium.org/67163/diff/1/6 File src/codegen-ia32.h (right): http://codereview.chromium.org/67163/diff/1/6#newcode435 Line 435: const OverwriteMode overwrite_mode); I wonder why this is const? http://codereview.chromium.org/67163/diff/1/9 File src/simulator-arm.cc (right): http://codereview.chromium.org/67163/diff/1/9#newcode433 Line 433: int sixty_four_bits[2]; Shouldn't this be int32 or something like that? http://codereview.chromium.org/67163/diff/1/9#newcode447 Line 447: int sixty_four_bits[2]; int32? http://codereview.chromium.org/67163/diff/1/9#newcode458 Line 458: registers_[2] = 0x50Bad4U; We have a recognizable zap value you could use here: kZapValue. http://codereview.chromium.org/67163/diff/1/9#newcode937 Line 937: double z = (swi == simulator_fp_add) ? This looks nasty. Why not write it as a series of if-else-if? http://codereview.chromium.org/67163/diff/1/3 File test/mjsunit/number-limits.js (right): http://codereview.chromium.org/67163/diff/1/3#newcode36 Line 36: print("i = " + i + ", 1/eps = " + (1/eps) + ", mulAboveMax = " + mulAboveMax + ", Number.MAX_VALUE = " + Number.MAX_VALUE + ", Infinity = " + Infinity); Lines too long. http://codereview.chromium.org/67163 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
