LGTM with comments.
http://codereview.chromium.org/7230045/diff/8002/src/arm/deoptimizer-arm.cc File src/arm/deoptimizer-arm.cc (right): http://codereview.chromium.org/7230045/diff/8002/src/arm/deoptimizer-arm.cc#newcode549 src/arm/deoptimizer-arm.cc:549: input_->SetDoubleRegister(i, -1.0 * i); Why not just set it to 0.0 or some other constant default instead? http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc File src/deoptimizer.cc (right): http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode169 src/deoptimizer.cc:169: Maybe put the GC-unsafe part inside an AssertNoAllocation-scope? http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode173 src/deoptimizer.cc:173: Don't you have to de-allocate the deoptimizer? Or can it be stack-allocated instead of new-allocated? http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode551 src/deoptimizer.cc:551: // Check of the heap number to materialize actually belongd to the frame Check that the heap number to materialize actually belongs... http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode564 src/deoptimizer.cc:564: PrintF("Materializing a new heap number %p [%e] in index %d\n", Remove duplicate PrintF, maybe add the expression index to the PrintF above? http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.h File src/deoptimizer.h (right): http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.h#newcode195 src/deoptimizer.h:195: void MaterializeHeapNumbersForDebuggerInspectableFrame( Maybe add #ifdef ENABLE_DEBUGGER_SUPPORT here too. http://codereview.chromium.org/7230045/diff/8002/src/ia32/deoptimizer-ia32.cc File src/ia32/deoptimizer-ia32.cc (right): http://codereview.chromium.org/7230045/diff/8002/src/ia32/deoptimizer-ia32.cc#newcode615 src/ia32/deoptimizer-ia32.cc:615: input_->SetRegister(i, i * 4); Maybe use some constant as default value? http://codereview.chromium.org/7230045/diff/8002/test/mjsunit/debug-evaluate-locals-optimized-double.js File test/mjsunit/debug-evaluate-locals-optimized-double.js (right): http://codereview.chromium.org/7230045/diff/8002/test/mjsunit/debug-evaluate-locals-optimized-double.js#newcode48 test/mjsunit/debug-evaluate-locals-optimized-double.js:48: assertEquals(i * 2 + 1 + (i * 2 + 1) / 100, frame.localValue(0).value()); Long line. http://codereview.chromium.org/7230045/diff/8002/test/mjsunit/debug-evaluate-locals-optimized-double.js#newcode49 test/mjsunit/debug-evaluate-locals-optimized-double.js:49: assertEquals(i * 2 + 2 + (i * 2 + 2) / 100, frame.localValue(1).value()); Long line. http://codereview.chromium.org/7230045/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
