First round of comments...
http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2593 src/hydrogen.cc:2593: static BoundsCheckKey* CreateBoundsCheckKey(Zone* zone, To quote memegen: Just drop the "BoundsCheckKey" part in the function name, it's cleaner... ;-) http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2597 src/hydrogen.cc:2597: if (check->index()->IsAdd()) { It might be worthwhile to reduce the redundancy and nesting here somehow, a human brain's stack depth is very limited. :-) http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2690 src/hydrogen.cc:2690: BoundsCheckBbData* father_in_dt_; I think using "bb" and "dt" is a little bit cryptic, and we normally don't use the abbreviations. "bb" e.g. is already used for "block buffer", etc. Names which are blindingly obviously while writing the code aren't necessarily like this 2 weeks later. ;-) http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2693 src/hydrogen.cc:2693: bool BoundsCheckKeyMatch(void* key1, void* key2) { This should probably be "static". http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2701 src/hydrogen.cc:2701: void EliminateRedundantChecks(HBasicBlock* bb) { I don't think this function really belongs into this class. BoundsCheckTable's responsibility should just be a more sane^H^H^H cleanly typed interface to ZoneHashMap. http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2704 src/hydrogen.cc:2704: for (HInstruction* i = bb->first(); i != NULL; i = i->next()) { Perhaps the first loop can be extracted into a separate function? http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2775 src/hydrogen.cc:2775: map = new ZoneHashMap(BoundsCheckKeyMatch); Using an initializer is a nicer and more standard way for doing this. http://codereview.chromium.org/10032029/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
