Reviewers: titzer, danno,
Message:
On 2013/12/04 16:10:19, danno wrote:
Ben, can you take a look?
Weiliang, did you consider my other suggestion to only check the map once
explicitly? This can be done both for the IsClass case and the non-IsClass
case,
since the equality guarantees one check suffices.
Hi Danno,
I have followed your suggestion to only check the map once. But I does not
do it
when building IR graph but just leave it to check_elimination pass when we
have
more info to make the correct decision.
Description:
Remove half of the checks for comparing two Obejcts
BUG=
Please review this at https://codereview.chromium.org/104843002/
SVN Base: git://github.com/v8/v8.git@master
Affected files (+50, -3 lines):
M src/hydrogen-check-elimination.cc
M src/hydrogen.cc
Index: src/hydrogen-check-elimination.cc
diff --git a/src/hydrogen-check-elimination.cc
b/src/hydrogen-check-elimination.cc
index
bbd3042fb7af6659f4107fffb7127e1298000593..ca37fa1c9d9fb3c151aa88b645b1a482b3767cc1
100644
--- a/src/hydrogen-check-elimination.cc
+++ b/src/hydrogen-check-elimination.cc
@@ -101,6 +101,10 @@ class HCheckTable : public ZoneObject {
ReduceCheckHeapObject(HCheckHeapObject::cast(instr));
break;
}
+ case HValue::kCompareObjectEqAndBranch: {
+
ReduceCompareObjectEqAndBranch(HCompareObjectEqAndBranch::cast(instr));
+ break;
+ }
default: {
// If the instruction changes maps uncontrollably, drop everything.
if (instr->CheckGVNFlag(kChangesMaps) ||
@@ -266,6 +270,50 @@ class HCheckTable : public ZoneObject {
}
}
+ void ReduceCompareObjectEqAndBranch(HCompareObjectEqAndBranch* instr) {
+ HCheckMaps* removed = NULL;
+ HValue* left = instr->left();
+ HValue* right = instr->right();
+ if (left->IsCheckMaps() && right->IsCheckMaps()) {
+ HCheckMaps* left_check = HCheckMaps::cast(left);
+ HCheckMaps* right_check = HCheckMaps::cast(right);
+ MapSet left_maps = left_check->map_set().Copy(phase_->zone());
+ MapSet right_maps = right_check->map_set().Copy(phase_->zone());
+ if (left_maps->Equals(right_maps)) {
+ // Object equality comparison guarantees one check suffices, so
+ // eliminate the other. After LICM, we could easily find the
correct
+ // one (loop invariant object's map check could be hoisted out of
loop).
+ removed = left->block()->block_id() > right->block()->block_id()
+ ? left_check : right_check;
+ }
+ } else if (left->IsCheckMaps() || right->IsCheckMaps()) {
+ // For the case one side map check is eliminated by previous
optimization.
+ HValue* obj = left->IsCheckMaps() ? right : left;
+ HCheckMaps* check = (obj == left) ? HCheckMaps::cast(right)
+ : HCheckMaps::cast(left);
+ MapSet obj_maps = FindMaps(obj);
+ MapSet check_maps = check->map_set().Copy(phase_->zone());
+ if (obj_maps->IsSubset(check_maps)) {
+ // Obj check is more strict; the remaining check is redundant.
+ removed = check;
+ }
+ }
+
+ if (removed != NULL) {
+ // Check whether there is prior CheckHeapObject refering to the same
+ // object as the removed CheckMaps.
+ if (removed->previous()->IsCheckHeapObject()) {
+ HCheckHeapObject* cho =
HCheckHeapObject::cast(removed->previous());
+ if (cho->value() == removed->ActualValue()) {
+ cho->DeleteAndReplaceWith(cho->value());
+ INC_STAT(removed_cho_);
+ }
+ }
+ removed->DeleteAndReplaceWith(removed->ActualValue());
+ INC_STAT(removed_);
+ }
+ }
+
void ReduceStoreNamedField(HStoreNamedField* instr) {
HValue* object = instr->object()->ActualValue();
if (instr->has_transition()) {
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index
900e07ecdf9fa68a32540fc76be401d0b064a343..61b55aa49e644664dc91612c3510f2054e61d448
100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -9231,10 +9231,9 @@ void
HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) {
// Can we get away with map check and not instance type check?
if (combined_type->IsClass()) {
Handle<Map> map = combined_type->AsClass();
- AddCheckMap(left, map);
- AddCheckMap(right, map);
HCompareObjectEqAndBranch* result =
- New<HCompareObjectEqAndBranch>(left, right);
+ New<HCompareObjectEqAndBranch>(AddCheckMap(left, map),
+ AddCheckMap(right, map));
if (FLAG_emit_opt_code_positions) {
result->set_operand_position(zone(), 0,
expr->left()->position());
result->set_operand_position(zone(), 1,
expr->right()->position());
--
--
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/groups/opt_out.
