Fix a bug in deoptimization environments.

The function HEnvironment::SetExpressionStackAt did not update the
environment's history.  This function is used to patch the bailout
environment for count operations and global function calls.

Reorganize class HEnvironment to make it fit V8's style a bit better
and to try to add some sanity to which C++ functions are intended to
be inlined.

Remove the flag --trace-environment which merely duplicated data in
the hydrogen.cfg file except without enough context to be useful.

BUG=1004

Review URL: http://codereview.chromium.org/5992011

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@6137 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
This commit is contained in:
kmillikin@chromium.org 2011-01-03 16:57:46 +00:00
parent 3d7d258339
commit c1fd8bcf60
6 changed files with 113 additions and 114 deletions

View File

@ -1016,9 +1016,6 @@ void LChunkBuilder::DoBasicBlock(HBasicBlock* block, HBasicBlock* next_block) {
HInstruction* current = block->first();
int start = chunk_->instructions()->length();
while (current != NULL && !is_aborted()) {
if (FLAG_trace_environment) {
PrintF("Process instruction %d\n", current->id());
}
// Code for constants in registers is generated lazily.
if (!current->EmitAtUses()) {
VisitInstruction(current);
@ -1125,7 +1122,7 @@ LEnvironment* LChunkBuilder::CreateEnvironment(HEnvironment* hydrogen_env) {
LEnvironment* outer = CreateEnvironment(hydrogen_env->outer());
int ast_id = hydrogen_env->ast_id();
ASSERT(ast_id != AstNode::kNoNumber);
int value_count = hydrogen_env->values()->length();
int value_count = hydrogen_env->length();
LEnvironment* result = new LEnvironment(hydrogen_env->closure(),
ast_id,
hydrogen_env->parameter_count(),
@ -2058,13 +2055,7 @@ LInstruction* LChunkBuilder::DoSimulate(HSimulate* instr) {
}
}
if (FLAG_trace_environment) {
PrintF("Reconstructed environment ast_id=%d, instr_id=%d\n",
instr->ast_id(),
instr->id());
env->PrintToStd();
}
ASSERT(env->values()->length() == instr->environment_height());
ASSERT(env->length() == instr->environment_length());
// If there is an instruction pending deoptimization environment create a
// lazy bailout instruction to capture the environment.

View File

@ -122,7 +122,6 @@ DEFINE_bool(trace_inlining, false, "trace inlining decisions")
DEFINE_bool(trace_alloc, false, "trace register allocator")
DEFINE_bool(trace_range, false, "trace range analysis")
DEFINE_bool(trace_gvn, false, "trace global value numbering")
DEFINE_bool(trace_environment, false, "trace lithium environments")
DEFINE_bool(trace_representation, false, "trace representation types")
DEFINE_bool(stress_pointer_maps, false, "pointer map for every instruction")
DEFINE_bool(stress_environments, false, "environment for every instruction")

View File

@ -1020,10 +1020,10 @@ class HChange: public HUnaryOperation {
class HSimulate: public HInstruction {
public:
HSimulate(int ast_id, int pop_count, int environment_height)
HSimulate(int ast_id, int pop_count, int environment_length)
: ast_id_(ast_id),
pop_count_(pop_count),
environment_height_(environment_height),
environment_length_(environment_length),
values_(2),
assigned_indexes_(2) {}
virtual ~HSimulate() {}
@ -1037,7 +1037,7 @@ class HSimulate: public HInstruction {
ast_id_ = id;
}
int environment_height() const { return environment_height_; }
int environment_length() const { return environment_length_; }
int pop_count() const { return pop_count_; }
const ZoneList<HValue*>* values() const { return &values_; }
int GetAssignedIndexAt(int index) const {
@ -1079,7 +1079,7 @@ class HSimulate: public HInstruction {
}
int ast_id_;
int pop_count_;
int environment_height_;
int environment_length_;
ZoneList<HValue*> values_;
ZoneList<int> assigned_indexes_;
};

View File

@ -128,7 +128,7 @@ HSimulate* HBasicBlock::CreateSimulate(int id) {
int push_count = environment->push_count();
int pop_count = environment->pop_count();
int length = environment->values()->length();
int length = environment->length();
HSimulate* instr = new HSimulate(id, pop_count, length);
for (int i = push_count - 1; i >= 0; --i) {
instr->AddPushedValue(environment->ExpressionStackAt(i));
@ -222,7 +222,7 @@ void HBasicBlock::RegisterPredecessor(HBasicBlock* pred) {
ASSERT(IsLoopHeader() || first_ == NULL);
HEnvironment* incoming_env = pred->last_environment();
if (IsLoopHeader()) {
ASSERT(phis()->length() == incoming_env->values()->length());
ASSERT(phis()->length() == incoming_env->length());
for (int i = 0; i < phis_.length(); ++i) {
phis_[i]->AddInput(incoming_env->values()->at(i));
}
@ -1982,7 +1982,7 @@ AstContext::AstContext(HGraphBuilder* owner, Expression::Context kind)
: owner_(owner), kind_(kind), outer_(owner->ast_context()) {
owner->set_ast_context(this); // Push.
#ifdef DEBUG
original_count_ = owner->environment()->total_count();
original_length_ = owner->environment()->length();
#endif
}
@ -1995,14 +1995,14 @@ AstContext::~AstContext() {
EffectContext::~EffectContext() {
ASSERT(owner()->HasStackOverflow() ||
!owner()->subgraph()->HasExit() ||
owner()->environment()->total_count() == original_count_);
owner()->environment()->length() == original_length_);
}
ValueContext::~ValueContext() {
ASSERT(owner()->HasStackOverflow() ||
!owner()->subgraph()->HasExit() ||
owner()->environment()->total_count() == original_count_ + 1);
owner()->environment()->length() == original_length_ + 1);
}
@ -2343,7 +2343,7 @@ void HGraphBuilder::SetupScope(Scope* scope) {
}
// Set the initial values of stack-allocated locals.
for (int i = count; i < environment()->values()->length(); ++i) {
for (int i = count; i < environment()->length(); ++i) {
environment()->Bind(i, undefined_constant);
}
@ -2702,7 +2702,7 @@ void HSubgraph::PreProcessOsrEntry(IterationStatement* statement) {
int osr_entry_id = statement->OsrEntryId();
// We want the correct environment at the OsrEntry instruction. Build
// it explicitly. The expression stack should be empty.
int count = osr_entry->last_environment()->total_count();
int count = osr_entry->last_environment()->length();
ASSERT(count == (osr_entry->last_environment()->parameter_count() +
osr_entry->last_environment()->local_count()));
for (int i = 0; i < count; ++i) {
@ -5279,6 +5279,19 @@ void HEnvironment::Initialize(int parameter_count,
}
void HEnvironment::Initialize(const HEnvironment* other) {
closure_ = other->closure();
values_.AddAll(other->values_);
assigned_variables_.AddAll(other->assigned_variables_);
parameter_count_ = other->parameter_count_;
local_count_ = other->local_count_;
if (other->outer_ != NULL) outer_ = other->outer_->Copy(); // Deep copy.
pop_count_ = other->pop_count_;
push_count_ = other->push_count_;
ast_id_ = other->ast_id_;
}
void HEnvironment::AddIncomingEdge(HBasicBlock* block, HEnvironment* other) {
ASSERT(!block->IsLoopHeader());
ASSERT(values_.length() == other->values_.length());
@ -5309,26 +5322,45 @@ void HEnvironment::AddIncomingEdge(HBasicBlock* block, HEnvironment* other) {
}
void HEnvironment::Initialize(const HEnvironment* other) {
closure_ = other->closure();
values_.AddAll(other->values_);
assigned_variables_.AddAll(other->assigned_variables_);
parameter_count_ = other->parameter_count_;
local_count_ = other->local_count_;
if (other->outer_ != NULL) outer_ = other->outer_->Copy(); // Deep copy.
pop_count_ = other->pop_count_;
push_count_ = other->push_count_;
ast_id_ = other->ast_id_;
void HEnvironment::Bind(int index, HValue* value) {
ASSERT(value != NULL);
if (!assigned_variables_.Contains(index)) {
assigned_variables_.Add(index);
}
values_[index] = value;
}
int HEnvironment::IndexFor(Variable* variable) const {
Slot* slot = variable->AsSlot();
ASSERT(slot != NULL && slot->IsStackAllocated());
if (slot->type() == Slot::PARAMETER) {
return slot->index() + 1;
} else {
return parameter_count_ + slot->index();
bool HEnvironment::HasExpressionAt(int index) const {
return index >= parameter_count_ + local_count_;
}
bool HEnvironment::ExpressionStackIsEmpty() const {
int first_expression = parameter_count() + local_count();
ASSERT(length() >= first_expression);
return length() == first_expression;
}
void HEnvironment::SetExpressionStackAt(int index_from_top, HValue* value) {
int count = index_from_top + 1;
int index = values_.length() - count;
ASSERT(HasExpressionAt(index));
// The push count must include at least the element in question or else
// the new value will not be included in this environment's history.
if (push_count_ < count) {
// This is the same effect as popping then re-pushing 'count' elements.
pop_count_ += (count - push_count_);
push_count_ = count;
}
values_[index] = value;
}
void HEnvironment::Drop(int count) {
for (int i = 0; i < count; ++i) {
Pop();
}
}
@ -5393,7 +5425,7 @@ HEnvironment* HEnvironment::CopyForInlining(Handle<JSFunction> target,
void HEnvironment::PrintTo(StringStream* stream) {
for (int i = 0; i < total_count(); i++) {
for (int i = 0; i < length(); i++) {
if (i == 0) stream->Add("parameters\n");
if (i == parameter_count()) stream->Add("locals\n");
if (i == parameter_count() + local_count()) stream->Add("expressions");

View File

@ -401,27 +401,33 @@ class HEnvironment: public ZoneObject {
Scope* scope,
Handle<JSFunction> closure);
// Simple accessors.
Handle<JSFunction> closure() const { return closure_; }
const ZoneList<HValue*>* values() const { return &values_; }
const ZoneList<int>* assigned_variables() const {
return &assigned_variables_;
}
int parameter_count() const { return parameter_count_; }
int local_count() const { return local_count_; }
HEnvironment* outer() const { return outer_; }
int pop_count() const { return pop_count_; }
int push_count() const { return push_count_; }
int ast_id() const { return ast_id_; }
void set_ast_id(int id) { ast_id_ = id; }
int length() const { return values_.length(); }
void Bind(Variable* variable, HValue* value) {
Bind(IndexFor(variable), value);
if (FLAG_trace_environment) {
PrintF("Slot index=%d name=%s\n",
variable->AsSlot()->index(),
*variable->name()->ToCString());
}
}
void Bind(int index, HValue* value) {
ASSERT(value != NULL);
if (!assigned_variables_.Contains(index)) {
assigned_variables_.Add(index);
}
values_[index] = value;
}
void Bind(int index, HValue* value);
HValue* Lookup(Variable* variable) const {
return Lookup(IndexFor(variable));
}
HValue* Lookup(int index) const {
HValue* result = values_[index];
ASSERT(result != NULL);
@ -434,53 +440,28 @@ class HEnvironment: public ZoneObject {
values_.Add(value);
}
HValue* Top() const { return ExpressionStackAt(0); }
HValue* ExpressionStackAt(int index_from_top) const {
int index = values_.length() - index_from_top - 1;
ASSERT(IsExpressionStackIndex(index));
return values_[index];
}
void SetExpressionStackAt(int index_from_top, HValue* value) {
int index = values_.length() - index_from_top - 1;
ASSERT(IsExpressionStackIndex(index));
values_[index] = value;
}
HValue* Pop() {
ASSERT(!IsExpressionStackEmpty());
ASSERT(!ExpressionStackIsEmpty());
if (push_count_ > 0) {
--push_count_;
ASSERT(push_count_ >= 0);
} else {
++pop_count_;
}
return values_.RemoveLast();
}
void Drop(int count) {
for (int i = 0; i < count; ++i) {
Pop();
}
void Drop(int count);
HValue* Top() const { return ExpressionStackAt(0); }
HValue* ExpressionStackAt(int index_from_top) const {
int index = length() - index_from_top - 1;
ASSERT(HasExpressionAt(index));
return values_[index];
}
Handle<JSFunction> closure() const { return closure_; }
void SetExpressionStackAt(int index_from_top, HValue* value);
// ID of the original AST node to identify deoptimization points.
int ast_id() const { return ast_id_; }
void set_ast_id(int id) { ast_id_ = id; }
const ZoneList<HValue*>* values() const { return &values_; }
const ZoneList<int>* assigned_variables() const {
return &assigned_variables_;
}
int parameter_count() const { return parameter_count_; }
int local_count() const { return local_count_; }
int push_count() const { return push_count_; }
int pop_count() const { return pop_count_; }
int total_count() const { return values_.length(); }
HEnvironment* outer() const { return outer_; }
HEnvironment* Copy() const;
HEnvironment* CopyWithoutHistory() const;
HEnvironment* CopyAsLoopHeader(HBasicBlock* block) const;
@ -496,13 +477,15 @@ class HEnvironment: public ZoneObject {
HConstant* undefined) const;
void AddIncomingEdge(HBasicBlock* block, HEnvironment* other);
void ClearHistory() {
pop_count_ = 0;
push_count_ = 0;
assigned_variables_.Clear();
}
void SetValueAt(int index, HValue* value) {
ASSERT(index < total_count());
ASSERT(index < length());
values_[index] = value;
}
@ -512,19 +495,23 @@ class HEnvironment: public ZoneObject {
private:
explicit HEnvironment(const HEnvironment* other);
bool IsExpressionStackIndex(int index) const {
return index >= parameter_count_ + local_count_;
}
bool IsExpressionStackEmpty() const {
int length = values_.length();
int first_expression = parameter_count() + local_count();
ASSERT(length >= first_expression);
return length == first_expression;
}
// True if index is included in the expression stack part of the environment.
bool HasExpressionAt(int index) const;
bool ExpressionStackIsEmpty() const;
void Initialize(int parameter_count, int local_count, int stack_height);
void Initialize(const HEnvironment* other);
int VariableToIndex(Variable* var);
int IndexFor(Variable* variable) const;
// Map a variable to an environment index. Parameter indices are shifted
// by 1 (receiver is parameter index -1 but environment index 0).
// Stack-allocated local indices are shifted by the number of parameters.
int IndexFor(Variable* variable) const {
Slot* slot = variable->AsSlot();
ASSERT(slot != NULL && slot->IsStackAllocated());
int shift = (slot->type() == Slot::PARAMETER) ? 1 : parameter_count_;
return slot->index() + shift;
}
Handle<JSFunction> closure_;
// Value array [parameters] [locals] [temporaries].
@ -567,7 +554,7 @@ class AstContext {
// We want to be able to assert, in a context-specific way, that the stack
// height makes sense when the context is filled.
#ifdef DEBUG
int original_count_;
int original_length_;
#endif
private:

View File

@ -1016,9 +1016,6 @@ void LChunkBuilder::DoBasicBlock(HBasicBlock* block, HBasicBlock* next_block) {
HInstruction* current = block->first();
int start = chunk_->instructions()->length();
while (current != NULL && !is_aborted()) {
if (FLAG_trace_environment) {
PrintF("Process instruction %d\n", current->id());
}
// Code for constants in registers is generated lazily.
if (!current->EmitAtUses()) {
VisitInstruction(current);
@ -1125,7 +1122,7 @@ LEnvironment* LChunkBuilder::CreateEnvironment(HEnvironment* hydrogen_env) {
LEnvironment* outer = CreateEnvironment(hydrogen_env->outer());
int ast_id = hydrogen_env->ast_id();
ASSERT(ast_id != AstNode::kNoNumber);
int value_count = hydrogen_env->values()->length();
int value_count = hydrogen_env->length();
LEnvironment* result = new LEnvironment(hydrogen_env->closure(),
ast_id,
hydrogen_env->parameter_count(),
@ -2063,14 +2060,7 @@ LInstruction* LChunkBuilder::DoSimulate(HSimulate* instr) {
env->Push(value);
}
}
if (FLAG_trace_environment) {
PrintF("Reconstructed environment ast_id=%d, instr_id=%d\n",
instr->ast_id(),
instr->id());
env->PrintToStd();
}
ASSERT(env->values()->length() == instr->environment_height());
ASSERT(env->length() == instr->environment_length());
// If there is an instruction pending deoptimization environment create a
// lazy bailout instruction to capture the environment.