http://codereview.chromium.org/8373029/diff/2002/src/hydrogen.cc File src/hydrogen.cc (right):
http://codereview.chromium.org/8373029/diff/2002/src/hydrogen.cc#newcode2693 src/hydrogen.cc:2693: enum { We don't use anonymous enums http://codereview.chromium.org/8373029/diff/2002/src/hydrogen.cc#newcode2694 src/hydrogen.cc:2694: none, smi, string use either kSmiSwitch or SMI_SWITCH naming for enum-members. http://codereview.chromium.org/8373029/diff/2002/src/hydrogen.cc#newcode2703 src/hydrogen.cc:2703: if (clause_type == none) { I would suggest extracting loop part that computes switch type into a separate method just to make it cleaner. http://codereview.chromium.org/8373029/diff/2002/src/hydrogen.cc#newcode2711 src/hydrogen.cc:2711: } else if (clause->label()->IsSmiLiteral() && clause_type == string || clause can be neither smi nor string literal. I think what you want here is (clause_type == kSmiSwitch && !clause->label()->IsSmiLiteral()) || ... http://codereview.chromium.org/8373029/diff/2002/src/hydrogen.cc#newcode2741 src/hydrogen.cc:2741: compare = reinterpret_cast<HControlInstruction*>(compare_); I don't think you need reinterpret_cast here. HCompareObjectEqAndBranch* should be assignable to HControlInstruction. http://codereview.chromium.org/8373029/diff/2002/src/hydrogen.cc#newcode2743 src/hydrogen.cc:2743: HCompareObjectEqAndBranch* compare_ = I think you don't need this temp variable http://codereview.chromium.org/8373029/diff/2002/src/hydrogen.cc#newcode2744 src/hydrogen.cc:2744: new(zone()) HCompareObjectEqAndBranch(tag_value, label_value); Why not CompareIDAndBranch? Once loop that computes type is extracted you can use computed type to emit is-symbol test before the switch. http://codereview.chromium.org/8373029/diff/2002/src/hydrogen.cc#newcode2745 src/hydrogen.cc:2745: compare = reinterpret_cast<HControlInstruction*>(compare_); I don't think you need reinterpret_cast here. HCompareObjectEqAndBranch* should be assignable to HControlInstruction. http://codereview.chromium.org/8373029/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
