Revision: 13196
Author: [email protected]
Date: Tue Dec 11 09:41:11 2012
Log: Merged r13193, r13195 into trunk branch.
Fix for when array bounds check elimination tries to modify a phi index.
Clear optimized code map during incremental marking.
[email protected]
Review URL: https://codereview.chromium.org/11529014
http://code.google.com/p/v8/source/detail?r=13196
Modified:
/trunk/src/hydrogen.cc
/trunk/src/objects-inl.h
/trunk/src/objects-visiting-inl.h
/trunk/src/objects.cc
/trunk/src/objects.h
/trunk/src/version.cc
/trunk/test/mjsunit/array-bounds-check-removal.js
=======================================
--- /trunk/src/hydrogen.cc Mon Dec 10 11:00:50 2012
+++ /trunk/src/hydrogen.cc Tue Dec 11 09:41:11 2012
@@ -3446,7 +3446,10 @@
// (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 @@
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 @@
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 @@
if (!keep_new_check) {
new_check->DeleteAndReplaceWith(NULL);
}
+
+ return true;
}
void RemoveZeroOperations() {
@@ -3528,20 +3535,34 @@
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();
+ }
+ if (check->index()->IsBinaryOperation()) {
+ return HBinaryOperation::cast(check->index())->context();
+ }
+ 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 +3572,7 @@
(*constant)->DeleteAndReplaceWith(new_constant);
}
*constant = new_constant;
+ return true;
}
void RemoveZeroAdd(HAdd** add, HConstant** constant) {
@@ -3625,9 +3647,11 @@
*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)) {
+ // 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();
=======================================
--- /trunk/src/objects-inl.h Mon Dec 10 11:00:50 2012
+++ /trunk/src/objects-inl.h Tue Dec 11 09:41:11 2012
@@ -4308,11 +4308,10 @@
void SharedFunctionInfo::BeforeVisitingPointers() {
if (IsInobjectSlackTrackingInProgress()) DetachInitialMap();
+}
- // Flush optimized code map on major GC.
- // Note: we may experiment with rebuilding it or retaining entries
- // which should survive as we iterate through optimized functions
- // anyway.
+
+void SharedFunctionInfo::ClearOptimizedCodeMap() {
set_optimized_code_map(Smi::FromInt(0));
}
=======================================
--- /trunk/src/objects-visiting-inl.h Fri Nov 16 06:43:43 2012
+++ /trunk/src/objects-visiting-inl.h Tue Dec 11 09:41:11 2012
@@ -299,6 +299,13 @@
if (shared->ic_age() != heap->global_ic_age()) {
shared->ResetForNewContext(heap->global_ic_age());
}
+ if (FLAG_cache_optimized_code) {
+ // Flush optimized code map on major GC.
+ // TODO(mstarzinger): We may experiment with rebuilding it or with
+ // retaining entries which should survive as we iterate through
+ // optimized functions anyway.
+ shared->ClearOptimizedCodeMap();
+ }
MarkCompactCollector* collector = heap->mark_compact_collector();
if (collector->is_code_flushing_enabled()) {
if (IsFlushable(heap, shared)) {
=======================================
--- /trunk/src/objects.cc Mon Dec 10 11:00:50 2012
+++ /trunk/src/objects.cc Tue Dec 11 09:41:11 2012
@@ -8071,11 +8071,6 @@
CompilationInfoWithZone info(shared);
return CompileLazyHelper(&info, flag);
}
-
-
-void SharedFunctionInfo::ClearOptimizedCodeMap() {
- set_optimized_code_map(Smi::FromInt(0));
-}
void SharedFunctionInfo::AddToOptimizedCodeMap(
=======================================
--- /trunk/src/objects.h Mon Dec 10 11:00:50 2012
+++ /trunk/src/objects.h Tue Dec 11 09:41:11 2012
@@ -5444,7 +5444,7 @@
void InstallFromOptimizedCodeMap(JSFunction* function, int index);
// Clear optimized code map.
- void ClearOptimizedCodeMap();
+ inline void ClearOptimizedCodeMap();
// Add a new entry to the optimized code map.
static void AddToOptimizedCodeMap(Handle<SharedFunctionInfo> shared,
=======================================
--- /trunk/src/version.cc Mon Dec 10 11:00:50 2012
+++ /trunk/src/version.cc Tue Dec 11 09:41:11 2012
@@ -35,7 +35,7 @@
#define MAJOR_VERSION 3
#define MINOR_VERSION 15
#define BUILD_NUMBER 11
-#define PATCH_LEVEL 0
+#define PATCH_LEVEL 1
// Use 1 for candidates and 0 otherwise.
// (Boolean macro values are not supported by all preprocessors.)
#define IS_CANDIDATE_VERSION 0
=======================================
--- /trunk/test/mjsunit/array-bounds-check-removal.js Thu Sep 20 05:51:09
2012
+++ /trunk/test/mjsunit/array-bounds-check-removal.js Tue Dec 11 09:41:11
2012
@@ -178,5 +178,29 @@
assertTrue(%GetOptimizationStatus(short_test) != 1);
+// A test for when we would modify a phi index.
+var data_phi = [0, 1, 2, 3, 4, 5, 6, 7, 8];
+function test_phi(a, base, check) {
+ var index;
+ if (check) {
+ index = base + 1;
+ } else {
+ index = base + 2;
+ }
+ var result = a[index];
+ result += a[index + 1];
+ result += a[index - 1];
+ return result;
+}
+var result_phi = 0;
+result_phi = test_phi(data_phi, 3, true);
+assertEquals(12, result_phi);
+result_phi = test_phi(data_phi, 3, true);
+assertEquals(12, result_phi);
+%OptimizeFunctionOnNextCall(test_phi);
+result_phi = test_phi(data_phi, 3, true);
+assertEquals(12, result_phi);
+
+
gc();
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
