Commit e9a750b5 authored by Michael Starzinger's avatar Michael Starzinger Committed by Commit Bot

Revert "[turbofan] Handle comparison operations in early lowering."

This reverts commit f967d3e9.

Reason for revert: Tanks Mandreel again. Needs investigation.

Original change's description:
> [turbofan] Handle comparison operations in early lowering.
> 
> This handles comparison operations (equality and relational) having
> number feedback during the early type-hint lowering (i.e. during graph
> construction).
> 
> R=​bmeurer@chromium.org
> 
> Change-Id: I97afd6c0d78a790ce38b731f2532ca18d812a32c
> Reviewed-on: https://chromium-review.googlesource.com/444766
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#43315}

TBR=mstarzinger@chromium.org,bmeurer@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Change-Id: Iec335827fe841ac6f1bd45ce095d0a741b2ff5b5
Reviewed-on: https://chromium-review.googlesource.com/445177Reviewed-by: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#43326}
parent 3d157f7f
......@@ -1517,13 +1517,12 @@ Node* BytecodeGraphBuilder::TryBuildSimplifiedBinaryOp(const Operator* op,
Node* effect = environment()->GetEffectDependency();
Node* control = environment()->GetControlDependency();
JSTypeHintLowering type_hint_lowering(jsgraph(), feedback_vector());
EarlyReduction early_reduction = type_hint_lowering.ReduceBinaryOperation(
Reduction early_reduction = type_hint_lowering.ReduceBinaryOperation(
op, left, right, effect, control, slot);
if (early_reduction.has_reduction()) {
Node* node = early_reduction.value();
if (early_reduction.has_effect()) {
DCHECK_GT(early_reduction.effect()->op()->EffectOutputCount(), 0);
environment()->UpdateEffectDependency(early_reduction.effect());
if (early_reduction.Changed()) {
Node* node = early_reduction.replacement();
if (node->op()->EffectOutputCount() > 0) {
environment()->UpdateEffectDependency(node);
}
return node;
}
......@@ -1746,25 +1745,12 @@ void BytecodeGraphBuilder::VisitGetSuperConstructor() {
Environment::kAttachFrameState);
}
void BytecodeGraphBuilder::BuildCompareOp(const Operator* op) {
void BytecodeGraphBuilder::BuildCompareOp(const Operator* js_op) {
PrepareEagerCheckpoint();
Node* left =
environment()->LookupRegister(bytecode_iterator().GetRegisterOperand(0));
Node* right = environment()->LookupAccumulator();
Node* node = nullptr;
int slot_index = bytecode_iterator().GetIndexOperand(1);
if (slot_index != 0) {
FeedbackSlot slot = feedback_vector()->ToSlot(slot_index);
if (Node* simplified = TryBuildSimplifiedBinaryOp(op, left, right, slot)) {
node = simplified;
} else {
node = NewNode(op, left, right);
}
} else {
node = NewNode(op, left, right);
}
Node* node = NewNode(js_op, left, right);
environment()->BindAccumulator(node, Environment::kAttachFrameState);
}
......@@ -1796,21 +1782,12 @@ void BytecodeGraphBuilder::VisitTestGreaterThanOrEqual() {
BuildCompareOp(javascript()->GreaterThanOrEqual(GetCompareOperationHint()));
}
void BytecodeGraphBuilder::BuildTestingOp(const Operator* op) {
PrepareEagerCheckpoint();
Node* left =
environment()->LookupRegister(bytecode_iterator().GetRegisterOperand(0));
Node* right = environment()->LookupAccumulator();
Node* node = NewNode(op, left, right);
environment()->BindAccumulator(node, Environment::kAttachFrameState);
}
void BytecodeGraphBuilder::VisitTestIn() {
BuildTestingOp(javascript()->HasProperty());
BuildCompareOp(javascript()->HasProperty());
}
void BytecodeGraphBuilder::VisitTestInstanceOf() {
BuildTestingOp(javascript()->InstanceOf());
BuildCompareOp(javascript()->InstanceOf());
}
void BytecodeGraphBuilder::VisitTestUndetectable() {
......
......@@ -155,7 +155,6 @@ class BytecodeGraphBuilder {
void BuildBinaryOp(const Operator* op);
void BuildBinaryOpWithImmediate(const Operator* op);
void BuildCompareOp(const Operator* op);
void BuildTestingOp(const Operator* op);
void BuildDelete(LanguageMode language_mode);
void BuildCastOperator(const Operator* op);
void BuildForInPrepare();
......
......@@ -33,12 +33,6 @@ class JSSpeculativeBinopBuilder final {
return nexus.GetBinaryOperationFeedback();
}
CompareOperationHint GetCompareOperationHint() {
DCHECK_EQ(FeedbackSlotKind::kCompareOp, feedback_vector()->GetKind(slot_));
CompareICNexus nexus(feedback_vector(), slot_);
return nexus.GetCompareOperationFeedback();
}
bool GetBinaryNumberOperationHint(NumberOperationHint* hint) {
switch (GetBinaryOperationHint()) {
case BinaryOperationHint::kSignedSmall:
......@@ -58,27 +52,6 @@ class JSSpeculativeBinopBuilder final {
return false;
}
bool GetCompareNumberOperationHint(NumberOperationHint* hint) {
switch (GetCompareOperationHint()) {
case CompareOperationHint::kSignedSmall:
*hint = NumberOperationHint::kSignedSmall;
return true;
case CompareOperationHint::kNumber:
*hint = NumberOperationHint::kNumber;
return true;
case CompareOperationHint::kNumberOrOddball:
*hint = NumberOperationHint::kNumberOrOddball;
return true;
case CompareOperationHint::kAny:
case CompareOperationHint::kNone:
case CompareOperationHint::kString:
case CompareOperationHint::kReceiver:
case CompareOperationHint::kInternalizedString:
break;
}
return false;
}
const Operator* SpeculativeNumberOp(NumberOperationHint hint) {
switch (op_->opcode()) {
case IrOpcode::kJSAdd:
......@@ -110,26 +83,7 @@ class JSSpeculativeBinopBuilder final {
return nullptr;
}
const Operator* SpeculativeCompareOp(NumberOperationHint hint) {
switch (op_->opcode()) {
case IrOpcode::kJSLessThan:
return simplified()->SpeculativeNumberLessThan(hint);
case IrOpcode::kJSGreaterThan:
std::swap(left_, right_); // a > b => b < a
return simplified()->SpeculativeNumberLessThan(hint);
case IrOpcode::kJSLessThanOrEqual:
return simplified()->SpeculativeNumberLessThanOrEqual(hint);
case IrOpcode::kJSGreaterThanOrEqual:
std::swap(left_, right_); // a >= b => b <= a
return simplified()->SpeculativeNumberLessThanOrEqual(hint);
default:
break;
}
UNREACHABLE();
return nullptr;
}
Node* BuildSpeculativeOperation(const Operator* op) {
Node* BuildSpeculativeOperator(const Operator* op) {
DCHECK_EQ(2, op->ValueInputCount());
DCHECK_EQ(1, op->EffectInputCount());
DCHECK_EQ(1, op->ControlInputCount());
......@@ -137,46 +91,9 @@ class JSSpeculativeBinopBuilder final {
DCHECK_EQ(false, OperatorProperties::HasContextInput(op));
DCHECK_EQ(1, op->EffectOutputCount());
DCHECK_EQ(0, op->ControlOutputCount());
Node* node = graph()->NewNode(op, left_, right_, effect_, control_);
effect_ = node; // Update the effect dependency.
return node;
}
Node* BuildInvert(Node* input) {
return graph()->NewNode(simplified()->BooleanNot(), input);
}
Node* TryBuildNumberBinop() {
NumberOperationHint hint;
if (GetBinaryNumberOperationHint(&hint)) {
const Operator* op = SpeculativeNumberOp(hint);
Node* node = BuildSpeculativeOperation(op);
return node;
}
return nullptr;
}
Node* TryBuildNumberEqual(bool invert) {
NumberOperationHint hint;
if (GetCompareNumberOperationHint(&hint)) {
const Operator* op = simplified()->SpeculativeNumberEqual(hint);
Node* compare = BuildSpeculativeOperation(op);
return invert ? BuildInvert(compare) : compare;
}
return nullptr;
}
Node* TryBuildNumberCompare() {
NumberOperationHint hint;
if (GetCompareNumberOperationHint(&hint)) {
const Operator* op = SpeculativeCompareOp(hint);
Node* node = BuildSpeculativeOperation(op);
return node;
}
return nullptr;
return graph()->NewNode(op, left_, right_, effect_, control_);
}
Node* effect() const { return effect_; }
JSGraph* jsgraph() const { return lowering_->jsgraph(); }
Graph* graph() const { return jsgraph()->graph(); }
JSOperatorBuilder* javascript() { return jsgraph()->javascript(); }
......@@ -200,36 +117,11 @@ JSTypeHintLowering::JSTypeHintLowering(JSGraph* jsgraph,
Handle<FeedbackVector> feedback_vector)
: jsgraph_(jsgraph), feedback_vector_(feedback_vector) {}
EarlyReduction JSTypeHintLowering::ReduceBinaryOperation(
const Operator* op, Node* left, Node* right, Node* effect, Node* control,
FeedbackSlot slot) {
Reduction JSTypeHintLowering::ReduceBinaryOperation(const Operator* op,
Node* left, Node* right,
Node* effect, Node* control,
FeedbackSlot slot) {
switch (op->opcode()) {
case IrOpcode::kJSEqual:
case IrOpcode::kJSStrictEqual: {
JSSpeculativeBinopBuilder b(this, op, left, right, effect, control, slot);
if (Node* node = b.TryBuildNumberEqual(false)) {
return EarlyReduction(node, b.effect());
}
break;
}
case IrOpcode::kJSNotEqual:
case IrOpcode::kJSStrictNotEqual: {
JSSpeculativeBinopBuilder b(this, op, left, right, effect, control, slot);
if (Node* node = b.TryBuildNumberEqual(true)) {
return EarlyReduction(node, b.effect());
}
break;
}
case IrOpcode::kJSLessThan:
case IrOpcode::kJSGreaterThan:
case IrOpcode::kJSLessThanOrEqual:
case IrOpcode::kJSGreaterThanOrEqual: {
JSSpeculativeBinopBuilder b(this, op, left, right, effect, control, slot);
if (Node* node = b.TryBuildNumberCompare()) {
return EarlyReduction(node, b.effect());
}
break;
}
case IrOpcode::kJSBitwiseOr:
case IrOpcode::kJSBitwiseXor:
case IrOpcode::kJSBitwiseAnd:
......@@ -242,8 +134,10 @@ EarlyReduction JSTypeHintLowering::ReduceBinaryOperation(
case IrOpcode::kJSDivide:
case IrOpcode::kJSModulus: {
JSSpeculativeBinopBuilder b(this, op, left, right, effect, control, slot);
if (Node* node = b.TryBuildNumberBinop()) {
return EarlyReduction(node, b.effect());
NumberOperationHint hint;
if (b.GetBinaryNumberOperationHint(&hint)) {
Node* node = b.BuildSpeculativeOperator(b.SpeculativeNumberOp(hint));
return Reduction(node);
}
break;
}
......@@ -251,7 +145,7 @@ EarlyReduction JSTypeHintLowering::ReduceBinaryOperation(
UNREACHABLE();
break;
}
return EarlyReduction();
return Reduction();
}
} // namespace compiler
......
......@@ -5,20 +5,14 @@
#ifndef V8_COMPILER_JS_TYPE_HINT_LOWERING_H_
#define V8_COMPILER_JS_TYPE_HINT_LOWERING_H_
#include "src/compiler/graph-reducer.h"
#include "src/handles.h"
namespace v8 {
namespace internal {
// Forward declarations.
class FeedbackSlot;
namespace compiler {
// Forward declarations.
class EarlyReduction;
class Node;
class Operator;
class JSGraph;
// The type-hint lowering consumes feedback about data operations (i.e. unary
......@@ -34,11 +28,10 @@ class JSTypeHintLowering {
public:
JSTypeHintLowering(JSGraph* jsgraph, Handle<FeedbackVector> feedback_vector);
// Potential reduction of binary (arithmetic, logical, shift, equalities and
// relational comparison) operations.
EarlyReduction ReduceBinaryOperation(const Operator*, Node* left, Node* right,
Node* effect, Node* control,
FeedbackSlot slot);
// Potential reduction of binary (arithmetic, logical and shift) operations.
Reduction ReduceBinaryOperation(const Operator* op, Node* left, Node* right,
Node* effect, Node* control,
FeedbackSlot slot);
private:
friend class JSSpeculativeBinopBuilder;
......@@ -54,25 +47,6 @@ class JSTypeHintLowering {
DISALLOW_COPY_AND_ASSIGN(JSTypeHintLowering);
};
// The result of a successful early reduction is a {value} node and an optional
// {effect} node (which might be different from the value). In case reduction
// failed, none of the above nodes are provided.
class EarlyReduction final {
public:
EarlyReduction() : value_(nullptr), effect_(nullptr) {}
EarlyReduction(Node* value, Node* effect) : value_(value), effect_(effect) {}
Node* value() const { return value_; }
Node* effect() const { return effect_; }
bool has_reduction() const { return value_ != nullptr; }
bool has_effect() const { return effect_ != nullptr; }
private:
Node* value_;
Node* effect_;
};
} // namespace compiler
} // namespace internal
} // namespace v8
......
......@@ -30,6 +30,30 @@ class JSBinopReduction final {
JSBinopReduction(JSTypedLowering* lowering, Node* node)
: lowering_(lowering), node_(node) {}
bool GetCompareNumberOperationHint(NumberOperationHint* hint) {
if (lowering_->flags() & JSTypedLowering::kDeoptimizationEnabled) {
DCHECK_EQ(1, node_->op()->EffectOutputCount());
switch (CompareOperationHintOf(node_->op())) {
case CompareOperationHint::kSignedSmall:
*hint = NumberOperationHint::kSignedSmall;
return true;
case CompareOperationHint::kNumber:
*hint = NumberOperationHint::kNumber;
return true;
case CompareOperationHint::kNumberOrOddball:
*hint = NumberOperationHint::kNumberOrOddball;
return true;
case CompareOperationHint::kAny:
case CompareOperationHint::kNone:
case CompareOperationHint::kString:
case CompareOperationHint::kReceiver:
case CompareOperationHint::kInternalizedString:
break;
}
}
return false;
}
bool IsInternalizedStringCompareOperation() {
if (lowering_->flags() & JSTypedLowering::kDeoptimizationEnabled) {
DCHECK_EQ(1, node_->op()->EffectOutputCount());
......@@ -229,10 +253,69 @@ class JSBinopReduction final {
return lowering_->Changed(node_);
}
Reduction ChangeToSpeculativeOperator(const Operator* op, bool invert,
Type* upper_bound) {
DCHECK_EQ(1, op->EffectInputCount());
DCHECK_EQ(1, op->EffectOutputCount());
DCHECK_EQ(false, OperatorProperties::HasContextInput(op));
DCHECK_EQ(1, op->ControlInputCount());
DCHECK_EQ(0, op->ControlOutputCount());
DCHECK_EQ(0, OperatorProperties::GetFrameStateInputCount(op));
DCHECK_EQ(2, op->ValueInputCount());
DCHECK_EQ(1, node_->op()->EffectInputCount());
DCHECK_EQ(1, node_->op()->EffectOutputCount());
DCHECK_EQ(1, node_->op()->ControlInputCount());
DCHECK_EQ(2, node_->op()->ValueInputCount());
// Reconnect the control output to bypass the IfSuccess node and
// possibly disconnect from the IfException node.
for (Edge edge : node_->use_edges()) {
Node* const user = edge.from();
DCHECK(!user->IsDead());
if (NodeProperties::IsControlEdge(edge)) {
if (user->opcode() == IrOpcode::kIfSuccess) {
user->ReplaceUses(NodeProperties::GetControlInput(node_));
user->Kill();
} else {
DCHECK_EQ(user->opcode(), IrOpcode::kIfException);
edge.UpdateTo(jsgraph()->Dead());
}
}
}
// Remove the frame state and the context.
if (OperatorProperties::HasFrameStateInput(node_->op())) {
node_->RemoveInput(NodeProperties::FirstFrameStateIndex(node_));
}
node_->RemoveInput(NodeProperties::FirstContextIndex(node_));
NodeProperties::ChangeOp(node_, op);
// Update the type to number.
Type* node_type = NodeProperties::GetType(node_);
NodeProperties::SetType(node_,
Type::Intersect(node_type, upper_bound, zone()));
if (invert) {
// Insert an boolean not to invert the value.
Node* value = graph()->NewNode(simplified()->BooleanNot(), node_);
node_->ReplaceUses(value);
// Note: ReplaceUses() smashes all uses, so smash it back here.
value->ReplaceInput(0, node_);
return lowering_->Replace(value);
}
return lowering_->Changed(node_);
}
Reduction ChangeToPureOperator(const Operator* op, Type* type) {
return ChangeToPureOperator(op, false, type);
}
Reduction ChangeToSpeculativeOperator(const Operator* op, Type* type) {
return ChangeToSpeculativeOperator(op, false, type);
}
const Operator* NumberOp() {
switch (node_->opcode()) {
case IrOpcode::kJSAdd:
......@@ -266,10 +349,6 @@ class JSBinopReduction final {
const Operator* NumberOpFromSpeculativeNumberOp() {
switch (node_->opcode()) {
case IrOpcode::kSpeculativeNumberLessThan:
return simplified()->NumberLessThan();
case IrOpcode::kSpeculativeNumberLessThanOrEqual:
return simplified()->NumberLessThanOrEqual();
case IrOpcode::kSpeculativeNumberAdd:
return simplified()->NumberAdd();
case IrOpcode::kSpeculativeNumberSubtract:
......@@ -691,15 +770,6 @@ Reduction JSTypedLowering::ReduceCreateConsString(Node* node) {
return Changed(node);
}
Reduction JSTypedLowering::ReduceSpeculativeNumberComparison(Node* node) {
JSBinopReduction r(this, node);
if (r.BothInputsAre(Type::Signed32()) ||
r.BothInputsAre(Type::Unsigned32())) {
return r.ChangeToPureOperator(r.NumberOpFromSpeculativeNumberOp());
}
return Changed(node);
}
Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
JSBinopReduction r(this, node);
if (r.BothInputsAre(Type::String())) {
......@@ -727,12 +797,16 @@ Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
return Changed(node);
}
NumberOperationHint hint;
const Operator* less_than;
const Operator* less_than_or_equal;
if (r.BothInputsAre(Type::Signed32()) ||
r.BothInputsAre(Type::Unsigned32())) {
less_than = simplified()->NumberLessThan();
less_than_or_equal = simplified()->NumberLessThanOrEqual();
} else if (r.GetCompareNumberOperationHint(&hint)) {
less_than = simplified()->SpeculativeNumberLessThan(hint);
less_than_or_equal = simplified()->SpeculativeNumberLessThanOrEqual(hint);
} else if (r.OneInputCannotBe(Type::StringOrReceiver()) &&
(r.BothInputsAre(Type::PlainPrimitive()) ||
!(flags() & kDeoptimizationEnabled))) {
......@@ -765,19 +839,11 @@ Reduction JSTypedLowering::ReduceJSComparison(Node* node) {
default:
return NoChange();
}
return r.ChangeToPureOperator(comparison);
}
Reduction JSTypedLowering::ReduceSpeculativeNumberEqual(Node* node) {
JSBinopReduction r(this, node);
if (r.BothInputsAre(Type::Boolean())) {
return r.ChangeToPureOperator(simplified()->ReferenceEqual());
}
if (r.BothInputsAre(Type::Signed32()) ||
r.BothInputsAre(Type::Unsigned32())) {
return r.ChangeToPureOperator(simplified()->NumberEqual());
if (comparison->EffectInputCount() > 0) {
return r.ChangeToSpeculativeOperator(comparison, Type::Boolean());
} else {
return r.ChangeToPureOperator(comparison);
}
return NoChange();
}
Reduction JSTypedLowering::ReduceJSTypeOf(Node* node) {
......@@ -898,9 +964,13 @@ Reduction JSTypedLowering::ReduceJSEqual(Node* node, bool invert) {
return Changed(node);
}
NumberOperationHint hint;
if (r.BothInputsAre(Type::Signed32()) ||
r.BothInputsAre(Type::Unsigned32())) {
return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);
} else if (r.GetCompareNumberOperationHint(&hint)) {
return r.ChangeToSpeculativeOperator(
simplified()->SpeculativeNumberEqual(hint), invert, Type::Boolean());
} else if (r.BothInputsAre(Type::Number())) {
return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);
} else if (r.IsReceiverCompareOperation()) {
......@@ -951,9 +1021,13 @@ Reduction JSTypedLowering::ReduceJSStrictEqual(Node* node, bool invert) {
return r.ChangeToPureOperator(simplified()->StringEqual(), invert);
}
NumberOperationHint hint;
if (r.BothInputsAre(Type::Signed32()) ||
r.BothInputsAre(Type::Unsigned32())) {
return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);
} else if (r.GetCompareNumberOperationHint(&hint)) {
return r.ChangeToSpeculativeOperator(
simplified()->SpeculativeNumberEqual(hint), invert, Type::Boolean());
} else if (r.BothInputsAre(Type::Number())) {
return r.ChangeToPureOperator(simplified()->NumberEqual(), invert);
} else if (r.IsReceiverCompareOperation()) {
......@@ -2377,11 +2451,6 @@ Reduction JSTypedLowering::Reduce(Node* node) {
case IrOpcode::kSpeculativeNumberDivide:
case IrOpcode::kSpeculativeNumberModulus:
return ReduceSpeculativeNumberBinop(node);
case IrOpcode::kSpeculativeNumberEqual:
return ReduceSpeculativeNumberEqual(node);
case IrOpcode::kSpeculativeNumberLessThan:
case IrOpcode::kSpeculativeNumberLessThanOrEqual:
return ReduceSpeculativeNumberComparison(node);
default:
break;
}
......
......@@ -86,8 +86,6 @@ class V8_EXPORT_PRIVATE JSTypedLowering final
Reduction ReduceCreateConsString(Node* node);
Reduction ReduceSpeculativeNumberAdd(Node* node);
Reduction ReduceSpeculativeNumberBinop(Node* node);
Reduction ReduceSpeculativeNumberEqual(Node* node);
Reduction ReduceSpeculativeNumberComparison(Node* node);
Factory* factory() const;
Graph* graph() const;
......
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