Reviewers: Sven Panne,
Message:
Easy starter for the new week :-)
PTAL
Description:
Fix field type handling in load elimination.
Drive-by-fix: map_set() must return a pointer to the UniqueSet
instead of a copy.
Please review this at https://codereview.chromium.org/244383002/
SVN Base: [email protected]:v8/v8.git@master
Affected files (+33, -30 lines):
M src/arm/lithium-codegen-arm.cc
M src/arm64/lithium-codegen-arm64.cc
M src/hydrogen-check-elimination.cc
M src/hydrogen-instructions.h
M src/hydrogen-load-elimination.cc
M src/ia32/lithium-codegen-ia32.cc
M src/mips/lithium-codegen-mips.cc
M src/unique.h
M src/x64/lithium-codegen-x64.cc
Index: src/arm/lithium-codegen-arm.cc
diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index
8294b1879e3668c293492d7d4b944b265d1b5ad0..0c3e5795391145d69106a18ed9485a9705df31e8
100644
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -5190,15 +5190,15 @@ void LCodeGen::DoCheckMaps(LCheckMaps* instr) {
__ bind(deferred->check_maps());
}
- UniqueSet<Map> map_set = instr->hydrogen()->map_set();
+ const UniqueSet<Map>* map_set = instr->hydrogen()->map_set();
Label success;
- for (int i = 0; i < map_set.size() - 1; i++) {
- Handle<Map> map = map_set.at(i).handle();
+ for (int i = 0; i < map_set->size() - 1; i++) {
+ Handle<Map> map = map_set->at(i).handle();
__ CompareMap(map_reg, map, &success);
__ b(eq, &success);
}
- Handle<Map> map = map_set.at(map_set.size() - 1).handle();
+ Handle<Map> map = map_set->at(map_set->size() - 1).handle();
__ CompareMap(map_reg, map, &success);
if (instr->hydrogen()->has_migration_target()) {
__ b(ne, deferred->entry());
Index: src/arm64/lithium-codegen-arm64.cc
diff --git a/src/arm64/lithium-codegen-arm64.cc
b/src/arm64/lithium-codegen-arm64.cc
index
292c7a0f287fc9bd904811b5f9c6673a9a6e392f..a59120c76daf26878664091b3352ce89483e9a06
100644
--- a/src/arm64/lithium-codegen-arm64.cc
+++ b/src/arm64/lithium-codegen-arm64.cc
@@ -2135,14 +2135,14 @@ void LCodeGen::DoCheckMaps(LCheckMaps* instr) {
__ Bind(deferred->check_maps());
}
- UniqueSet<Map> map_set = instr->hydrogen()->map_set();
+ const UniqueSet<Map>* map_set = instr->hydrogen()->map_set();
Label success;
- for (int i = 0; i < map_set.size() - 1; i++) {
- Handle<Map> map = map_set.at(i).handle();
+ for (int i = 0; i < map_set->size() - 1; i++) {
+ Handle<Map> map = map_set->at(i).handle();
__ CompareMap(map_reg, map);
__ B(eq, &success);
}
- Handle<Map> map = map_set.at(map_set.size() - 1).handle();
+ Handle<Map> map = map_set->at(map_set->size() - 1).handle();
__ CompareMap(map_reg, map);
// We didn't match a map.
Index: src/hydrogen-check-elimination.cc
diff --git a/src/hydrogen-check-elimination.cc
b/src/hydrogen-check-elimination.cc
index
8eb760120d88cd958989c7cd9fccbbe65b77ed50..66851b0ad9da75a22cef90a880dcab63521c8667
100644
--- a/src/hydrogen-check-elimination.cc
+++ b/src/hydrogen-check-elimination.cc
@@ -305,7 +305,7 @@ class HCheckTable : public ZoneObject {
if (entry != NULL) {
// entry found;
MapSet a = entry->maps_;
- MapSet i = instr->map_set().Copy(phase_->zone());
+ MapSet i = instr->map_set()->Copy(phase_->zone());
if (a->IsSubset(i)) {
// The first check is more strict; the second is redundant.
if (entry->check_ != NULL) {
@@ -364,7 +364,7 @@ class HCheckTable : public ZoneObject {
}
} else {
// No entry; insert a new one.
- Insert(object, instr, instr->map_set().Copy(phase_->zone()));
+ Insert(object, instr, instr->map_set()->Copy(phase_->zone()));
}
}
@@ -384,8 +384,8 @@ class HCheckTable : public ZoneObject {
// Reduce a load of the map field when it is known to be a constant.
if (!IsMapAccess(instr->access())) {
// Check if we introduce field maps here.
- if (instr->map_set().size() != 0) {
- Insert(instr, instr, instr->map_set().Copy(phase_->zone()));
+ if (instr->map_set()->size() != 0) {
+ Insert(instr, instr, instr->map_set()->Copy(phase_->zone()));
}
return;
}
Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index
7094cab29ea5acd180025e2b4148d524a9ba8c9c..ee4056bc7e0a39fa1832e6acf6b10c4f840b33f3
100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -2770,7 +2770,7 @@ class HCheckMaps V8_FINAL : public
HTemplateInstruction<2> {
HValue* typecheck() { return OperandAt(1); }
Unique<Map> first_map() const { return map_set_.at(0); }
- UniqueSet<Map> map_set() const { return map_set_; }
+ const UniqueSet<Map>* map_set() const { return &map_set_; }
void set_map_set(UniqueSet<Map>* maps, Zone *zone) {
map_set_.Clear();
@@ -6172,7 +6172,7 @@ class HLoadNamedField V8_FINAL : public
HTemplateInstruction<2> {
return access_.representation();
}
- UniqueSet<Map> map_set() const { return map_set_; }
+ const UniqueSet<Map>* map_set() const { return &map_set_; }
virtual bool HasEscapingOperandAt(int index) V8_OVERRIDE { return false;
}
virtual bool HasOutOfBoundsAccess(int size) V8_OVERRIDE {
Index: src/hydrogen-load-elimination.cc
diff --git a/src/hydrogen-load-elimination.cc
b/src/hydrogen-load-elimination.cc
index
f84eac046b5d88297ba9d75eda72c57e356037af..094793473aeacb7ac4f25a981194f836780a3091
100644
--- a/src/hydrogen-load-elimination.cc
+++ b/src/hydrogen-load-elimination.cc
@@ -78,7 +78,10 @@ class HLoadEliminationTable : public ZoneObject {
HValue* result = load(l);
if (result != instr &&
result->type().Equals(instr->type()) &&
- result->representation().Equals(instr->representation())) {
+ result->representation().Equals(instr->representation()) &&
+ (!result->IsLoadNamedField() ||
+ HLoadNamedField::cast(instr)->map_set()->IsSubset(
+ HLoadNamedField::cast(result)->map_set()))) {
// The load can be replaced with a previous load or a value.
TRACE((" replace L%d -> v%d\n", instr->id(), result->id()));
instr->DeleteAndReplaceWith(result);
Index: src/ia32/lithium-codegen-ia32.cc
diff --git a/src/ia32/lithium-codegen-ia32.cc
b/src/ia32/lithium-codegen-ia32.cc
index
fad23ade32c6e6c22ff6bd70461a9921756be0da..f9c43741e20c39b99a1348456f6254958f2a4fa1
100644
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -5638,15 +5638,15 @@ void LCodeGen::DoCheckMaps(LCheckMaps* instr) {
__ bind(deferred->check_maps());
}
- UniqueSet<Map> map_set = instr->hydrogen()->map_set();
+ const UniqueSet<Map>* map_set = instr->hydrogen()->map_set();
Label success;
- for (int i = 0; i < map_set.size() - 1; i++) {
- Handle<Map> map = map_set.at(i).handle();
+ for (int i = 0; i < map_set->size() - 1; i++) {
+ Handle<Map> map = map_set->at(i).handle();
__ CompareMap(reg, map);
__ j(equal, &success, Label::kNear);
}
- Handle<Map> map = map_set.at(map_set.size() - 1).handle();
+ Handle<Map> map = map_set->at(map_set->size() - 1).handle();
__ CompareMap(reg, map);
if (instr->hydrogen()->has_migration_target()) {
__ j(not_equal, deferred->entry());
Index: src/mips/lithium-codegen-mips.cc
diff --git a/src/mips/lithium-codegen-mips.cc
b/src/mips/lithium-codegen-mips.cc
index
b60ad50408cf2444f68e62c7daa77a3c96111052..9a9688bde5ce97c7f3236f61b25a7906c7a7c84f
100644
--- a/src/mips/lithium-codegen-mips.cc
+++ b/src/mips/lithium-codegen-mips.cc
@@ -5197,13 +5197,13 @@ void LCodeGen::DoCheckMaps(LCheckMaps* instr) {
__ bind(deferred->check_maps());
}
- UniqueSet<Map> map_set = instr->hydrogen()->map_set();
+ const UniqueSet<Map>* map_set = instr->hydrogen()->map_set();
Label success;
- for (int i = 0; i < map_set.size() - 1; i++) {
- Handle<Map> map = map_set.at(i).handle();
+ for (int i = 0; i < map_set->size() - 1; i++) {
+ Handle<Map> map = map_set->at(i).handle();
__ CompareMapAndBranch(map_reg, map, &success, eq, &success);
}
- Handle<Map> map = map_set.at(map_set.size() - 1).handle();
+ Handle<Map> map = map_set->at(map_set->size() - 1).handle();
// Do the CompareMap() directly within the Branch() and DeoptimizeIf().
if (instr->hydrogen()->has_migration_target()) {
__ Branch(deferred->entry(), ne, map_reg, Operand(map));
Index: src/unique.h
diff --git a/src/unique.h b/src/unique.h
index
2f6008c5a2b007a3f763c8849be06223ea988c81..b1b4e97bc1da84f92996e560a46a9b4b2bd6fccb
100644
--- a/src/unique.h
+++ b/src/unique.h
@@ -189,7 +189,7 @@ class UniqueSet V8_FINAL : public ZoneObject {
}
// Compare this set against another set. O(|this|).
- bool Equals(UniqueSet<T>* that) const {
+ bool Equals(const UniqueSet<T>* that) const {
if (that->size_ != this->size_) return false;
for (int i = 0; i < this->size_; i++) {
if (this->array_[i] != that->array_[i]) return false;
@@ -200,7 +200,7 @@ class UniqueSet V8_FINAL : public ZoneObject {
// Check whether this set contains the given element. O(|this|)
// TODO(titzer): use binary search for large sets to make this O(log|
this|)
template <typename U>
- bool Contains(Unique<U> elem) const {
+ bool Contains(const Unique<U> elem) const {
for (int i = 0; i < size_; i++) {
if (this->array_[i] == elem) return true;
}
@@ -208,7 +208,7 @@ class UniqueSet V8_FINAL : public ZoneObject {
}
// Check if this set is a subset of the given set. O(|this| + |that|).
- bool IsSubset(UniqueSet<T>* that) const {
+ bool IsSubset(const UniqueSet<T>* that) const {
if (that->size_ < this->size_) return false;
int j = 0;
for (int i = 0; i < this->size_; i++) {
Index: src/x64/lithium-codegen-x64.cc
diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index
44651cb942b7ac3063d018f04d71cf2cfe7f6806..837bbcd0a55d77c8b45b54053ba31326ef9aac2a
100644
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -5052,15 +5052,15 @@ void LCodeGen::DoCheckMaps(LCheckMaps* instr) {
__ bind(deferred->check_maps());
}
- UniqueSet<Map> map_set = instr->hydrogen()->map_set();
+ const UniqueSet<Map>* map_set = instr->hydrogen()->map_set();
Label success;
- for (int i = 0; i < map_set.size() - 1; i++) {
- Handle<Map> map = map_set.at(i).handle();
+ for (int i = 0; i < map_set->size() - 1; i++) {
+ Handle<Map> map = map_set->at(i).handle();
__ CompareMap(reg, map);
__ j(equal, &success, Label::kNear);
}
- Handle<Map> map = map_set.at(map_set.size() - 1).handle();
+ Handle<Map> map = map_set->at(map_set->size() - 1).handle();
__ CompareMap(reg, map);
if (instr->hydrogen()->has_migration_target()) {
__ j(not_equal, deferred->entry());
--
--
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.
