From 25b450b2b7f7e54d6fd4a1a1cc40a47a7dad6569 Mon Sep 17 00:00:00 2001 From: "rafaelw@chromium.org" Date: Thu, 14 Nov 2013 21:45:01 +0000 Subject: [PATCH] Revert "Improvements in positions handling in optimizing compiler." (r17765) Original issue: https://codereview.chromium.org/49203002/ TBR=vegorov Review URL: https://codereview.chromium.org/63343003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@17768 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/hydrogen-instructions.h | 129 +-------------------------------- src/hydrogen-representation-changes.cc | 4 +- src/hydrogen.cc | 25 +------ 3 files changed, 9 insertions(+), 149 deletions(-) diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h index 1167212..9bd0b90 100644 --- a/src/hydrogen-instructions.h +++ b/src/hydrogen-instructions.h @@ -642,7 +642,6 @@ class HValue : public ZoneObject { virtual ~HValue() {} virtual int position() const { return RelocInfo::kNoPosition; } - virtual int operand_position(int index) const { return position(); } HBasicBlock* block() const { return block_; } void SetBlock(HBasicBlock* block); @@ -1106,102 +1105,6 @@ class HValue : public ZoneObject { } -// A helper class to represent per-operand position information attached to -// the HInstruction in the compact form. Uses tagging to distinguish between -// case when only instruction's position is available and case when operands' -// positions are also available. -// In the first case it contains intruction's position as a tagged value. -// In the second case it points to an array which contains instruction's -// position and operands' positions. -// TODO(vegorov): what we really want to track here is a combination of -// source position and a script id because cross script inlining can easily -// result in optimized functions composed of several scripts. -class HPositionInfo { - public: - explicit HPositionInfo(int pos) : data_(TagPosition(pos)) { } - - int position() const { - if (has_operand_positions()) { - return operand_positions()[kInstructionPosIndex]; - } - return UntagPosition(data_); - } - - void set_position(int pos) { - if (has_operand_positions()) { - operand_positions()[kInstructionPosIndex] = pos; - } else { - data_ = TagPosition(pos); - } - } - - void ensure_storage_for_operand_positions(Zone* zone, int operand_count) { - if (has_operand_positions()) { - return; - } - - const int length = kFirstOperandPosIndex + operand_count; - intptr_t* positions = - zone->NewArray(length); - for (int i = 0; i < length; i++) { - positions[i] = RelocInfo::kNoPosition; - } - - const int pos = position(); - data_ = reinterpret_cast(positions); - set_position(pos); - - ASSERT(has_operand_positions()); - } - - int operand_position(int idx) const { - if (!has_operand_positions()) { - return position(); - } - return static_cast(*operand_position_slot(idx)); - } - - void set_operand_position(int idx, int pos) { - *operand_position_slot(idx) = pos; - } - - private: - static const intptr_t kInstructionPosIndex = 0; - static const intptr_t kFirstOperandPosIndex = 1; - - intptr_t* operand_position_slot(int idx) const { - ASSERT(has_operand_positions()); - return &(operand_positions()[kFirstOperandPosIndex + idx]); - } - - bool has_operand_positions() const { - return !IsTaggedPosition(data_); - } - - intptr_t* operand_positions() const { - ASSERT(has_operand_positions()); - return reinterpret_cast(data_); - } - - static const intptr_t kPositionTag = 1; - static const intptr_t kPositionShift = 1; - static bool IsTaggedPosition(intptr_t val) { - return (val & kPositionTag) != 0; - } - static intptr_t UntagPosition(intptr_t val) { - ASSERT(IsTaggedPosition(val)); - return val >> kPositionShift; - } - static intptr_t TagPosition(intptr_t val) { - const intptr_t result = (val << kPositionShift) | kPositionTag; - ASSERT(UntagPosition(result) == val); - return result; - } - - intptr_t data_; -}; - - class HInstruction : public HValue { public: HInstruction* next() const { return next_; } @@ -1216,26 +1119,12 @@ class HInstruction : public HValue { void InsertAfter(HInstruction* previous); // The position is a write-once variable. - virtual int position() const V8_OVERRIDE { - return position_.position(); - } - bool has_position() const { - return position_.position() != RelocInfo::kNoPosition; - } + virtual int position() const V8_OVERRIDE { return position_; } + bool has_position() const { return position_ != RelocInfo::kNoPosition; } void set_position(int position) { ASSERT(!has_position()); ASSERT(position != RelocInfo::kNoPosition); - position_.set_position(position); - } - - virtual int operand_position(int index) const V8_OVERRIDE { - const int pos = position_.operand_position(index); - return (pos != RelocInfo::kNoPosition) ? pos : position(); - } - void set_operand_position(Zone* zone, int index, int pos) { - ASSERT(0 <= index && index < OperandCount()); - position_.ensure_storage_for_operand_positions(zone, OperandCount()); - position_.set_operand_position(index, pos); + position_ = position; } bool CanTruncateToInt32() const { return CheckFlag(kTruncatingToInt32); } @@ -1271,7 +1160,7 @@ class HInstruction : public HValue { HInstruction* next_; HInstruction* previous_; - HPositionInfo position_; + int position_; friend class HBasicBlock; }; @@ -3799,11 +3688,6 @@ class HBinaryOperation : public HTemplateInstruction<3> { return representation(); } - void SetOperandPositions(Zone* zone, int left_pos, int right_pos) { - set_operand_position(zone, 1, left_pos); - set_operand_position(zone, 2, right_pos); - } - DECLARE_ABSTRACT_INSTRUCTION(BinaryOperation) private: @@ -4237,11 +4121,6 @@ class HCompareNumericAndBranch : public HTemplateControlInstruction<2, 2> { } virtual void PrintDataTo(StringStream* stream) V8_OVERRIDE; - void SetOperandPositions(Zone* zone, int left_pos, int right_pos) { - set_operand_position(zone, 0, left_pos); - set_operand_position(zone, 1, right_pos); - } - DECLARE_CONCRETE_INSTRUCTION(CompareNumericAndBranch) private: diff --git a/src/hydrogen-representation-changes.cc b/src/hydrogen-representation-changes.cc index 07fc8be..d0c9b58 100644 --- a/src/hydrogen-representation-changes.cc +++ b/src/hydrogen-representation-changes.cc @@ -61,8 +61,8 @@ void HRepresentationChangesPhase::InsertRepresentationChangeForUse( if (new_value == NULL) { new_value = new(graph()->zone()) HChange( value, to, is_truncating_to_smi, is_truncating_to_int); - if (use_value->operand_position(use_index) != RelocInfo::kNoPosition) { - new_value->set_position(use_value->operand_position(use_index)); + if (use_value->position() != RelocInfo::kNoPosition) { + new_value->set_position(use_value->position()); } else { ASSERT(!FLAG_emit_opt_code_positions || !graph()->info()->IsOptimizing()); } diff --git a/src/hydrogen.cc b/src/hydrogen.cc index 72f276c..b47ec80 100644 --- a/src/hydrogen.cc +++ b/src/hydrogen.cc @@ -8627,14 +8627,10 @@ void HOptimizedGraphBuilder::VisitLogicalExpression(BinaryOperation* expr) { void HOptimizedGraphBuilder::VisitArithmeticExpression(BinaryOperation* expr) { CHECK_ALIVE(VisitForValue(expr->left())); CHECK_ALIVE(VisitForValue(expr->right())); - SetSourcePosition(expr->position()); + if (!FLAG_emit_opt_code_positions) SetSourcePosition(expr->position()); HValue* right = Pop(); HValue* left = Pop(); HInstruction* instr = BuildBinaryOperation(expr, left, right); - if (FLAG_emit_opt_code_positions && instr->IsBinaryOperation()) { - HBinaryOperation::cast(instr)->SetOperandPositions( - zone(), expr->left()->position(), expr->right()->position()); - } return ast_context()->ReturnInstruction(instr, expr->id()); } @@ -8643,7 +8639,7 @@ void HOptimizedGraphBuilder::HandleLiteralCompareTypeof(CompareOperation* expr, Expression* sub_expr, Handle check) { CHECK_ALIVE(VisitForTypeOf(sub_expr)); - SetSourcePosition(expr->position()); + if (!FLAG_emit_opt_code_positions) SetSourcePosition(expr->position()); HValue* value = Pop(); HTypeofIsAndBranch* instr = New(value, check); return ast_context()->ReturnControl(instr, expr->id()); @@ -8705,8 +8701,6 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { CHECK_ALIVE(VisitForValue(expr->left())); CHECK_ALIVE(VisitForValue(expr->right())); - if (FLAG_emit_opt_code_positions) SetSourcePosition(expr->position()); - HValue* right = Pop(); HValue* left = Pop(); Token::Value op = expr->op(); @@ -8785,10 +8779,6 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { AddCheckMap(right, map); HCompareObjectEqAndBranch* result = New(left, right); - if (FLAG_emit_opt_code_positions) { - result->set_operand_position(zone(), 0, expr->left()->position()); - result->set_operand_position(zone(), 1, expr->right()->position()); - } return ast_context()->ReturnControl(result, expr->id()); } else { BuildCheckHeapObject(left); @@ -8830,11 +8820,6 @@ void HOptimizedGraphBuilder::VisitCompareOperation(CompareOperation* expr) { HCompareNumericAndBranch* result = New(left, right, op); result->set_observed_input_representation(left_rep, right_rep); - if (FLAG_emit_opt_code_positions) { - result->SetOperandPositions(zone(), - expr->left()->position(), - expr->right()->position()); - } return ast_context()->ReturnControl(result, expr->id()); } } @@ -10151,8 +10136,7 @@ void HTracer::Trace(const char* name, HGraph* graph, LChunk* chunk) { Tag HIR_tag(this, "HIR"); for (HInstructionIterator it(current); !it.Done(); it.Advance()) { HInstruction* instruction = it.Current(); - int bci = FLAG_emit_opt_code_positions && instruction->has_position() ? - instruction->position() : 0; + int bci = 0; int uses = instruction->UseCount(); PrintIndent(); trace_.Add("%d %d ", bci, uses); @@ -10177,9 +10161,6 @@ void HTracer::Trace(const char* name, HGraph* graph, LChunk* chunk) { trace_.Add("%d ", LifetimePosition::FromInstructionIndex(i).Value()); linstr->PrintTo(&trace_); - trace_.Add(" [hir:"); - linstr->hydrogen_value()->PrintNameTo(&trace_); - trace_.Add("]"); trace_.Add(" <|@\n"); } } -- 2.7.4