Skip to content
forked from v8/v8

Commit

Permalink
[interpreter] Add shortcut for !!foo
Browse files Browse the repository at this point in the history
Special case nested logical not in the interpreter, to shortcut repeated
logical nots. This in particular adds a faster path for the !!foo
convention of converting a value to a boolean (which, it turns out,
means we have to add a ToBoolean bytecode).

As a related fix, support ToBoolean/ToBooleanLogicalNot in maglev's
BuildBranchIfToBooleanTrue, avoiding a repeat ToBoolean operation on
them.

Change-Id: I68f99d71dc69b385b5a5fa144c885bafea1e2dee
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4466570
Auto-Submit: Leszek Swirski <[email protected]>
Reviewed-by: Michael Lippautz <[email protected]>
Commit-Queue: Michael Lippautz <[email protected]>
Commit-Queue: Leszek Swirski <[email protected]>
Cr-Commit-Position: refs/heads/main@{#87241}
  • Loading branch information
LeszekSwirski authored and V8 LUCI CQ committed Apr 24, 2023
1 parent 56dbf8e commit 5483d8e
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 4 deletions.
4 changes: 4 additions & 0 deletions src/baseline/baseline-compiler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1733,6 +1733,10 @@ void BaselineCompiler::VisitToString() {
CallBuiltin<Builtin::kToString>(kInterpreterAccumulatorRegister);
}

void BaselineCompiler::VisitToBoolean() {
CallBuiltin<Builtin::kToBoolean>(kInterpreterAccumulatorRegister);
}

void BaselineCompiler::VisitCreateRegExpLiteral() {
CallBuiltin<Builtin::kCreateRegExpLiteral>(
FeedbackVector(), // feedback vector
Expand Down
6 changes: 6 additions & 0 deletions src/compiler/bytecode-graph-builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3421,6 +3421,12 @@ void BytecodeGraphBuilder::VisitToString() {
environment()->BindAccumulator(value, Environment::kAttachFrameState);
}

void BytecodeGraphBuilder::VisitToBoolean() {
Node* value =
NewNode(simplified()->ToBoolean(), environment()->LookupAccumulator());
environment()->BindAccumulator(value, Environment::kAttachFrameState);
}

void BytecodeGraphBuilder::VisitToNumber() {
PrepareEagerCheckpoint();
Node* object = environment()->LookupAccumulator();
Expand Down
1 change: 1 addition & 0 deletions src/debug/debug-evaluate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,7 @@ bool BytecodeHasNoSideEffect(interpreter::Bytecode bytecode) {
case Bytecode::kToNumber:
case Bytecode::kToNumeric:
case Bytecode::kToString:
case Bytecode::kToBoolean:
// Misc.
case Bytecode::kIncBlockCounter: // Coverage counters.
case Bytecode::kForInEnumerate:
Expand Down
11 changes: 11 additions & 0 deletions src/interpreter/bytecode-array-builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1085,6 +1085,17 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::ToString() {
return *this;
}

BytecodeArrayBuilder& BytecodeArrayBuilder::ToBoolean(ToBooleanMode mode) {
if (mode == ToBooleanMode::kAlreadyBoolean) {
// No-op, the accumulator is already a boolean and ToBoolean both reads and
// writes the accumulator.
} else {
DCHECK_EQ(mode, ToBooleanMode::kConvertToBoolean);
OutputToBoolean();
}
return *this;
}

BytecodeArrayBuilder& BytecodeArrayBuilder::ToNumber(int feedback_slot) {
OutputToNumber(feedback_slot);
return *this;
Expand Down
3 changes: 2 additions & 1 deletion src/interpreter/bytecode-array-builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -407,9 +407,10 @@ class V8_EXPORT_PRIVATE BytecodeArrayBuilder final {
// Converts accumulator and stores result in register |out|.
BytecodeArrayBuilder& ToObject(Register out);
BytecodeArrayBuilder& ToName(Register out);
BytecodeArrayBuilder& ToString();

// Converts accumulator and stores result back in accumulator.
BytecodeArrayBuilder& ToString();
BytecodeArrayBuilder& ToBoolean(ToBooleanMode mode);
BytecodeArrayBuilder& ToNumber(int feedback_slot);
BytecodeArrayBuilder& ToNumeric(int feedback_slot);

Expand Down
13 changes: 11 additions & 2 deletions src/interpreter/bytecode-generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "src/common/globals.h"
#include "src/compiler-dispatcher/lazy-compile-dispatcher.h"
#include "src/heap/parked-scope.h"
#include "src/interpreter/bytecode-array-builder.h"
#include "src/interpreter/bytecode-flags.h"
#include "src/interpreter/bytecode-jump-table.h"
#include "src/interpreter/bytecode-label.h"
Expand Down Expand Up @@ -6208,8 +6209,16 @@ void BytecodeGenerator::VisitNot(UnaryOperation* expr) {
test_result->InvertControlFlow();
VisitInSameTestExecutionScope(expr->expression());
} else {
TypeHint type_hint = VisitForAccumulatorValue(expr->expression());
builder()->LogicalNot(ToBooleanModeFromTypeHint(type_hint));
UnaryOperation* unary_op = expr->expression()->AsUnaryOperation();
if (unary_op && unary_op->op() == Token::Value::NOT) {
// Shortcut repeated nots, to capture the `!!foo` pattern for converting
// expressions to booleans.
TypeHint type_hint = VisitForAccumulatorValue(unary_op->expression());
builder()->ToBoolean(ToBooleanModeFromTypeHint(type_hint));
} else {
TypeHint type_hint = VisitForAccumulatorValue(expr->expression());
builder()->LogicalNot(ToBooleanModeFromTypeHint(type_hint));
}
// Always returns a boolean value.
execution_result()->SetResultIsBoolean();
}
Expand Down
1 change: 1 addition & 0 deletions src/interpreter/bytecodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ namespace interpreter {
V(ToNumeric, ImplicitRegisterUse::kReadWriteAccumulator, OperandType::kIdx) \
V(ToObject, ImplicitRegisterUse::kReadAccumulator, OperandType::kRegOut) \
V(ToString, ImplicitRegisterUse::kReadWriteAccumulator) \
V(ToBoolean, ImplicitRegisterUse::kReadWriteAccumulator) \
\
/* Literals */ \
V(CreateRegExpLiteral, ImplicitRegisterUse::kWriteAccumulator, \
Expand Down
23 changes: 23 additions & 0 deletions src/interpreter/interpreter-generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1208,6 +1208,29 @@ IGNITION_HANDLER(ToString, InterpreterAssembler) {
Dispatch();
}

// ToString
//
// Convert the accumulator to a String.
IGNITION_HANDLER(ToBoolean, InterpreterAssembler) {
TNode<Object> value = GetAccumulator();
TVARIABLE(Oddball, result);
Label if_true(this), if_false(this), end(this);
BranchIfToBooleanIsTrue(value, &if_true, &if_false);
BIND(&if_true);
{
result = TrueConstant();
Goto(&end);
}
BIND(&if_false);
{
result = FalseConstant();
Goto(&end);
}
BIND(&end);
SetAccumulator(result.value());
Dispatch();
}

// Inc
//
// Increments value in the accumulator by one.
Expand Down
24 changes: 24 additions & 0 deletions src/maglev/maglev-graph-builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6311,6 +6311,23 @@ void MaglevGraphBuilder::VisitToString() {
SetAccumulator(BuildToString(value, ToString::kThrowOnSymbol));
}

void MaglevGraphBuilder::VisitToBoolean() {
ValueNode* value = GetAccumulatorTagged();
switch (value->opcode()) {
#define CASE(Name) \
case Opcode::k##Name: { \
SetAccumulator( \
GetBooleanConstant(value->Cast<Name>()->ToBoolean(local_isolate()))); \
break; \
}
CONSTANT_VALUE_NODE_LIST(CASE)
#undef CASE
default:
SetAccumulator(AddNewNode<ToBoolean>({value}));
break;
}
}

void MaglevGraphBuilder::VisitCreateRegExpLiteral() {
// CreateRegExpLiteral <pattern_idx> <literal_idx> <flags>
compiler::StringRef pattern = GetRefOperand<String>(0);
Expand Down Expand Up @@ -7304,6 +7321,13 @@ void MaglevGraphBuilder::BuildBranchIfToBooleanTrue(ValueNode* node,
Operation::kEqual, true_target, false_target);
break;
}
// Known boolean-valued codes, we don't need to call ToBoolean on them.
case Opcode::kToBoolean:
case Opcode::kToBooleanLogicalNot: {
block = FinishBlock<BranchIfRootConstant>({node}, RootIndex::kTrueValue,
true_target, false_target);
break;
}
default:
block =
BuildSpecializedBranchIfCompareNode(node, true_target, false_target);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,12 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
.CompareNull();

// Emit conversion operator invocations.
builder.ToNumber(1).ToNumeric(1).ToObject(reg).ToName(reg).ToString();
builder.ToNumber(1)
.ToNumeric(1)
.ToObject(reg)
.ToName(reg)
.ToString()
.ToBoolean(ToBooleanMode::kConvertToBoolean);

// Emit GetSuperConstructor.
builder.GetSuperConstructor(reg);
Expand Down

0 comments on commit 5483d8e

Please sign in to comment.