Allow HPhis to have an invalid merge index.

All phis that do not represent local variables or values on the operand
stack are not allowed to carry a merge index, as the replay of the
HEnvironment during LChunkBuilder time might get out of sync due to
colliding indexes.

R=danno@chromium.org
BUG=v8:2815

Review URL: https://codereview.chromium.org/22494003

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@16135 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
parent f0cb71a3
......@@ -815,7 +815,11 @@ void LChunkBuilder::DoBasicBlock(HBasicBlock* block, HBasicBlock* next_block) {
HEnvironment* last_environment = pred->last_environment();
for (int i = 0; i < block->phis()->length(); ++i) {
HPhi* phi = block->phis()->at(i);
if (phi->merged_index() < last_environment->length()) {
// TODO(mstarzinger): The length check below should actually not
// be necessary, but some array stubs already rely on it. This
// should be investigated and fixed.
if (phi->HasMergedIndex() &&
phi->merged_index() < last_environment->length()) {
last_environment->SetValueAt(phi->merged_index(), phi);
}
}
......
......@@ -118,7 +118,9 @@ void HDeadCodeEliminationPhase::RemoveDeadInstructions() {
HPhi* phi = worklist.RemoveLast();
HBasicBlock* block = phi->block();
phi->DeleteAndReplaceWith(NULL);
block->RecordDeletedPhi(phi->merged_index());
if (phi->HasMergedIndex()) {
block->RecordDeletedPhi(phi->merged_index());
}
}
}
......
......@@ -110,14 +110,11 @@ HCapturedObject* HEscapeAnalysisPhase::NewStateCopy(
// Insert a newly created phi into the given block and fill all incoming
// edges with the given value. The merge index is chosen so that it is
// unique for this particular scalar replacement index.
// edges with the given value.
HPhi* HEscapeAnalysisPhase::NewPhiAndInsert(
HBasicBlock* block, HValue* incoming_value, int index) {
Zone* zone = graph()->zone();
HBasicBlock* pred = block->predecessors()->first();
int phi_index = pred->last_environment()->length() + cumulative_values_;
HPhi* phi = new(zone) HPhi(phi_index + index, zone);
HPhi* phi = new(zone) HPhi(HPhi::kInvalidMergedIndex, zone);
for (int i = 0; i < block->predecessors()->length(); i++) {
phi->AddInput(incoming_value);
}
......
......@@ -3042,7 +3042,7 @@ class HPhi: public HValue {
non_phi_uses_[i] = 0;
indirect_uses_[i] = 0;
}
ASSERT(merged_index >= 0);
ASSERT(merged_index >= 0 || merged_index == kInvalidMergedIndex);
SetFlag(kFlexibleRepresentation);
SetFlag(kAllowUndefinedAsNaN);
}
......@@ -3065,6 +3065,7 @@ class HPhi: public HValue {
bool HasRealUses();
bool IsReceiver() const { return merged_index_ == 0; }
bool HasMergedIndex() const { return merged_index_ != kInvalidMergedIndex; }
int merged_index() const { return merged_index_; }
......@@ -3127,6 +3128,9 @@ class HPhi: public HValue {
void SimplifyConstantInputs();
// Marker value representing an invalid merge index.
static const int kInvalidMergedIndex = -1;
protected:
virtual void DeleteFromGraph();
virtual void InternalSetOperandAt(int index, HValue* value) {
......
......@@ -117,6 +117,7 @@ void HOsrBuilder::FinishOsrValues() {
const ZoneList<HPhi*>* phis = osr_loop_entry_->phis();
for (int j = 0; j < phis->length(); j++) {
HPhi* phi = phis->at(j);
ASSERT(phi->HasMergedIndex());
osr_values_->at(phi->merged_index())->set_incoming_value(phi);
}
}
......
......@@ -148,6 +148,16 @@ void HBasicBlock::AddInstruction(HInstruction* instr) {
}
HPhi* HBasicBlock::AddNewPhi(int merged_index) {
if (graph()->IsInsideNoSideEffectsScope()) {
merged_index = HPhi::kInvalidMergedIndex;
}
HPhi* phi = new(zone()) HPhi(merged_index, zone());
AddPhi(phi);
return phi;
}
HSimulate* HBasicBlock::CreateSimulate(BailoutId ast_id,
RemovableSimulate removable) {
ASSERT(HasEnvironment());
......@@ -203,7 +213,7 @@ void HBasicBlock::Goto(HBasicBlock* block,
UpdateEnvironment(last_environment()->DiscardInlined(drop_extra));
}
if (add_simulate) AddSimulate(BailoutId::None());
if (add_simulate) AddNewSimulate(BailoutId::None());
HGoto* instr = new(zone()) HGoto(block);
Finish(instr);
}
......@@ -219,7 +229,7 @@ void HBasicBlock::AddLeaveInlined(HValue* return_value,
AddInstruction(new(zone()) HLeaveInlined());
UpdateEnvironment(last_environment()->DiscardInlined(drop_extra));
last_environment()->Push(return_value);
AddSimulate(BailoutId::None());
AddNewSimulate(BailoutId::None());
HGoto* instr = new(zone()) HGoto(target);
Finish(instr);
}
......@@ -904,8 +914,7 @@ HValue* HGraphBuilder::LoopBuilder::BeginBody(
HValue* terminating,
Token::Value token) {
HEnvironment* env = builder_->environment();
phi_ = new(zone()) HPhi(env->values()->length(), zone());
header_block_->AddPhi(phi_);
phi_ = header_block_->AddNewPhi(env->values()->length());
phi_->AddInput(initial);
env->Push(initial);
builder_->current_block()->GotoNoSimulate(header_block_);
......@@ -982,7 +991,7 @@ HGraph* HGraphBuilder::CreateGraph() {
HInstruction* HGraphBuilder::AddInstruction(HInstruction* instr) {
ASSERT(current_block() != NULL);
current_block()->AddInstruction(instr);
if (no_side_effects_scope_count_ > 0) {
if (graph()->IsInsideNoSideEffectsScope()) {
instr->SetFlag(HValue::kHasNoObservableSideEffects);
}
return instr;
......@@ -1006,8 +1015,8 @@ void HGraphBuilder::AddIncrementCounter(StatsCounter* counter,
void HGraphBuilder::AddSimulate(BailoutId id,
RemovableSimulate removable) {
ASSERT(current_block() != NULL);
ASSERT(no_side_effects_scope_count_ == 0);
current_block()->AddSimulate(id, removable);
ASSERT(!graph()->IsInsideNoSideEffectsScope());
current_block()->AddNewSimulate(id, removable);
}
......@@ -1037,7 +1046,7 @@ void HGraphBuilder::FinishExitWithHardDeoptimization(
const char* reason, HBasicBlock* continuation) {
PadEnvironmentForContinuation(current_block(), continuation);
Add<HDeoptimize>(reason, Deoptimizer::EAGER);
if (no_side_effects_scope_count_ > 0) {
if (graph()->IsInsideNoSideEffectsScope()) {
current_block()->GotoNoSimulate(continuation);
} else {
current_block()->Goto(continuation);
......@@ -2049,7 +2058,8 @@ HGraph::HGraph(CompilationInfo* info)
has_soft_deoptimize_(false),
depends_on_empty_array_proto_elements_(false),
type_change_checksum_(0),
maximum_environment_size_(0) {
maximum_environment_size_(0),
no_side_effects_scope_count_(0) {
if (info->IsStub()) {
HydrogenCodeStub* stub = info->code_stub();
CodeStubInterfaceDescriptor* descriptor =
......@@ -9326,20 +9336,19 @@ void HEnvironment::AddIncomingEdge(HBasicBlock* block, HEnvironment* other) {
// There is already a phi for the i'th value.
HPhi* phi = HPhi::cast(value);
// Assert index is correct and that we haven't missed an incoming edge.
ASSERT(phi->merged_index() == i);
ASSERT(phi->merged_index() == i || !phi->HasMergedIndex());
ASSERT(phi->OperandCount() == block->predecessors()->length());
phi->AddInput(other->values_[i]);
} else if (values_[i] != other->values_[i]) {
// There is a fresh value on the incoming edge, a phi is needed.
ASSERT(values_[i] != NULL && other->values_[i] != NULL);
HPhi* phi = new(zone()) HPhi(i, zone());
HPhi* phi = block->AddNewPhi(i);
HValue* old_value = values_[i];
for (int j = 0; j < block->predecessors()->length(); j++) {
phi->AddInput(old_value);
}
phi->AddInput(other->values_[i]);
this->values_[i] = phi;
block->AddPhi(phi);
}
}
}
......@@ -9400,10 +9409,9 @@ HEnvironment* HEnvironment::CopyWithoutHistory() const {
HEnvironment* HEnvironment::CopyAsLoopHeader(HBasicBlock* loop_header) const {
HEnvironment* new_env = Copy();
for (int i = 0; i < values_.length(); ++i) {
HPhi* phi = new(zone()) HPhi(i, zone());
HPhi* phi = loop_header->AddNewPhi(i);
phi->AddInput(values_[i]);
new_env->values_[i] = phi;
loop_header->AddPhi(phi);
}
new_env->ClearHistory();
return new_env;
......
......@@ -142,8 +142,9 @@ class HBasicBlock: public ZoneObject {
}
int PredecessorIndexOf(HBasicBlock* predecessor) const;
HSimulate* AddSimulate(BailoutId ast_id,
RemovableSimulate removable = FIXED_SIMULATE) {
HPhi* AddNewPhi(int merged_index);
HSimulate* AddNewSimulate(BailoutId ast_id,
RemovableSimulate removable = FIXED_SIMULATE) {
HSimulate* instr = CreateSimulate(ast_id, removable);
AddInstruction(instr);
return instr;
......@@ -453,6 +454,10 @@ class HGraph: public ZoneObject {
uint32_instructions_->Add(instr, zone());
}
void IncrementInNoSideEffectsScope() { no_side_effects_scope_count_++; }
void DecrementInNoSideEffectsScope() { no_side_effects_scope_count_--; }
bool IsInsideNoSideEffectsScope() { return no_side_effects_scope_count_ > 0; }
private:
HConstant* GetConstant(SetOncePointer<HConstant>* pointer,
int32_t integer_value);
......@@ -498,6 +503,7 @@ class HGraph: public ZoneObject {
bool depends_on_empty_array_proto_elements_;
int type_change_checksum_;
int maximum_environment_size_;
int no_side_effects_scope_count_;
DISALLOW_COPY_AND_ASSIGN(HGraph);
};
......@@ -970,8 +976,7 @@ class HGraphBuilder {
explicit HGraphBuilder(CompilationInfo* info)
: info_(info),
graph_(NULL),
current_block_(NULL),
no_side_effects_scope_count_(0) {}
current_block_(NULL) {}
virtual ~HGraphBuilder() {}
HBasicBlock* current_block() const { return current_block_; }
......@@ -1186,16 +1191,7 @@ class HGraphBuilder {
AddInstruction(NewUncasted<I>(p1, p2, p3, p4, p5, p6, p7, p8)));
}
void AddSimulate(BailoutId id,
RemovableSimulate removable = FIXED_SIMULATE);
void IncrementInNoSideEffectsScope() {
no_side_effects_scope_count_++;
}
void DecrementInNoSideEffectsScope() {
no_side_effects_scope_count_--;
}
void AddSimulate(BailoutId id, RemovableSimulate removable = FIXED_SIMULATE);
protected:
virtual bool BuildGraph() = 0;
......@@ -1442,20 +1438,6 @@ class HGraphBuilder {
bool finished_;
};
class NoObservableSideEffectsScope {
public:
explicit NoObservableSideEffectsScope(HGraphBuilder* builder) :
builder_(builder) {
builder_->IncrementInNoSideEffectsScope();
}
~NoObservableSideEffectsScope() {
builder_->DecrementInNoSideEffectsScope();
}
private:
HGraphBuilder* builder_;
};
HValue* BuildNewElementsCapacity(HValue* old_capacity);
void BuildNewSpaceArrayCheck(HValue* length,
......@@ -1577,7 +1559,6 @@ class HGraphBuilder {
CompilationInfo* info_;
HGraph* graph_;
HBasicBlock* current_block_;
int no_side_effects_scope_count_;
};
......@@ -2330,6 +2311,21 @@ class HTracer: public Malloced {
};
class NoObservableSideEffectsScope {
public:
explicit NoObservableSideEffectsScope(HGraphBuilder* builder) :
builder_(builder) {
builder_->graph()->IncrementInNoSideEffectsScope();
}
~NoObservableSideEffectsScope() {
builder_->graph()->DecrementInNoSideEffectsScope();
}
private:
HGraphBuilder* builder_;
};
} } // namespace v8::internal
#endif // V8_HYDROGEN_H_
......@@ -870,7 +870,11 @@ void LChunkBuilder::DoBasicBlock(HBasicBlock* block, HBasicBlock* next_block) {
HEnvironment* last_environment = pred->last_environment();
for (int i = 0; i < block->phis()->length(); ++i) {
HPhi* phi = block->phis()->at(i);
if (phi->merged_index() < last_environment->length()) {
// TODO(mstarzinger): The length check below should actually not
// be necessary, but some array stubs already rely on it. This
// should be investigated and fixed.
if (phi->HasMergedIndex() &&
phi->merged_index() < last_environment->length()) {
last_environment->SetValueAt(phi->merged_index(), phi);
}
}
......
......@@ -820,7 +820,11 @@ void LChunkBuilder::DoBasicBlock(HBasicBlock* block, HBasicBlock* next_block) {
HEnvironment* last_environment = pred->last_environment();
for (int i = 0; i < block->phis()->length(); ++i) {
HPhi* phi = block->phis()->at(i);
if (phi->merged_index() < last_environment->length()) {
// TODO(mstarzinger): The length check below should actually not
// be necessary, but some array stubs already rely on it. This
// should be investigated and fixed.
if (phi->HasMergedIndex() &&
phi->merged_index() < last_environment->length()) {
last_environment->SetValueAt(phi->merged_index(), phi);
}
}
......
......@@ -814,7 +814,11 @@ void LChunkBuilder::DoBasicBlock(HBasicBlock* block, HBasicBlock* next_block) {
HEnvironment* last_environment = pred->last_environment();
for (int i = 0; i < block->phis()->length(); ++i) {
HPhi* phi = block->phis()->at(i);
if (phi->merged_index() < last_environment->length()) {
// TODO(mstarzinger): The length check below should actually not
// be necessary, but some array stubs already rely on it. This
// should be investigated and fixed.
if (phi->HasMergedIndex() &&
phi->merged_index() < last_environment->length()) {
last_environment->SetValueAt(phi->merged_index(), phi);
}
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment