Skip to content

Commit

Permalink
Replace the recursion in PropagateMinusZeroChecks() with a loop and a…
Browse files Browse the repository at this point in the history 
… worklist.

Also refactor the related code in preparation for fixing the
range analysis.

BUG=v8:3204
LOG=y
R=svenpanne@chromium.org

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

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@19737 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
  • Loading branch information
bmeurer@chromium.org committed Mar 10, 2014
1 parent dc0c208 commit 3ccb838
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 158 deletions.
93 changes: 0 additions & 93 deletions src/hydrogen-instructions.cc
View file
Expand Up @@ -3703,99 +3703,6 @@ void HAllocate::PrintDataTo(StringStream* stream) {
}


HValue* HUnaryMathOperation::EnsureAndPropagateNotMinusZero(
BitVector* visited) {
visited->Add(id());
if (representation().IsSmiOrInteger32() &&
!value()->representation().Equals(representation())) {
if (value()->range() == NULL || value()->range()->CanBeMinusZero()) {
SetFlag(kBailoutOnMinusZero);
}
}
if (RequiredInputRepresentation(0).IsSmiOrInteger32() &&
representation().Equals(RequiredInputRepresentation(0))) {
return value();
}
return NULL;
}


HValue* HChange::EnsureAndPropagateNotMinusZero(BitVector* visited) {
visited->Add(id());
if (from().IsSmiOrInteger32()) return NULL;
if (CanTruncateToInt32()) return NULL;
if (value()->range() == NULL || value()->range()->CanBeMinusZero()) {
SetFlag(kBailoutOnMinusZero);
}
ASSERT(!from().IsSmiOrInteger32() || !to().IsSmiOrInteger32());
return NULL;
}


HValue* HForceRepresentation::EnsureAndPropagateNotMinusZero(
BitVector* visited) {
visited->Add(id());
return value();
}


HValue* HMod::EnsureAndPropagateNotMinusZero(BitVector* visited) {
visited->Add(id());
if (range() == NULL || range()->CanBeMinusZero()) {
SetFlag(kBailoutOnMinusZero);
return left();
}
return NULL;
}


HValue* HDiv::EnsureAndPropagateNotMinusZero(BitVector* visited) {
visited->Add(id());
if (range() == NULL || range()->CanBeMinusZero()) {
SetFlag(kBailoutOnMinusZero);
}
return NULL;
}


HValue* HMathFloorOfDiv::EnsureAndPropagateNotMinusZero(BitVector* visited) {
visited->Add(id());
SetFlag(kBailoutOnMinusZero);
return NULL;
}


HValue* HMul::EnsureAndPropagateNotMinusZero(BitVector* visited) {
visited->Add(id());
if (range() == NULL || range()->CanBeMinusZero()) {
SetFlag(kBailoutOnMinusZero);
}
return NULL;
}


HValue* HSub::EnsureAndPropagateNotMinusZero(BitVector* visited) {
visited->Add(id());
// Propagate to the left argument. If the left argument cannot be -0, then
// the result of the add operation cannot be either.
if (range() == NULL || range()->CanBeMinusZero()) {
return left();
}
return NULL;
}


HValue* HAdd::EnsureAndPropagateNotMinusZero(BitVector* visited) {
visited->Add(id());
// Propagate to the left argument. If the left argument cannot be -0, then
// the result of the sub operation cannot be either.
if (range() == NULL || range()->CanBeMinusZero()) {
return left();
}
return NULL;
}


bool HStoreKeyed::NeedsCanonicalization() {
// If value is an integer or smi or comes from the result of a keyed load or
// constant then it is either be a non-hole value or in the case of a constant
Expand Down
41 changes: 0 additions & 41 deletions src/hydrogen-instructions.h
View file
Expand Up @@ -739,21 +739,6 @@ class HValue : public ZoneObject {
return representation_.IsHeapObject() || type_.IsHeapObject();
}

// An operation needs to override this function iff:
// 1) it can produce an int32 output.
// 2) the true value of its output can potentially be minus zero.
// The implementation must set a flag so that it bails out in the case where
// it would otherwise output what should be a minus zero as an int32 zero.
// If the operation also exists in a form that takes int32 and outputs int32
// then the operation should return its input value so that we can propagate
// back. There are three operations that need to propagate back to more than
// one input. They are phi and binary div and mul. They always return NULL
// and expect the caller to take care of things.
virtual HValue* EnsureAndPropagateNotMinusZero(BitVector* visited) {
visited->Add(id());
return NULL;
}

// There are HInstructions that do not really change a value, they
// only add pieces of information to it (like bounds checks, map checks,
// smi checks...).
Expand Down Expand Up @@ -1719,9 +1704,6 @@ class HForceRepresentation V8_FINAL : public HTemplateInstruction<1> {

HValue* value() { return OperandAt(0); }

virtual HValue* EnsureAndPropagateNotMinusZero(
BitVector* visited) V8_OVERRIDE;

virtual Representation RequiredInputRepresentation(int index) V8_OVERRIDE {
return representation(); // Same as the output representation.
}
Expand Down Expand Up @@ -1767,8 +1749,6 @@ class HChange V8_FINAL : public HUnaryOperation {
return CheckUsesForFlag(kAllowUndefinedAsNaN);
}

virtual HValue* EnsureAndPropagateNotMinusZero(
BitVector* visited) V8_OVERRIDE;
virtual HType CalculateInferredType() V8_OVERRIDE;
virtual HValue* Canonicalize() V8_OVERRIDE;

Expand Down Expand Up @@ -2631,9 +2611,6 @@ class HUnaryMathOperation V8_FINAL : public HTemplateInstruction<2> {

virtual void PrintDataTo(StringStream* stream) V8_OVERRIDE;

virtual HValue* EnsureAndPropagateNotMinusZero(
BitVector* visited) V8_OVERRIDE;

virtual Representation RequiredInputRepresentation(int index) V8_OVERRIDE {
if (index == 0) {
return Representation::Tagged();
Expand Down Expand Up @@ -4111,9 +4088,6 @@ class HMathFloorOfDiv V8_FINAL : public HBinaryOperation {
HValue*,
HValue*);

virtual HValue* EnsureAndPropagateNotMinusZero(
BitVector* visited) V8_OVERRIDE;

DECLARE_CONCRETE_INSTRUCTION(MathFloorOfDiv)

protected:
Expand Down Expand Up @@ -4714,9 +4688,6 @@ class HAdd V8_FINAL : public HArithmeticBinaryOperation {
return !representation().IsTagged() && !representation().IsExternal();
}

virtual HValue* EnsureAndPropagateNotMinusZero(
BitVector* visited) V8_OVERRIDE;

virtual HValue* Canonicalize() V8_OVERRIDE;

virtual bool TryDecompose(DecompositionResult* decomposition) V8_OVERRIDE {
Expand Down Expand Up @@ -4773,9 +4744,6 @@ class HSub V8_FINAL : public HArithmeticBinaryOperation {
HValue* left,
HValue* right);

virtual HValue* EnsureAndPropagateNotMinusZero(
BitVector* visited) V8_OVERRIDE;

virtual HValue* Canonicalize() V8_OVERRIDE;

virtual bool TryDecompose(DecompositionResult* decomposition) V8_OVERRIDE {
Expand Down Expand Up @@ -4822,9 +4790,6 @@ class HMul V8_FINAL : public HArithmeticBinaryOperation {
return mul;
}

virtual HValue* EnsureAndPropagateNotMinusZero(
BitVector* visited) V8_OVERRIDE;

virtual HValue* Canonicalize() V8_OVERRIDE;

// Only commutative if it is certain that not two objects are multiplicated.
Expand Down Expand Up @@ -4862,9 +4827,6 @@ class HMod V8_FINAL : public HArithmeticBinaryOperation {
HValue* left,
HValue* right);

virtual HValue* EnsureAndPropagateNotMinusZero(
BitVector* visited) V8_OVERRIDE;

virtual HValue* Canonicalize() V8_OVERRIDE;

virtual void UpdateRepresentation(Representation new_rep,
Expand Down Expand Up @@ -4898,9 +4860,6 @@ class HDiv V8_FINAL : public HArithmeticBinaryOperation {
HValue* left,
HValue* right);

virtual HValue* EnsureAndPropagateNotMinusZero(
BitVector* visited) V8_OVERRIDE;

virtual HValue* Canonicalize() V8_OVERRIDE;

virtual void UpdateRepresentation(Representation new_rep,
Expand Down
89 changes: 67 additions & 22 deletions src/hydrogen-minus-zero.cc
View file
Expand Up @@ -45,17 +45,13 @@ void HComputeMinusZeroChecksPhase::Run() {
ASSERT(change->to().IsTagged() ||
change->to().IsDouble() ||
change->to().IsSmiOrInteger32());
ASSERT(visited_.IsEmpty());
PropagateMinusZeroChecks(change->value());
visited_.Clear();
}
} else if (current->IsCompareMinusZeroAndBranch()) {
HCompareMinusZeroAndBranch* check =
HCompareMinusZeroAndBranch::cast(current);
if (check->value()->representation().IsSmiOrInteger32()) {
ASSERT(visited_.IsEmpty());
PropagateMinusZeroChecks(check->value());
visited_.Clear();
}
}
}
Expand All @@ -64,28 +60,77 @@ void HComputeMinusZeroChecksPhase::Run() {


void HComputeMinusZeroChecksPhase::PropagateMinusZeroChecks(HValue* value) {
for (HValue* current = value;
current != NULL && !visited_.Contains(current->id());
current = current->EnsureAndPropagateNotMinusZero(&visited_)) {
// For phis, we must propagate the check to all of its inputs.
if (current->IsPhi()) {
visited_.Add(current->id());
HPhi* phi = HPhi::cast(current);
ASSERT(worklist_.is_empty());
ASSERT(in_worklist_.IsEmpty());

AddToWorklist(value);
while (!worklist_.is_empty()) {
value = worklist_.RemoveLast();

if (value->IsPhi()) {
// For phis, we must propagate the check to all of its inputs.
HPhi* phi = HPhi::cast(value);
for (int i = 0; i < phi->OperandCount(); ++i) {
PropagateMinusZeroChecks(phi->OperandAt(i));
AddToWorklist(phi->OperandAt(i));
}
break;
}

// For multiplication, division, and Math.min/max(), we must propagate
// to the left and the right side.
if (current->IsMul() || current->IsDiv() || current->IsMathMinMax()) {
HBinaryOperation* operation = HBinaryOperation::cast(current);
operation->EnsureAndPropagateNotMinusZero(&visited_);
PropagateMinusZeroChecks(operation->left());
PropagateMinusZeroChecks(operation->right());
} else if (value->IsUnaryMathOperation()) {
HUnaryMathOperation* instr = HUnaryMathOperation::cast(value);
if (instr->representation().IsSmiOrInteger32() &&
!instr->value()->representation().Equals(instr->representation())) {
if (instr->value()->range() == NULL ||
instr->value()->range()->CanBeMinusZero()) {
instr->SetFlag(HValue::kBailoutOnMinusZero);
}
}
if (instr->RequiredInputRepresentation(0).IsSmiOrInteger32() &&
instr->representation().Equals(
instr->RequiredInputRepresentation(0))) {
AddToWorklist(instr->value());
}
} else if (value->IsChange()) {
HChange* instr = HChange::cast(value);
if (!instr->from().IsSmiOrInteger32() &&
!instr->CanTruncateToInt32() &&
(instr->value()->range() == NULL ||
instr->value()->range()->CanBeMinusZero())) {
instr->SetFlag(HValue::kBailoutOnMinusZero);
}
} else if (value->IsForceRepresentation()) {
HForceRepresentation* instr = HForceRepresentation::cast(value);
AddToWorklist(instr->value());
} else if (value->IsMod()) {
HMod* instr = HMod::cast(value);
if (instr->range() == NULL || instr->range()->CanBeMinusZero()) {
instr->SetFlag(HValue::kBailoutOnMinusZero);
AddToWorklist(instr->left());
}
} else if (value->IsDiv() || value->IsMul()) {
HBinaryOperation* instr = HBinaryOperation::cast(value);
if (instr->range() == NULL || instr->range()->CanBeMinusZero()) {
instr->SetFlag(HValue::kBailoutOnMinusZero);
}
AddToWorklist(instr->right());
AddToWorklist(instr->left());
} else if (value->IsMathFloorOfDiv()) {
HMathFloorOfDiv* instr = HMathFloorOfDiv::cast(value);
instr->SetFlag(HValue::kBailoutOnMinusZero);
} else if (value->IsAdd() || value->IsSub()) {
HBinaryOperation* instr = HBinaryOperation::cast(value);
if (instr->range() == NULL || instr->range()->CanBeMinusZero()) {
// Propagate to the left argument. If the left argument cannot be -0,
// then the result of the add/sub operation cannot be either.
AddToWorklist(instr->left());
}
} else if (value->IsMathMinMax()) {
HMathMinMax* instr = HMathMinMax::cast(value);
AddToWorklist(instr->right());
AddToWorklist(instr->left());
}
}

in_worklist_.Clear();
ASSERT(in_worklist_.IsEmpty());
ASSERT(worklist_.is_empty());
}

} } // namespace v8::internal
12 changes: 10 additions & 2 deletions src/hydrogen-minus-zero.h
View file
Expand Up @@ -38,14 +38,22 @@ class HComputeMinusZeroChecksPhase : public HPhase {
public:
explicit HComputeMinusZeroChecksPhase(HGraph* graph)
: HPhase("H_Compute minus zero checks", graph),
visited_(graph->GetMaximumValueID(), zone()) { }
in_worklist_(graph->GetMaximumValueID(), zone()),
worklist_(32, zone()) {}

void Run();

private:
void AddToWorklist(HValue* value) {
if (value->CheckFlag(HValue::kBailoutOnMinusZero)) return;
if (in_worklist_.Contains(value->id())) return;
in_worklist_.Add(value->id());
worklist_.Add(value, zone());
}
void PropagateMinusZeroChecks(HValue* value);

BitVector visited_;
BitVector in_worklist_;
ZoneList<HValue*> worklist_;

DISALLOW_COPY_AND_ASSIGN(HComputeMinusZeroChecksPhase);
};
Expand Down

0 comments on commit 3ccb838

Please sign in to comment.