[Interpreter] Change LogicalNot to ToBooleanLogicalNot and add non-ToBoolean version.

Makes LogicalNot bytecode not do the ToBoolean operation, and add support in the
peephole optimizer to choose between the appropriate bytecode depending upon
whether the previous bytecode emitted a boolean or not.

BUG=v8:4280
LOG=N

Review-Url: https://codereview.chromium.org/1985033002
Cr-Commit-Position: refs/heads/master@{#36295}

src/bailout-reason.h

@@ -62,6 +62,7 @@ namespace internal {
     "EmitLoadRegister: Unsupported double immediate")                          \
   V(kEval, "eval")                                                             \
   V(kExpectedAllocationSite, "Expected allocation site")                       \
+  V(kExpectedBooleanValue, "Expected boolean value")                           \
   V(kExpectedFunctionObject, "Expected function object in register")           \
   V(kExpectedHeapNumber, "Expected HeapNumber")                                \
   V(kExpectedNativeContext, "Expected native context")                         \

src/compiler/bytecode-graph-builder.cc

@@ -1140,6 +1140,13 @@ void BytecodeGraphBuilder::VisitDec() {
 }
 
 void BytecodeGraphBuilder::VisitLogicalNot() {
+  Node* value = environment()->LookupAccumulator();
+  Node* node = NewNode(common()->Select(MachineRepresentation::kTagged), value,
+                       jsgraph()->FalseConstant(), jsgraph()->TrueConstant());
+  environment()->BindAccumulator(node);
+}
+
+void BytecodeGraphBuilder::VisitToBooleanLogicalNot() {
   Node* value = NewNode(javascript()->ToBoolean(ToBooleanHint::kAny),
                         environment()->LookupAccumulator());
   Node* node = NewNode(common()->Select(MachineRepresentation::kTagged), value,

src/interpreter/bytecode-array-builder.cc

@@ -188,7 +188,7 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::CountOperation(Token::Value op) {
 
 
 BytecodeArrayBuilder& BytecodeArrayBuilder::LogicalNot() {
-  Output(Bytecode::kLogicalNot);
+  Output(Bytecode::kToBooleanLogicalNot);
   return *this;
 }
 

src/interpreter/bytecode-peephole-optimizer.cc

@@ -95,15 +95,21 @@ bool BytecodePeepholeOptimizer::LastBytecodePutsNameInAccumulator() const {
 }
 
 void BytecodePeepholeOptimizer::UpdateCurrentBytecode(BytecodeNode* current) {
-  // Conditional jumps with boolean conditions are emiitted in
+  if (Bytecodes::IsJumpIfToBoolean(current->bytecode()) &&
+      Bytecodes::WritesBooleanToAccumulator(last_.bytecode())) {
+    // Conditional jumps with boolean conditions are emitted in
     // ToBoolean form by the bytecode array builder,
     // i.e. JumpIfToBooleanTrue rather JumpIfTrue. The ToBoolean element
     // can be removed if the previous bytecode put a boolean value in
     // the accumulator.
-  if (Bytecodes::IsJumpIfToBoolean(current->bytecode()) &&
-      Bytecodes::WritesBooleanToAccumulator(last_.bytecode())) {
     Bytecode jump = Bytecodes::GetJumpWithoutToBoolean(current->bytecode());
     current->set_bytecode(jump, current->operand(0), current->operand_scale());
+  } else if (current->bytecode() == Bytecode::kToBooleanLogicalNot &&
+             Bytecodes::WritesBooleanToAccumulator(last_.bytecode())) {
+    // Logical-nots are emitted in ToBoolean form by the bytecode array
+    // builder, The ToBoolean element can be removed if the previous bytecode
+    // put a boolean value in the accumulator.
+    current->set_bytecode(Bytecode::kLogicalNot);
   }
 }
 

src/interpreter/bytecodes.cc

@@ -243,6 +243,7 @@ bool Bytecodes::WritesBooleanToAccumulator(Bytecode bytecode) {
   switch (bytecode) {
     case Bytecode::kLdaTrue:
     case Bytecode::kLdaFalse:
+    case Bytecode::kToBooleanLogicalNot:
     case Bytecode::kLogicalNot:
     case Bytecode::kTestEqual:
     case Bytecode::kTestNotEqual:

src/interpreter/bytecodes.h

@@ -150,6 +150,7 @@ namespace interpreter {
   /* Unary Operators */                                                       \
   V(Inc, AccumulatorUse::kReadWrite)                                          \
   V(Dec, AccumulatorUse::kReadWrite)                                          \
+  V(ToBooleanLogicalNot, AccumulatorUse::kReadWrite)                          \
   V(LogicalNot, AccumulatorUse::kReadWrite)                                   \
   V(TypeOf, AccumulatorUse::kReadWrite)                                       \
   V(DeletePropertyStrict, AccumulatorUse::kReadWrite, OperandType::kReg)      \

src/interpreter/interpreter-assembler.h

@@ -143,6 +143,8 @@ class InterpreterAssembler : public CodeStubAssembler {
 
   // Abort with the given bailout reason.
   void Abort(BailoutReason bailout_reason);
+  void AbortIfWordNotEqual(compiler::Node* lhs, compiler::Node* rhs,
+                           BailoutReason bailout_reason);
 
  protected:
   Bytecode bytecode() const { return bytecode_; }
@@ -223,10 +225,6 @@ class InterpreterAssembler : public CodeStubAssembler {
   compiler::Node* DispatchToBytecodeHandlerEntry(
       compiler::Node* handler_entry, compiler::Node* bytecode_offset);
 
-  // Abort operations for debug code.
-  void AbortIfWordNotEqual(compiler::Node* lhs, compiler::Node* rhs,
-                           BailoutReason bailout_reason);
-
   OperandScale operand_scale() const { return operand_scale_; }
 
   Bytecode bytecode_;

src/interpreter/interpreter.cc

@@ -862,22 +862,11 @@ void Interpreter::DoDec(InterpreterAssembler* assembler) {
   DoCountOp(CodeFactory::Dec(isolate_), assembler);
 }
 
-
-// LogicalNot
-//
-// Perform logical-not on the accumulator, first casting the
-// accumulator to a boolean value if required.
-void Interpreter::DoLogicalNot(InterpreterAssembler* assembler) {
-  Callable callable = CodeFactory::ToBoolean(isolate_);
-  Node* target = __ HeapConstant(callable.code());
-  Node* accumulator = __ GetAccumulator();
-  Node* context = __ GetContext();
-  Node* to_boolean_value =
-      __ CallStub(callable.descriptor(), target, context, accumulator);
+void Interpreter::DoLogicalNotOp(Node* value, InterpreterAssembler* assembler) {
   Label if_true(assembler), if_false(assembler), end(assembler);
   Node* true_value = __ BooleanConstant(true);
   Node* false_value = __ BooleanConstant(false);
-  __ BranchIfWordEqual(to_boolean_value, true_value, &if_true, &if_false);
+  __ BranchIfWordEqual(value, true_value, &if_true, &if_false);
   __ Bind(&if_true);
   {
     __ SetAccumulator(false_value);
@@ -885,10 +874,38 @@ void Interpreter::DoLogicalNot(InterpreterAssembler* assembler) {
   }
   __ Bind(&if_false);
   {
+    if (FLAG_debug_code) {
+      __ AbortIfWordNotEqual(value, false_value,
+                             BailoutReason::kExpectedBooleanValue);
+    }
     __ SetAccumulator(true_value);
     __ Goto(&end);
   }
   __ Bind(&end);
+}
+
+// ToBooleanLogicalNot
+//
+// Perform logical-not on the accumulator, first casting the
+// accumulator to a boolean value if required.
+void Interpreter::DoToBooleanLogicalNot(InterpreterAssembler* assembler) {
+  Callable callable = CodeFactory::ToBoolean(isolate_);
+  Node* target = __ HeapConstant(callable.code());
+  Node* accumulator = __ GetAccumulator();
+  Node* context = __ GetContext();
+  Node* to_boolean_value =
+      __ CallStub(callable.descriptor(), target, context, accumulator);
+  DoLogicalNotOp(to_boolean_value, assembler);
+  __ Dispatch();
+}
+
+// LogicalNot
+//
+// Perform logical-not on the accumulator, which must already be a boolean
+// value.
+void Interpreter::DoLogicalNot(InterpreterAssembler* assembler) {
+  Node* value = __ GetAccumulator();
+  DoLogicalNotOp(value, assembler);
   __ Dispatch();
 }
 

src/interpreter/interpreter.h

@@ -21,6 +21,10 @@ class Isolate;
 class Callable;
 class CompilationInfo;
 
+namespace compiler {
+class Node;
+}  // namespace compiler
+
 namespace interpreter {
 
 class InterpreterAssembler;
@@ -123,6 +127,9 @@ class Interpreter {
   // Generates code to perform a type conversion.
   void DoTypeConversionOp(Callable callable, InterpreterAssembler* assembler);
 
+  // Generates code to perform logical-not on boolean |value|.
+  void DoLogicalNotOp(compiler::Node* value, InterpreterAssembler* assembler);
+
   // Generates code to perform delete via function_id.
   void DoDelete(Runtime::FunctionId function_id,
                 InterpreterAssembler* assembler);

test/cctest/interpreter/bytecode_expectations/ForOf.golden

@@ -37,7 +37,7 @@ bytecodes: [
   /*   45 E> */ B(Star), R(2),
                 B(Star), R(13),
                 B(InvokeIntrinsic), U16(Runtime::k_IsJSReceiver), R(13), U8(1),
-                B(LogicalNot),
+                B(ToBooleanLogicalNot),
                 B(JumpIfFalse), U8(11),
                 B(Ldar), R(2),
                 B(Star), R(13),
@@ -96,7 +96,7 @@ bytecodes: [
                 B(Star), R(12),
                 B(LdaUndefined),
                 B(TestEqualStrict), R(12),
-                B(LogicalNot),
+                B(ToBooleanLogicalNot),
                 B(JumpIfFalseConstant), U8(9),
                 B(Ldar), R(1),
                 B(Star), R(12),
@@ -218,7 +218,7 @@ bytecodes: [
   /*   65 E> */ B(Star), R(2),
                 B(Star), R(14),
                 B(InvokeIntrinsic), U16(Runtime::k_IsJSReceiver), R(14), U8(1),
-                B(LogicalNot),
+                B(ToBooleanLogicalNot),
                 B(JumpIfFalse), U8(11),
                 B(Ldar), R(2),
                 B(Star), R(14),
@@ -279,7 +279,7 @@ bytecodes: [
                 B(Star), R(13),
                 B(LdaUndefined),
                 B(TestEqualStrict), R(13),
-                B(LogicalNot),
+                B(ToBooleanLogicalNot),
                 B(JumpIfFalseConstant), U8(9),
                 B(Ldar), R(1),
                 B(Star), R(13),
@@ -406,7 +406,7 @@ bytecodes: [
   /*   45 E> */ B(Star), R(2),
                 B(Star), R(13),
                 B(InvokeIntrinsic), U16(Runtime::k_IsJSReceiver), R(13), U8(1),
-                B(LogicalNot),
+                B(ToBooleanLogicalNot),
                 B(JumpIfFalse), U8(11),
                 B(Ldar), R(2),
                 B(Star), R(13),
@@ -476,7 +476,7 @@ bytecodes: [
                 B(Star), R(12),
                 B(LdaUndefined),
                 B(TestEqualStrict), R(12),
-                B(LogicalNot),
+                B(ToBooleanLogicalNot),
                 B(JumpIfFalseConstant), U8(9),
                 B(Ldar), R(1),
                 B(Star), R(12),
@@ -599,7 +599,7 @@ bytecodes: [
   /*   74 E> */ B(Star), R(1),
                 B(Star), R(12),
                 B(InvokeIntrinsic), U16(Runtime::k_IsJSReceiver), R(12), U8(1),
-                B(LogicalNot),
+                B(ToBooleanLogicalNot),
                 B(JumpIfFalse), U8(11),
                 B(Ldar), R(1),
                 B(Star), R(12),
@@ -663,7 +663,7 @@ bytecodes: [
                 B(Star), R(11),
                 B(LdaUndefined),
                 B(TestEqualStrict), R(11),
-                B(LogicalNot),
+                B(ToBooleanLogicalNot),
                 B(JumpIfFalseConstant), U8(11),
                 B(Ldar), R(0),
                 B(Star), R(11),

test/cctest/interpreter/bytecode_expectations/Generators.golden

@@ -24,7 +24,7 @@ bytecodes: [
                 B(LdaZero),
                 B(TestEqualStrict), R(1),
                 B(JumpIfTrue), U8(57),
-                B(LdaSmi), U8(75),
+                B(LdaSmi), U8(76),
                 B(Star), R(2),
                 B(CallRuntime), U16(Runtime::kAbort), R(2), U8(1),
                 B(CallRuntime), U16(Runtime::kNewFunctionContext), R(closure), U8(1),
@@ -133,7 +133,7 @@ bytecodes: [
                 B(LdaSmi), U8(1),
                 B(TestEqualStrict), R(1),
                 B(JumpIfTrueConstant), U8(0),
-                B(LdaSmi), U8(75),
+                B(LdaSmi), U8(76),
                 B(Star), R(2),
                 B(CallRuntime), U16(Runtime::kAbort), R(2), U8(1),
                 B(CallRuntime), U16(Runtime::kNewFunctionContext), R(closure), U8(1),
@@ -283,7 +283,7 @@ bytecodes: [
                 B(LdaSmi), U8(1),
                 B(TestEqualStrict), R(3),
                 B(JumpIfTrueConstant), U8(3),
-                B(LdaSmi), U8(75),
+                B(LdaSmi), U8(76),
                 B(Star), R(4),
                 B(CallRuntime), U16(Runtime::kAbort), R(4), U8(1),
                 B(CallRuntime), U16(Runtime::kNewFunctionContext), R(closure), U8(1),
@@ -355,7 +355,7 @@ bytecodes: [
                 B(LdaSmi), U8(1),
                 B(TestEqualStrict), R(3),
                 B(JumpIfTrueConstant), U8(9),
-                B(LdaSmi), U8(75),
+                B(LdaSmi), U8(76),
                 B(Star), R(11),
                 B(CallRuntime), U16(Runtime::kAbort), R(11), U8(1),
   /*   27 S> */ B(LdaContextSlot), R(1), U8(7),
@@ -366,7 +366,7 @@ bytecodes: [
   /*   27 E> */ B(StaContextSlot), R(1), U8(8),
                 B(Star), R(11),
                 B(InvokeIntrinsic), U16(Runtime::k_IsJSReceiver), R(11), U8(1),
-                B(LogicalNot),
+                B(ToBooleanLogicalNot),
                 B(JumpIfFalse), U8(12),
                 B(LdaContextSlot), R(1), U8(8),
                 B(Star), R(11),
@@ -481,7 +481,7 @@ bytecodes: [
                 B(Star), R(10),
                 B(LdaUndefined),
                 B(TestEqualStrict), R(10),
-                B(LogicalNot),
+                B(ToBooleanLogicalNot),
                 B(JumpIfFalseConstant), U8(16),
                 B(LdaContextSlot), R(1), U8(7),
                 B(Star), R(10),

test/cctest/interpreter/bytecode_expectations/UnaryOperators.golden

@@ -60,7 +60,7 @@ bytecodes: [
   /*   42 E> */ B(Star), R(0),
   /*   49 E> */ B(StackCheck),
   /*   56 S> */ B(Ldar), R(0),
-                B(LogicalNot),
+                B(ToBooleanLogicalNot),
   /*   58 E> */ B(Star), R(0),
   /*   74 S> */ B(Ldar), R(0),
                 B(Star), R(1),

test/unittests/interpreter/bytecode-array-builder-unittest.cc

@@ -138,7 +138,10 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
   builder.CountOperation(Token::Value::ADD).CountOperation(Token::Value::SUB);
 
   // Emit unary operator invocations.
-  builder.LogicalNot().TypeOf();
+  builder
+      .LogicalNot()  // ToBooleanLogicalNot
+      .LogicalNot()  // non-ToBoolean LogicalNot
+      .TypeOf();
 
   // Emit delete
   builder.Delete(reg, LanguageMode::SLOPPY).Delete(reg, LanguageMode::STRICT);

test/unittests/interpreter/bytecode-peephole-optimizer-unittest.cc

@@ -142,6 +142,32 @@ TEST_F(BytecodePeepholeOptimizerTest, ElideJumpIfToBooleanTrue) {
   CHECK_EQ(last_written().operand(0), second.operand(0));
 }
 
+TEST_F(BytecodePeepholeOptimizerTest, KeepToBooleanLogicalNot) {
+  BytecodeNode first(Bytecode::kLdaNull);
+  BytecodeNode second(Bytecode::kToBooleanLogicalNot);
+  optimizer()->Write(&first);
+  CHECK_EQ(write_count(), 0);
+  optimizer()->Write(&second);
+  CHECK_EQ(write_count(), 1);
+  CHECK_EQ(last_written(), first);
+  optimizer()->FlushBasicBlock();
+  CHECK_EQ(write_count(), 2);
+  CHECK_EQ(last_written(), second);
+}
+
+TEST_F(BytecodePeepholeOptimizerTest, ElideToBooleanLogicalNot) {
+  BytecodeNode first(Bytecode::kLdaTrue);
+  BytecodeNode second(Bytecode::kToBooleanLogicalNot);
+  optimizer()->Write(&first);
+  CHECK_EQ(write_count(), 0);
+  optimizer()->Write(&second);
+  CHECK_EQ(write_count(), 1);
+  CHECK_EQ(last_written(), first);
+  optimizer()->FlushBasicBlock();
+  CHECK_EQ(write_count(), 2);
+  CHECK_EQ(last_written().bytecode(), Bytecode::kLogicalNot);
+}
+
 // Tests covering BytecodePeepholeOptimizer::CanElideCurrent().
 
 TEST_F(BytecodePeepholeOptimizerTest, LdarRxLdarRy) {