Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(428)

Unified Diff: src/hydrogen.cc

Issue 11486007: Fix for when array bounds check elimination tries to modify a phi index. (Closed) Base URL: https://v8.googlecode.com/svn/branches/bleeding_edge
Patch Set: Created 8 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | test/mjsunit/array-bounds-check-removal.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index d1e5b51a5e3e94ce7aac330bcf6df14a9ba0f3d4..1afa772665a7fe37543834ac532f0bafaae5dadd 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -3446,7 +3446,10 @@ class BoundsCheckBbData: public ZoneObject {
// (either upper or lower; note that HasSingleCheck() becomes false).
// Otherwise one of the current checks is modified so that it also covers
// new_offset, and new_check is removed.
- void CoverCheck(HBoundsCheck* new_check,
+ //
+ // If the check cannot be modified because the context is unknown it
+ // returns false, otherwise it returns true.
+ bool CoverCheck(HBoundsCheck* new_check,
int32_t new_offset) {
ASSERT(new_check->index()->representation().IsInteger32());
bool keep_new_check = false;
@@ -3457,12 +3460,13 @@ class BoundsCheckBbData: public ZoneObject {
keep_new_check = true;
upper_check_ = new_check;
} else {
- BuildOffsetAdd(upper_check_,
- &added_upper_index_,
- &added_upper_offset_,
- Key()->IndexBase(),
- new_check->index()->representation(),
- new_offset);
+ bool result = BuildOffsetAdd(upper_check_,
+ &added_upper_index_,
+ &added_upper_offset_,
+ Key()->IndexBase(),
+ new_check->index()->representation(),
+ new_offset);
+ if (!result) return false;
upper_check_->SetOperandAt(0, added_upper_index_);
}
} else if (new_offset < lower_offset_) {
@@ -3471,12 +3475,13 @@ class BoundsCheckBbData: public ZoneObject {
keep_new_check = true;
lower_check_ = new_check;
} else {
- BuildOffsetAdd(lower_check_,
- &added_lower_index_,
- &added_lower_offset_,
- Key()->IndexBase(),
- new_check->index()->representation(),
- new_offset);
+ bool result = BuildOffsetAdd(lower_check_,
+ &added_lower_index_,
+ &added_lower_offset_,
+ Key()->IndexBase(),
+ new_check->index()->representation(),
+ new_offset);
+ if (!result) return false;
lower_check_->SetOperandAt(0, added_lower_index_);
}
} else {
@@ -3486,6 +3491,8 @@ class BoundsCheckBbData: public ZoneObject {
if (!keep_new_check) {
new_check->DeleteAndReplaceWith(NULL);
}
+
+ return true;
}
void RemoveZeroOperations() {
@@ -3528,20 +3535,32 @@ class BoundsCheckBbData: public ZoneObject {
BoundsCheckBbData* next_in_bb_;
BoundsCheckBbData* father_in_dt_;
- void BuildOffsetAdd(HBoundsCheck* check,
+ // Given an existing add instruction and a bounds check it tries to
+ // find the current context (either of the add or of the check index).
+ HValue* IndexContext(HAdd* add, HBoundsCheck* check) {
+ if (add != NULL)
+ return add->context();
Jakob Kummerow 2012/12/11 13:42:15 nit: {}
+ if (check->index()->IsBinaryOperation())
+ return HBinaryOperation::cast(check->index())->context();
Jakob Kummerow 2012/12/11 13:42:15 nit: {}
+ return NULL;
+ }
+
+ // This function returns false if it cannot build the add because the
+ // current context cannot be determined.
+ bool BuildOffsetAdd(HBoundsCheck* check,
HAdd** add,
HConstant** constant,
HValue* original_value,
Representation representation,
int32_t new_offset) {
+ HValue* index_context = IndexContext(*add, check);
+ if (index_context == NULL) return false;
+
HConstant* new_constant = new(BasicBlock()->zone())
HConstant(new_offset, Representation::Integer32());
if (*add == NULL) {
new_constant->InsertBefore(check);
- // Because of the bounds checks elimination algorithm, the index is always
- // an HAdd or an HSub here, so we can safely cast to an HBinaryOperation.
- HValue* context = HBinaryOperation::cast(check->index())->context();
- *add = new(BasicBlock()->zone()) HAdd(context,
+ *add = new(BasicBlock()->zone()) HAdd(index_context,
original_value,
new_constant);
(*add)->AssumeRepresentation(representation);
@@ -3551,6 +3570,7 @@ class BoundsCheckBbData: public ZoneObject {
(*constant)->DeleteAndReplaceWith(new_constant);
}
*constant = new_constant;
+ return true;
}
void RemoveZeroAdd(HAdd** add, HConstant** constant) {
@@ -3625,9 +3645,11 @@ void HGraph::EliminateRedundantBoundsChecks(HBasicBlock* bb,
*data_p = bb_data_list;
} else if (data->OffsetIsCovered(offset)) {
check->DeleteAndReplaceWith(NULL);
- } else if (data->BasicBlock() == bb) {
- data->CoverCheck(check, offset);
- } else {
+ } else if (data->BasicBlock() != bb ||
+ !data->CoverCheck(check, offset)) {
Jakob Kummerow 2012/12/11 13:42:15 nit: please align: } else if (data->BasicBlock
+ // If the check is in the current BB we try to modify it by calling
+ // "CoverCheck", but if also that fails we record the current offsets
+ // in a new data instance because from now on they are covered.
int32_t new_lower_offset = offset < data->LowerOffset()
? offset
: data->LowerOffset();
« no previous file with comments | « no previous file | test/mjsunit/array-bounds-check-removal.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698