First round of comments, just to keep you busy :-)
https://codereview.chromium.org/10701054/diff/69002/src/arm/code-stubs-arm.cc File src/arm/code-stubs-arm.cc (right): https://codereview.chromium.org/10701054/diff/69002/src/arm/code-stubs-arm.cc#newcode3792 src/arm/code-stubs-arm.cc:3792: save_doubles.GetCode()->set_is_pregenerated(true); Suggestion: save the result of save_doubles.GetCode in a variable, and do the ->set_is_pregenerated(true) call after the if/else-block. https://codereview.chromium.org/10701054/diff/69002/src/arm/deoptimizer-arm.cc File src/arm/deoptimizer-arm.cc (right): https://codereview.chromium.org/10701054/diff/69002/src/arm/deoptimizer-arm.cc#newcode464 src/arm/deoptimizer-arm.cc:464: void Deoptimizer::DoCompiledStubPseudoFrame(TranslationIterator* iterator, s/Pseudo// https://codereview.chromium.org/10701054/diff/69002/src/arm/deoptimizer-arm.cc#newcode467 src/arm/deoptimizer-arm.cc:467: // Code::Kind stub_kind = static_cast<Code::Kind>(iterator->Next()); remove https://codereview.chromium.org/10701054/diff/69002/src/arm/deoptimizer-arm.cc#newcode468 src/arm/deoptimizer-arm.cc:468: FrameDescription* output_frame = new(0) FrameDescription(0, 0); just "NULL"? https://codereview.chromium.org/10701054/diff/69002/src/arm/deoptimizer-arm.cc#newcode474 src/arm/deoptimizer-arm.cc:474: Handle<Code> miss_ic = isolate_->builtins()->KeyedLoadIC_Miss(); use stub_kind (see line 478) to figure this out https://codereview.chromium.org/10701054/diff/69002/src/arm/lithium-arm.cc File src/arm/lithium-arm.cc (right): https://codereview.chromium.org/10701054/diff/69002/src/arm/lithium-arm.cc#newcode2146 src/arm/lithium-arm.cc:2146: case KEYED_STORE_IC_PARAMETER: remove for now https://codereview.chromium.org/10701054/diff/69002/src/arm/lithium-codegen-arm.cc File src/arm/lithium-codegen-arm.cc (right): https://codereview.chromium.org/10701054/diff/69002/src/arm/lithium-codegen-arm.cc#newcode592 src/arm/lithium-codegen-arm.cc:592: translation->BeginCompiledStubPseudoFrame(Code::KEYED_LOAD_IC); can we get the code type dynamically? https://codereview.chromium.org/10701054/diff/69002/src/arm/lithium-codegen-arm.cc#newcode811 src/arm/lithium-codegen-arm.cc:811: __ Jump(entry, RelocInfo::RUNTIME_ENTRY); nit: indentation https://codereview.chromium.org/10701054/diff/69002/src/arm/lithium-codegen-arm.cc#newcode4597 src/arm/lithium-codegen-arm.cc:4597: FloatingPointHelper::ConvertIntToDouble(masm(), nit: fix format https://codereview.chromium.org/10701054/diff/69002/src/arm/macro-assembler-arm.cc File src/arm/macro-assembler-arm.cc (right): https://codereview.chromium.org/10701054/diff/69002/src/arm/macro-assembler-arm.cc#newcode2700 src/arm/macro-assembler-arm.cc:2700: CEntryStub stub(1, CpuFeatures::IsSupported(VFP2) nit: formatting https://codereview.chromium.org/10701054/diff/69002/src/code-stubs.cc File src/code-stubs.cc (right): https://codereview.chromium.org/10701054/diff/69002/src/code-stubs.cc#newcode71 src/code-stubs.cc:71: stub->Print(); Don't forget to remove this (or surround with "if (FLAG_print_code)"). https://codereview.chromium.org/10701054/diff/69002/src/disassembler.cc File src/disassembler.cc (right): https://codereview.chromium.org/10701054/diff/69002/src/disassembler.cc#newcode1 src/disassembler.cc:1: // Copyright 2012 the V8 project authors. All rights reserved. we don't do this kind of change any more. https://codereview.chromium.org/10701054/diff/69002/src/flag-definitions.h File src/flag-definitions.h (right): https://codereview.chromium.org/10701054/diff/69002/src/flag-definitions.h#newcode286 src/flag-definitions.h:286: DEFINE_bool(enable_vfp3, false, don't forget to undo this change https://codereview.chromium.org/10701054/diff/69002/src/hydrogen-instructions.cc File src/hydrogen-instructions.cc (right): https://codereview.chromium.org/10701054/diff/69002/src/hydrogen-instructions.cc#newcode382 src/hydrogen-instructions.cc:382: HValue* operand = OperandAt(i); as discussed, just pass in any existing HValue as fake dependency, so you don't need this code. If you do keep it, replace OperandAt(i) below with |operand| ;-) https://codereview.chromium.org/10701054/diff/69002/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): https://codereview.chromium.org/10701054/diff/69002/src/hydrogen-instructions.h#newcode3775 src/hydrogen-instructions.h:3775: KEYED_STORE_IC_PARAMETER Remove this https://codereview.chromium.org/10701054/diff/69002/src/hydrogen.cc File src/hydrogen.cc (right): https://codereview.chromium.org/10701054/diff/69002/src/hydrogen.cc#newcode32 src/hydrogen.cc:32: #include "elements-hydrogen.h" remove https://codereview.chromium.org/10701054/diff/69002/src/hydrogen.cc#newcode280 src/hydrogen.cc:280: HEnvironment* last = pred->last_environment(); remove this change? https://codereview.chromium.org/10701054/diff/69002/src/hydrogen.cc#newcode6096 src/hydrogen.cc:6096: checked_key, nit: indentation https://codereview.chromium.org/10701054/diff/69002/src/hydrogen.cc#newcode9829 src/hydrogen.cc:9829: PrintStringProperty("name", "stub"); do we have more information here maybe? stub type? https://codereview.chromium.org/10701054/diff/69002/src/hydrogen.h File src/hydrogen.h (right): https://codereview.chromium.org/10701054/diff/69002/src/hydrogen.h#newcode1125 src/hydrogen.h:1125: HLoadNamedField* BuildLoadNamedField(HValue* object, merge problem? https://codereview.chromium.org/10701054/diff/69002/src/ia32/assembler-ia32.h File src/ia32/assembler-ia32.h (right): https://codereview.chromium.org/10701054/diff/69002/src/ia32/assembler-ia32.h#newcode219 src/ia32/assembler-ia32.h:219: struct X87TopOfStackProxyRegister : IntelDoubleRegister { s/Proxy// https://codereview.chromium.org/10701054/diff/69002/src/log.cc File src/log.cc (right): https://codereview.chromium.org/10701054/diff/69002/src/log.cc#newcode1 src/log.cc:1: // Copyright 2012 the V8 project authors. All rights reserved. you know.. https://codereview.chromium.org/10701054/diff/69002/src/safepoint-table.cc File src/safepoint-table.cc (left): https://codereview.chromium.org/10701054/diff/69002/src/safepoint-table.cc#oldcode164 src/safepoint-table.cc:164: int target_offset = assembler->pc_offset() + Deoptimizer::patch_size(); Please make sure it's safe not to do this. https://codereview.chromium.org/10701054/diff/69002/src/serialize.cc File src/serialize.cc (right): https://codereview.chromium.org/10701054/diff/69002/src/serialize.cc#newcode540 src/serialize.cc:540: Add(address, LAZY_DEOPTIMIZATION, 51 + entry, "lazy_deopt"); 52! https://codereview.chromium.org/10701054/diff/69002/src/utils.h File src/utils.h (right): https://codereview.chromium.org/10701054/diff/69002/src/utils.h#newcode1035 src/utils.h:1035: static const int kFirstUsableId = 4; every https://codereview.chromium.org/10701054/diff/69002/src/utils.h#newcode1038 src/utils.h:1038: static const int kStubEntryId = 5; every https://codereview.chromium.org/10701054/diff/69002/test/mjsunit/keyed-call-ic.js File test/mjsunit/keyed-call-ic.js (left): https://codereview.chromium.org/10701054/diff/69002/test/mjsunit/keyed-call-ic.js#oldcode64 test/mjsunit/keyed-call-ic.js:64: var f = new F(); uhm...? https://codereview.chromium.org/10701054/diff/69002/tools/gyp/v8.gyp File tools/gyp/v8.gyp (right): https://codereview.chromium.org/10701054/diff/69002/tools/gyp/v8.gyp#newcode350 tools/gyp/v8.gyp:350: '../../src/hydrogen.cc', might wanna remove this. https://codereview.chromium.org/10701054/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
