I fixed the issues described in the comments.
I also did the following:
- Do what "ReplaceCheckedValues" did inside the
new "EliminateRedundantChecks".
- Do not move the index when we move a check, instead recreate it just
before
the "target" check.
- Also add a HSimulate instruction to the index copy.
- In BoundsCheckBbData, added fields to remember created indexes and their
HSimulate.
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,
On 2012/04/12 09:45:42, Sven Panne wrote:
To quote memegen: Just drop the "BoundsCheckKey" part in the function
name, it's
cleaner... ;-)
Done.
http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2597
src/hydrogen.cc:2597: if (check->index()->IsAdd()) {
On 2012/04/12 09:45:42, Sven Panne wrote:
It might be worthwhile to reduce the redundancy and nesting here
somehow, a
human brain's stack depth is very limited. :-)
Done.
http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2690
src/hydrogen.cc:2690: BoundsCheckBbData* father_in_dt_;
On 2012/04/12 09:45:42, Sven Panne wrote:
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. ;-)
Done.
http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2693
src/hydrogen.cc:2693: bool BoundsCheckKeyMatch(void* key1, void* key2) {
On 2012/04/12 09:45:42, Sven Panne wrote:
This should probably be "static".
Done.
http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2701
src/hydrogen.cc:2701: void EliminateRedundantChecks(HBasicBlock* bb) {
On 2012/04/12 09:45:42, Sven Panne wrote:
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.
Done.
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()) {
On 2012/04/12 09:45:42, Sven Panne wrote:
Perhaps the first loop can be extracted into a separate function?
Done.
http://codereview.chromium.org/10032029/diff/1/src/hydrogen.cc#newcode2775
src/hydrogen.cc:2775: map = new ZoneHashMap(BoundsCheckKeyMatch);
On 2012/04/12 09:45:42, Sven Panne wrote:
Using an initializer is a nicer and more standard way for doing this.
Done.
http://codereview.chromium.org/10032029/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
