Fix HConstant::InNewSpace() for parallel compilation. (issue 13977019)

1 view
Skip to first unread message

mstar...@chromium.org

unread,
Apr 29, 2013, 1:24:43 PM4/29/13
to mvst...@chromium.org, v8-...@googlegroups.com
Reviewers: mvstanton,

Description:
Fix HConstant::InNewSpace() for parallel compilation.

R=mvst...@chromium.org

Please review this at https://codereview.chromium.org/13977019/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
M src/hydrogen-instructions.h
M src/hydrogen-instructions.cc
M src/hydrogen.cc
M src/ia32/lithium-ia32.cc
M src/x64/lithium-x64.cc


Index: src/hydrogen-instructions.cc
diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc
index
aadf87572df411eb5d8c9c9aeb1ce8b985010aea..70e2395574747085a8a554cc0a84d00eecd81390
100644
--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -2062,7 +2062,12 @@ HConstant::HConstant(Handle<Object> handle,
Representation r)
has_int32_value_(false),
has_double_value_(false),
is_internalized_string_(false),
+ is_not_in_new_space_(true),
boolean_value_(handle->BooleanValue()) {
+ if (handle_->IsHeapObject()) {
+ Heap* heap = Handle<HeapObject>::cast(handle)->GetHeap();
+ is_not_in_new_space_ = !heap->InNewSpace(*handle);
+ }
if (handle_->IsNumber()) {
double n = handle_->Number();
has_int32_value_ = IsInteger32(n);
@@ -2091,12 +2096,14 @@ HConstant::HConstant(Handle<Object> handle,
Representation r,
HType type,
bool is_internalize_string,
+ bool is_not_in_new_space,
bool boolean_value)
: handle_(handle),
unique_id_(unique_id),
has_int32_value_(false),
has_double_value_(false),
is_internalized_string_(is_internalize_string),
+ is_not_in_new_space_(is_not_in_new_space),
boolean_value_(boolean_value),
type_from_value_(type) {
ASSERT(!handle.is_null());
@@ -2108,12 +2115,14 @@ HConstant::HConstant(Handle<Object> handle,

HConstant::HConstant(int32_t integer_value,
Representation r,
+ bool is_not_in_new_space,
Handle<Object> optional_handle)
: handle_(optional_handle),
unique_id_(),
has_int32_value_(true),
has_double_value_(true),
is_internalized_string_(false),
+ is_not_in_new_space_(is_not_in_new_space),
boolean_value_(integer_value != 0),
int32_value_(integer_value),
double_value_(FastI2D(integer_value)) {
@@ -2123,12 +2132,14 @@ HConstant::HConstant(int32_t integer_value,

HConstant::HConstant(double double_value,
Representation r,
+ bool is_not_in_new_space,
Handle<Object> optional_handle)
: handle_(optional_handle),
unique_id_(),
has_int32_value_(IsInteger32(double_value)),
has_double_value_(true),
is_internalized_string_(false),
+ is_not_in_new_space_(is_not_in_new_space),
boolean_value_(double_value != 0 && !std::isnan(double_value)),
int32_value_(DoubleToInt32(double_value)),
double_value_(double_value) {
@@ -2148,26 +2159,35 @@ void HConstant::Initialize(Representation r) {
HConstant* HConstant::CopyToRepresentation(Representation r, Zone* zone)
const {
if (r.IsInteger32() && !has_int32_value_) return NULL;
if (r.IsDouble() && !has_double_value_) return NULL;
- if (has_int32_value_) return new(zone) HConstant(int32_value_, r,
handle_);
- if (has_double_value_) return new(zone) HConstant(double_value_, r,
handle_);
+ if (has_int32_value_) {
+ return new(zone) HConstant(int32_value_, r, is_not_in_new_space_,
handle_);
+ }
+ if (has_double_value_) {
+ return new(zone) HConstant(double_value_, r, is_not_in_new_space_,
handle_);
+ }
ASSERT(!handle_.is_null());
return new(zone) HConstant(handle_,
unique_id_,
r,
type_from_value_,
is_internalized_string_,
+ is_not_in_new_space_,
boolean_value_);
}


HConstant* HConstant::CopyToTruncatedInt32(Zone* zone) const {
if (has_int32_value_) {
- return new(zone) HConstant(
- int32_value_, Representation::Integer32(), handle_);
+ return new(zone) HConstant(int32_value_,
+ Representation::Integer32(),
+ is_not_in_new_space_,
+ handle_);
}
if (has_double_value_) {
- return new(zone) HConstant(
- DoubleToInt32(double_value_), Representation::Integer32(),
handle_);
+ return new(zone) HConstant(DoubleToInt32(double_value_),
+ Representation::Integer32(),
+ is_not_in_new_space_,
+ handle_);
}
return NULL;
}
Index: src/hydrogen-instructions.h
diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index
3090f46f1f050bc0b810a9d1a4a90bb4e8026f76..c09f261c2e1772e7bfe6548c01f0c5946a4eb8f2
100644
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -3186,20 +3186,25 @@ class HConstant: public HTemplateInstruction<0> {
HConstant(Handle<Object> handle, Representation r);
HConstant(int32_t value,
Representation r,
+ bool is_not_in_new_space = true,
Handle<Object> optional_handle = Handle<Object>::null());
HConstant(double value,
Representation r,
+ bool is_not_in_new_space = true,
Handle<Object> optional_handle = Handle<Object>::null());
HConstant(Handle<Object> handle,
UniqueValueId unique_id,
Representation r,
HType type,
bool is_internalized_string,
+ bool is_not_in_new_space,
bool boolean_value);

Handle<Object> handle() {
if (handle_.is_null()) {
- handle_ = FACTORY->NewNumber(double_value_, pretenure());
+ // Default arguments to is_not_in_new_space depend on this heap
number
+ // to be tenured so that it's guaranteed not be be located in new
space.
+ handle_ = FACTORY->NewNumber(double_value_, TENURED);
}
ALLOW_HANDLE_DEREF(Isolate::Current(), "smi check");
ASSERT(has_int32_value_ || !handle_->IsSmi());
@@ -3213,15 +3218,8 @@ class HConstant: public HTemplateInstruction<0> {
std::isnan(double_value_));
}

- bool InNewSpace() const {
- if (!handle_.is_null()) {
- ALLOW_HANDLE_DEREF(isolate(), "using raw address");
- return isolate()->heap()->InNewSpace(*handle_);
- }
- // If the handle wasn't created yet, then we have a number.
- // If the handle is created it'll be tenured in old space.
- ASSERT(pretenure() == TENURED);
- return false;
+ bool NotInNewSpace() const {
+ return is_not_in_new_space_;
}

bool ImmortalImmovable() const {
@@ -3359,8 +3357,6 @@ class HConstant: public HTemplateInstruction<0> {
// HeapObject the constant originated from or is null. If the
// constant is non-numeric, handle_ always points to a valid
// constant HeapObject.
- static PretenureFlag pretenure() { return TENURED; }
-
Handle<Object> handle_;
UniqueValueId unique_id_;

@@ -3372,6 +3368,7 @@ class HConstant: public HTemplateInstruction<0> {
bool has_int32_value_ : 1;
bool has_double_value_ : 1;
bool is_internalized_string_ : 1; // TODO(yangguo): make this part of
HType.
+ bool is_not_in_new_space_ : 1;
bool boolean_value_ : 1;
int32_t int32_value_;
double double_value_;
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index
63d55dd166ac8f66ae267340080d4706b3f7f3fc..d1312202bcf6b038b34bcaec75ebf3359091f0aa
100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -641,6 +641,7 @@ HConstant* HGraph::GetConstant##Name()
{ \

Representation::Tagged(), \

htype, \

false, \
+
true, \

boolean_value); \

constant->InsertAfter(GetConstantUndefined()); \

constant_##name##_.set(constant); \
Index: src/ia32/lithium-ia32.cc
diff --git a/src/ia32/lithium-ia32.cc b/src/ia32/lithium-ia32.cc
index
7ae1f01cd43203db4802a1578c5b2bbdc6b3fd5c..cffe5b16266170542ac58f82184264f3d3de7650
100644
--- a/src/ia32/lithium-ia32.cc
+++ b/src/ia32/lithium-ia32.cc
@@ -2423,7 +2423,7 @@ LInstruction*
LChunkBuilder::DoStoreNamedField(HStoreNamedField* instr) {
}

bool can_be_constant = instr->value()->IsConstant() &&
- !HConstant::cast(instr->value())->InNewSpace() &&
+ HConstant::cast(instr->value())->NotInNewSpace() &&
!(FLAG_track_double_fields &&
instr->field_representation().IsDouble());

LOperand* val;
Index: src/x64/lithium-x64.cc
diff --git a/src/x64/lithium-x64.cc b/src/x64/lithium-x64.cc
index
4c5819076b913cbe0b3343b1aadba913bee6196e..094f5ed346c076f39077fdd1f049083d798fecd6
100644
--- a/src/x64/lithium-x64.cc
+++ b/src/x64/lithium-x64.cc
@@ -2246,7 +2246,7 @@ LInstruction*
LChunkBuilder::DoStoreNamedField(HStoreNamedField* instr) {
}

bool can_be_constant = instr->value()->IsConstant() &&
- !HConstant::cast(instr->value())->InNewSpace() &&
+ HConstant::cast(instr->value())->NotInNewSpace() &&
!(FLAG_track_double_fields &&
instr->field_representation().IsDouble());

LOperand* val;


mvst...@chromium.org

unread,
Apr 30, 2013, 3:48:31 AM4/30/13
to mstar...@chromium.org, v8-...@googlegroups.com
Hey that is great, thx for the help. lgtm.


https://codereview.chromium.org/13977019/

mstar...@chromium.org

unread,
Apr 30, 2013, 4:00:56 AM4/30/13
to mvst...@chromium.org, v8-...@googlegroups.com
Committed patchset #1 manually as r14488 (presubmit successful).

https://codereview.chromium.org/13977019/
Reply all
Reply to author
Forward
0 new messages