Commit d369c4dc authored by bmeurer's avatar bmeurer Committed by Commit bot

Revert of [turbofan] Constant propagation for JumpIfFalse/JumpIfTrue....

Revert of [turbofan] Constant propagation for JumpIfFalse/JumpIfTrue. (patchset #4 id:60001 of https://codereview.chromium.org/2666283002/ )

Reason for revert:
Breaks win64 it seems.

Original issue's description:
> [turbofan] Constant propagation for JumpIfFalse/JumpIfTrue.
>
> The JumpIfFalse and JumpIfTrue bytecodes test the accumulator, and
> branch based on whether the accumulator is true or false (no other
> value allowed, and in fact TurboFan would blow up if you would pass
> anything else, since Branch operator can only deal with Boolean).
> So for either branch we know exactly the value of the accumulator,
> and we can update the environment to this constant value instead.
>
> This helps to avoid the useless bit materialization that currently
> happens when || or && is being used in a value context.
>
> R=jarin@chromium.org
> BUG=v8:5267
>
> Review-Url: https://codereview.chromium.org/2666283002
> Cr-Commit-Position: refs/heads/master@{#42843}
> Committed: https://chromium.googlesource.com/v8/v8/+/158ac9287193f315342ad31c38fe451620d176eb

TBR=jarin@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5267

Review-Url: https://codereview.chromium.org/2668933002
Cr-Commit-Position: refs/heads/master@{#42845}
parent ed3d888d
...@@ -2969,10 +2969,6 @@ Node* CodeStubAssembler::IsWeakCell(Node* object) { ...@@ -2969,10 +2969,6 @@ Node* CodeStubAssembler::IsWeakCell(Node* object) {
return HasInstanceType(object, WEAK_CELL_TYPE); return HasInstanceType(object, WEAK_CELL_TYPE);
} }
Node* CodeStubAssembler::IsBoolean(Node* object) {
return IsBooleanMap(LoadMap(object));
}
Node* CodeStubAssembler::IsName(Node* object) { Node* CodeStubAssembler::IsName(Node* object) {
return Int32LessThanOrEqual(LoadInstanceType(object), return Int32LessThanOrEqual(LoadInstanceType(object),
Int32Constant(LAST_NAME_TYPE)); Int32Constant(LAST_NAME_TYPE));
......
...@@ -685,7 +685,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler { ...@@ -685,7 +685,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler : public compiler::CodeAssembler {
Node* IsJSReceiver(Node* object); Node* IsJSReceiver(Node* object);
Node* IsMap(Node* object); Node* IsMap(Node* object);
Node* IsCallableMap(Node* map); Node* IsCallableMap(Node* map);
Node* IsBoolean(Node* object);
Node* IsName(Node* object); Node* IsName(Node* object);
Node* IsSymbol(Node* object); Node* IsSymbol(Node* object);
Node* IsPrivateSymbol(Node* object); Node* IsPrivateSymbol(Node* object);
......
...@@ -2072,25 +2072,11 @@ void BytecodeGraphBuilder::BuildJumpIfEqual(Node* comperand) { ...@@ -2072,25 +2072,11 @@ void BytecodeGraphBuilder::BuildJumpIfEqual(Node* comperand) {
} }
void BytecodeGraphBuilder::BuildJumpIfFalse() { void BytecodeGraphBuilder::BuildJumpIfFalse() {
NewBranch(environment()->LookupAccumulator()); BuildJumpIfNot(environment()->LookupAccumulator());
Environment* if_true_environment = environment()->Copy();
environment()->BindAccumulator(jsgraph()->FalseConstant());
NewIfFalse();
MergeIntoSuccessorEnvironment(bytecode_iterator().GetJumpTargetOffset());
if_true_environment->BindAccumulator(jsgraph()->TrueConstant());
set_environment(if_true_environment);
NewIfTrue();
} }
void BytecodeGraphBuilder::BuildJumpIfTrue() { void BytecodeGraphBuilder::BuildJumpIfTrue() {
NewBranch(environment()->LookupAccumulator()); BuildJumpIf(environment()->LookupAccumulator());
Environment* if_false_environment = environment()->Copy();
environment()->BindAccumulator(jsgraph()->TrueConstant());
NewIfTrue();
MergeIntoSuccessorEnvironment(bytecode_iterator().GetJumpTargetOffset());
if_false_environment->BindAccumulator(jsgraph()->FalseConstant());
set_environment(if_false_environment);
NewIfFalse();
} }
void BytecodeGraphBuilder::BuildJumpIfToBooleanTrue() { void BytecodeGraphBuilder::BuildJumpIfToBooleanTrue() {
......
...@@ -894,7 +894,6 @@ class RepresentationSelector { ...@@ -894,7 +894,6 @@ class RepresentationSelector {
// Helper for handling selects. // Helper for handling selects.
void VisitSelect(Node* node, Truncation truncation, void VisitSelect(Node* node, Truncation truncation,
SimplifiedLowering* lowering) { SimplifiedLowering* lowering) {
DCHECK(TypeOf(node->InputAt(0))->Is(Type::Boolean()));
ProcessInput(node, 0, UseInfo::Bool()); ProcessInput(node, 0, UseInfo::Bool());
MachineRepresentation output = MachineRepresentation output =
...@@ -1425,12 +1424,10 @@ class RepresentationSelector { ...@@ -1425,12 +1424,10 @@ class RepresentationSelector {
return; return;
} }
case IrOpcode::kBranch: { case IrOpcode::kBranch:
DCHECK(TypeOf(node->InputAt(0))->Is(Type::Boolean()));
ProcessInput(node, 0, UseInfo::Bool()); ProcessInput(node, 0, UseInfo::Bool());
EnqueueInput(node, NodeProperties::FirstControlIndex(node)); EnqueueInput(node, NodeProperties::FirstControlIndex(node));
return; return;
}
case IrOpcode::kSwitch: case IrOpcode::kSwitch:
ProcessInput(node, 0, UseInfo::TruncatingWord32()); ProcessInput(node, 0, UseInfo::TruncatingWord32());
EnqueueInput(node, NodeProperties::FirstControlIndex(node)); EnqueueInput(node, NodeProperties::FirstControlIndex(node));
......
...@@ -207,8 +207,6 @@ void Verifier::Visitor::Check(Node* node) { ...@@ -207,8 +207,6 @@ void Verifier::Visitor::Check(Node* node) {
} }
CHECK_EQ(1, count_true); CHECK_EQ(1, count_true);
CHECK_EQ(1, count_false); CHECK_EQ(1, count_false);
// Condition must be a Boolean.
CheckValueInputIs(node, 0, Type::Boolean());
// Type is empty. // Type is empty.
CheckNotTyped(node); CheckNotTyped(node);
break; break;
......
...@@ -2407,58 +2407,46 @@ void Interpreter::DoJumpConstant(InterpreterAssembler* assembler) { ...@@ -2407,58 +2407,46 @@ void Interpreter::DoJumpConstant(InterpreterAssembler* assembler) {
// JumpIfTrue <imm> // JumpIfTrue <imm>
// //
// Jump by number of bytes represented by an immediate operand if the // Jump by number of bytes represented by an immediate operand if the
// accumulator contains true. This only works for boolean inputs, and // accumulator contains true.
// will misbehave if passed arbitrary input values.
void Interpreter::DoJumpIfTrue(InterpreterAssembler* assembler) { void Interpreter::DoJumpIfTrue(InterpreterAssembler* assembler) {
Node* accumulator = __ GetAccumulator(); Node* accumulator = __ GetAccumulator();
Node* relative_jump = __ BytecodeOperandUImmWord(0); Node* relative_jump = __ BytecodeOperandUImmWord(0);
Node* true_value = __ BooleanConstant(true); Node* true_value = __ BooleanConstant(true);
CSA_ASSERT(assembler, assembler->TaggedIsNotSmi(accumulator));
CSA_ASSERT(assembler, assembler->IsBoolean(accumulator));
__ JumpIfWordEqual(accumulator, true_value, relative_jump); __ JumpIfWordEqual(accumulator, true_value, relative_jump);
} }
// JumpIfTrueConstant <idx> // JumpIfTrueConstant <idx>
// //
// Jump by number of bytes in the Smi in the |idx| entry in the constant pool // Jump by number of bytes in the Smi in the |idx| entry in the constant pool
// if the accumulator contains true. This only works for boolean inputs, and // if the accumulator contains true.
// will misbehave if passed arbitrary input values.
void Interpreter::DoJumpIfTrueConstant(InterpreterAssembler* assembler) { void Interpreter::DoJumpIfTrueConstant(InterpreterAssembler* assembler) {
Node* accumulator = __ GetAccumulator(); Node* accumulator = __ GetAccumulator();
Node* index = __ BytecodeOperandIdx(0); Node* index = __ BytecodeOperandIdx(0);
Node* relative_jump = __ LoadAndUntagConstantPoolEntry(index); Node* relative_jump = __ LoadAndUntagConstantPoolEntry(index);
Node* true_value = __ BooleanConstant(true); Node* true_value = __ BooleanConstant(true);
CSA_ASSERT(assembler, assembler->TaggedIsNotSmi(accumulator));
CSA_ASSERT(assembler, assembler->IsBoolean(accumulator));
__ JumpIfWordEqual(accumulator, true_value, relative_jump); __ JumpIfWordEqual(accumulator, true_value, relative_jump);
} }
// JumpIfFalse <imm> // JumpIfFalse <imm>
// //
// Jump by number of bytes represented by an immediate operand if the // Jump by number of bytes represented by an immediate operand if the
// accumulator contains false. This only works for boolean inputs, and // accumulator contains false.
// will misbehave if passed arbitrary input values.
void Interpreter::DoJumpIfFalse(InterpreterAssembler* assembler) { void Interpreter::DoJumpIfFalse(InterpreterAssembler* assembler) {
Node* accumulator = __ GetAccumulator(); Node* accumulator = __ GetAccumulator();
Node* relative_jump = __ BytecodeOperandUImmWord(0); Node* relative_jump = __ BytecodeOperandUImmWord(0);
Node* false_value = __ BooleanConstant(false); Node* false_value = __ BooleanConstant(false);
CSA_ASSERT(assembler, assembler->TaggedIsNotSmi(accumulator));
CSA_ASSERT(assembler, assembler->IsBoolean(accumulator));
__ JumpIfWordEqual(accumulator, false_value, relative_jump); __ JumpIfWordEqual(accumulator, false_value, relative_jump);
} }
// JumpIfFalseConstant <idx> // JumpIfFalseConstant <idx>
// //
// Jump by number of bytes in the Smi in the |idx| entry in the constant pool // Jump by number of bytes in the Smi in the |idx| entry in the constant pool
// if the accumulator contains false. This only works for boolean inputs, and // if the accumulator contains false.
// will misbehave if passed arbitrary input values.
void Interpreter::DoJumpIfFalseConstant(InterpreterAssembler* assembler) { void Interpreter::DoJumpIfFalseConstant(InterpreterAssembler* assembler) {
Node* accumulator = __ GetAccumulator(); Node* accumulator = __ GetAccumulator();
Node* index = __ BytecodeOperandIdx(0); Node* index = __ BytecodeOperandIdx(0);
Node* relative_jump = __ LoadAndUntagConstantPoolEntry(index); Node* relative_jump = __ LoadAndUntagConstantPoolEntry(index);
Node* false_value = __ BooleanConstant(false); Node* false_value = __ BooleanConstant(false);
CSA_ASSERT(assembler, assembler->TaggedIsNotSmi(accumulator));
CSA_ASSERT(assembler, assembler->IsBoolean(accumulator));
__ JumpIfWordEqual(accumulator, false_value, relative_jump); __ JumpIfWordEqual(accumulator, false_value, relative_jump);
} }
......
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