Reviewers: ulan,
Message:
PTAL. This should help next time we need to debug BCE.
Description:
Add FLAG_trace_bce
Please review this at https://codereview.chromium.org/203193006/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+43, -3 lines):
M src/flag-definitions.h
M src/hydrogen-bce.cc
Index: src/flag-definitions.h
diff --git a/src/flag-definitions.h b/src/flag-definitions.h
index
d69ab89cbc1fe3078d1279f2caa57719784c2e3f..c9d6b638184f24ded3e9421bffbd4a8ab5c3d47c
100644
--- a/src/flag-definitions.h
+++ b/src/flag-definitions.h
@@ -302,6 +302,7 @@ DEFINE_bool(polymorphic_inlining, true, "polymorphic
inlining")
DEFINE_bool(use_osr, true, "use on-stack replacement")
DEFINE_bool(array_bounds_checks_elimination, true,
"perform array bounds checks elimination")
+DEFINE_bool(trace_bce, false, "trace array bounds check elimination")
DEFINE_bool(array_bounds_checks_hoisting, false,
"perform array bounds checks hoisting")
DEFINE_bool(array_index_dehoisting, true,
Index: src/hydrogen-bce.cc
diff --git a/src/hydrogen-bce.cc b/src/hydrogen-bce.cc
index
c98a03cb5ab32d78b79a214c07edf5ecd2b1ee97..19931c3e4f484d527ba10a887fe47e03f2b5c5de
100644
--- a/src/hydrogen-bce.cc
+++ b/src/hydrogen-bce.cc
@@ -30,6 +30,7 @@
namespace v8 {
namespace internal {
+
// We try to "factor up" HBoundsCheck instructions towards the root of the
// dominator tree.
// For now we handle checks where the index is like "exp + int32value".
@@ -173,7 +174,7 @@ class BoundsCheckBbData: public ZoneObject {
keep_new_check = true;
upper_check_ = new_check;
} else {
- TightenCheck(upper_check_, new_check);
+ TightenCheck(upper_check_, new_check, new_offset);
UpdateUpperOffsets(upper_check_, upper_offset_);
}
} else if (new_offset < lower_offset_) {
@@ -182,7 +183,7 @@ class BoundsCheckBbData: public ZoneObject {
keep_new_check = true;
lower_check_ = new_check;
} else {
- TightenCheck(lower_check_, new_check);
+ TightenCheck(lower_check_, new_check, new_offset);
UpdateLowerOffsets(lower_check_, lower_offset_);
}
} else {
@@ -191,12 +192,20 @@ class BoundsCheckBbData: public ZoneObject {
}
if (!keep_new_check) {
+ if (FLAG_trace_bce) {
+ OS::Print("Eliminating check #%d after tightening\n",
+ new_check->id());
+ }
new_check->block()->graph()->isolate()->counters()->
bounds_checks_eliminated()->Increment();
new_check->DeleteAndReplaceWith(new_check->ActualValue());
} else {
HBoundsCheck* first_check = new_check == lower_check_ ? upper_check_
: lower_check_;
+ if (FLAG_trace_bce) {
+ OS::Print("Moving second check #%d after first check #%d\n",
+ new_check->id(), first_check->id());
+ }
// The length is guaranteed to be live at first_check.
ASSERT(new_check->length() == first_check->length());
HInstruction* old_position = new_check->next();
@@ -275,11 +284,16 @@ class BoundsCheckBbData: public ZoneObject {
}
void TightenCheck(HBoundsCheck* original_check,
- HBoundsCheck* tighter_check) {
+ HBoundsCheck* tighter_check,
+ int32_t new_offset) {
ASSERT(original_check->length() == tighter_check->length());
MoveIndexIfNecessary(tighter_check->index(), original_check,
tighter_check);
original_check->ReplaceAllUsesWith(original_check->index());
original_check->SetOperandAt(0, tighter_check->index());
+ if (FLAG_trace_bce) {
+ OS::Print("Tightened check #%d with offset %d from #%d\n",
+ original_check->id(), new_offset, tighter_check->id());
+ }
}
DISALLOW_COPY_AND_ASSIGN(BoundsCheckBbData);
@@ -389,11 +403,32 @@ BoundsCheckBbData*
HBoundsCheckEliminationPhase::PreProcessBlock(
bb_data_list,
NULL);
*data_p = bb_data_list;
+ if (FLAG_trace_bce) {
+ OS::Print("Fresh bounds check data for block #%d: [%d]\n",
+ bb->block_id(), offset);
+ }
} else if (data->OffsetIsCovered(offset)) {
bb->graph()->isolate()->counters()->
bounds_checks_eliminated()->Increment();
+ if (FLAG_trace_bce) {
+ OS::Print("Eliminating bounds check #%d, offset %d is covered\n",
+ check->id(), offset);
+ }
check->DeleteAndReplaceWith(check->ActualValue());
} else if (data->BasicBlock() == bb) {
+ // TODO(jkummerow): I think the following logic would be preferable:
+ // if (data->Basicblock() == bb ||
+ // graph()->use_optimistic_licm() ||
+ // bb->IsLoopSuccessorDominator()) {
+ // data->CoverCheck(check, offset)
+ // } else {
+ // /* add pristine BCBbData like in (data == NULL) case above */
+ // }
+ // Even better would be: distinguish between read-only
dominator-imposed
+ // knowledge and modifiable upper/lower checks.
+ // What happens currently is that the first bounds check in a
dominated
+ // block will stay around while any further checks are hoisted out,
+ // which doesn't make sense. Investigate/fix this in a future CL.
data->CoverCheck(check, offset);
} else if (graph()->use_optimistic_licm() ||
bb->IsLoopSuccessorDominator()) {
@@ -411,6 +446,10 @@ BoundsCheckBbData*
HBoundsCheckEliminationPhase::PreProcessBlock(
data->UpperCheck(),
bb_data_list,
data);
+ if (FLAG_trace_bce) {
+ OS::Print("Updated bounds check data for block #%d: [%d - %d]\n",
+ bb->block_id(), new_lower_offset, new_upper_offset);
+ }
table_.Insert(key, bb_data_list, zone());
}
}
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
