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); On 2011/06/29 10:47:35, fschneider wrote:
Why not just set it to 0.0 or some other constant default instead?
It was for debugging to see if these values turned up somewhere. Changed to 0.0. 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: On 2011/06/29 10:47:35, fschneider wrote:
Maybe put the GC-unsafe part inside an AssertNoAllocation-scope?
The already happens due to the Deoptimizer constructor turns off allocation and Deoptimizer::DeleteFrameDescriptions() turns it back on (deoptimizer.cc:358). http://codereview.chromium.org/7230045/diff/8002/src/deoptimizer.cc#newcode173 src/deoptimizer.cc:173: On 2011/06/29 10:47:35, fschneider wrote:
Don't you have to de-allocate the deoptimizer? Or can it be
stack-allocated
instead of new-allocated?
Good catch! Of cause. 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 On 2011/06/29 10:47:35, fschneider wrote:
Check that the heap number to materialize actually belongs...
Done. 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", On 2011/06/29 10:47:35, fschneider wrote:
Remove duplicate PrintF, maybe add the expression index to the PrintF
above? Done. 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( On 2011/06/29 10:47:35, fschneider wrote:
Maybe add
#ifdef ENABLE_DEBUGGER_SUPPORT
here too.
Done. 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); On 2011/06/29 10:47:35, fschneider wrote:
Maybe use some constant as default value?
Done. 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()); On 2011/06/29 10:47:35, fschneider wrote:
Long line.
Done. 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()); On 2011/06/29 10:47:35, fschneider wrote:
Long line.
Done. http://codereview.chromium.org/7230045/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
