[Interpreter] Improve handling of a === true / false.

Add support for direct jumping on True/False for strict equals of boolean
literals. This improves the score for such comparisons by around 75% on
baseline code, and by around 40x on optimized code for the added performance
test.

Bug=v8:6403

Change-Id: I81ea16a057e081eb6d159cd64c8e8615f65f9abb
Reviewed-on: https://chromium-review.googlesource.com/509570
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45450}

src/ast/ast.cc

@@ -102,6 +102,11 @@ bool Expression::IsStringLiteral() const {
   return IsLiteral() && AsLiteral()->raw_value()->IsString();
 }
 
+bool Expression::IsBooleanLiteral() const {
+  return IsLiteral() && (AsLiteral()->raw_value()->IsTrue() ||
+                         AsLiteral()->raw_value()->IsFalse());
+}
+
 bool Expression::IsPropertyName() const {
   return IsLiteral() && AsLiteral()->IsPropertyName();
 }
@@ -995,7 +1000,6 @@ bool CompareOperation::IsLiteralCompareUndefined(Expression** expr) {
          MatchLiteralCompareUndefined(right_, op(), left_, expr);
 }
 
-
 // Check for the pattern: null equals <expression>
 static bool MatchLiteralCompareNull(Expression* left,
                                     Token::Value op,
@@ -1008,12 +1012,28 @@ static bool MatchLiteralCompareNull(Expression* left,
   return false;
 }
 
-
 bool CompareOperation::IsLiteralCompareNull(Expression** expr) {
   return MatchLiteralCompareNull(left_, op(), right_, expr) ||
          MatchLiteralCompareNull(right_, op(), left_, expr);
 }
 
+// Check for the pattern: true/false equals <expression>
+static bool MatchLiteralStrictEqualBoolean(Expression* left, Token::Value op,
+                                           Expression* right, Expression** expr,
+                                           Literal** literal) {
+  if (left->IsBooleanLiteral() && op == Token::EQ_STRICT) {
+    *expr = right;
+    *literal = left->AsLiteral();
+    return true;
+  }
+  return false;
+}
+
+bool CompareOperation::IsLiteralStrictEqualBoolean(Expression** expr,
+                                                   Literal** literal) {
+  return MatchLiteralStrictEqualBoolean(right_, op(), left_, expr, literal) ||
+         MatchLiteralStrictEqualBoolean(left_, op(), right_, expr, literal);
+}
 
 // ----------------------------------------------------------------------------
 // Recording of type feedback

src/ast/ast.h

@@ -315,6 +315,9 @@ class Expression : public AstNode {
   // True iff the expression is a string literal.
   bool IsStringLiteral() const;
 
+  // True iff the expression is a boolean literal.
+  bool IsBooleanLiteral() const;
+
   // True iff the expression is the null literal.
   bool IsNullLiteral() const;
 
@@ -1184,6 +1187,11 @@ class Literal final : public Expression {
     return raw_value()->AsSmi();
   }
 
+  bool AsBooleanLiteral() {
+    DCHECK(IsBooleanLiteral());
+    return raw_value()->IsTrue();
+  }
+
   bool ToBooleanIsTrue() const { return raw_value()->BooleanValue(); }
   bool ToBooleanIsFalse() const { return !raw_value()->BooleanValue(); }
 
@@ -2294,6 +2302,7 @@ class CompareOperation final : public Expression {
 
   // Match special cases.
   bool IsLiteralCompareTypeof(Expression** expr, Literal** literal);
+  bool IsLiteralStrictEqualBoolean(Expression** expr, Literal** literal);
   bool IsLiteralCompareUndefined(Expression** expr);
   bool IsLiteralCompareNull(Expression** expr);
 

src/compiler/bytecode-graph-builder.cc

@@ -2461,28 +2461,18 @@ void BytecodeGraphBuilder::BuildJumpIfNotEqual(Node* comperand) {
   BuildJumpIfNot(condition);
 }
 
-void BytecodeGraphBuilder::BuildJumpIfFalse() {
-  NewBranch(environment()->LookupAccumulator());
-  {
-    SubEnvironment sub_environment(this);
-    NewIfFalse();
-    environment()->BindAccumulator(jsgraph()->FalseConstant());
-    MergeIntoSuccessorEnvironment(bytecode_iterator().GetJumpTargetOffset());
-  }
-  NewIfTrue();
-  environment()->BindAccumulator(jsgraph()->TrueConstant());
+void BytecodeGraphBuilder::BuildJumpIfTrue() {
+  Node* accumulator = environment()->LookupAccumulator();
+  Node* condition = NewNode(simplified()->ReferenceEqual(), accumulator,
+                            jsgraph()->TrueConstant());
+  BuildJumpIf(condition);
 }
 
-void BytecodeGraphBuilder::BuildJumpIfTrue() {
-  NewBranch(environment()->LookupAccumulator());
-  {
-    SubEnvironment sub_environment(this);
-    NewIfTrue();
-    environment()->BindAccumulator(jsgraph()->TrueConstant());
-    MergeIntoSuccessorEnvironment(bytecode_iterator().GetJumpTargetOffset());
-  }
-  NewIfFalse();
-  environment()->BindAccumulator(jsgraph()->FalseConstant());
+void BytecodeGraphBuilder::BuildJumpIfFalse() {
+  Node* accumulator = environment()->LookupAccumulator();
+  Node* condition = NewNode(simplified()->ReferenceEqual(), accumulator,
+                            jsgraph()->FalseConstant());
+  BuildJumpIf(condition);
 }
 
 void BytecodeGraphBuilder::BuildJumpIfToBooleanTrue() {

src/interpreter/bytecode-array-builder.cc

@@ -1064,6 +1064,15 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::JumpIfFalse(ToBooleanMode mode,
   return *this;
 }
 
+BytecodeArrayBuilder& BytecodeArrayBuilder::JumpIfBoolean(
+    bool boolean_comparison, ToBooleanMode mode, BytecodeLabel* label) {
+  if (boolean_comparison) {
+    return JumpIfTrue(mode, label);
+  } else {
+    return JumpIfFalse(mode, label);
+  }
+}
+
 BytecodeArrayBuilder& BytecodeArrayBuilder::JumpIfNull(BytecodeLabel* label) {
   DCHECK(!label->is_bound());
   OutputJumpIfNull(label, 0);

src/interpreter/bytecode-array-builder.h

@@ -367,6 +367,8 @@ class V8_EXPORT_PRIVATE BytecodeArrayBuilder final
 
   BytecodeArrayBuilder& JumpIfTrue(ToBooleanMode mode, BytecodeLabel* label);
   BytecodeArrayBuilder& JumpIfFalse(ToBooleanMode mode, BytecodeLabel* label);
+  BytecodeArrayBuilder& JumpIfBoolean(bool boolean_comparison,
+                                      ToBooleanMode mode, BytecodeLabel* label);
   BytecodeArrayBuilder& JumpIfNotHole(BytecodeLabel* label);
   BytecodeArrayBuilder& JumpIfJSReceiver(BytecodeLabel* label);
   BytecodeArrayBuilder& JumpIfNull(BytecodeLabel* label);

src/interpreter/bytecode-generator.cc

@@ -3182,6 +3182,25 @@ void BytecodeGenerator::BuildLiteralCompareNil(Token::Value op, NilValue nil) {
   }
 }
 
+void BytecodeGenerator::BuildLiteralBooleanStrictEquals(Literal* literal) {
+  TestResultScope* test_result = execution_result()->AsTest();
+  switch (test_result->fallthrough()) {
+    case TestFallthrough::kElse:
+      builder()->JumpIfBoolean(literal->AsBooleanLiteral(),
+                               ToBooleanMode::kAlreadyBoolean,
+                               test_result->NewThenLabel());
+      break;
+    case TestFallthrough::kNone:
+    case TestFallthrough::kThen:
+      builder()
+          ->JumpIfBoolean(literal->AsBooleanLiteral(),
+                          ToBooleanMode::kAlreadyBoolean,
+                          test_result->NewThenLabel())
+          .Jump(test_result->NewElseLabel());
+  }
+  test_result->SetResultConsumedByTest();
+}
+
 void BytecodeGenerator::VisitCompareOperation(CompareOperation* expr) {
   Expression* sub_expr;
   Literal* literal;
@@ -3197,6 +3216,11 @@ void BytecodeGenerator::VisitCompareOperation(CompareOperation* expr) {
     } else {
       builder()->CompareTypeOf(literal_flag);
     }
+  } else if (execution_result()->IsTest() &&
+             expr->IsLiteralStrictEqualBoolean(&sub_expr, &literal)) {
+    VisitForAccumulatorValue(sub_expr);
+    builder()->SetExpressionPosition(expr);
+    BuildLiteralBooleanStrictEquals(literal);
   } else if (expr->IsLiteralCompareUndefined(&sub_expr)) {
     VisitForAccumulatorValue(sub_expr);
     builder()->SetExpressionPosition(expr);

src/interpreter/bytecode-generator.h

@@ -114,6 +114,7 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
                                FeedbackSlot slot,
                                HoleCheckMode hole_check_mode);
   void BuildLiteralCompareNil(Token::Value compare_op, NilValue nil);
+  void BuildLiteralBooleanStrictEquals(Literal* literal);
   void BuildReturn();
   void BuildAsyncReturn();
   void BuildAsyncGeneratorReturn();

src/interpreter/interpreter-generator.cc

@@ -2442,8 +2442,6 @@ IGNITION_HANDLER(JumpIfTrue, InterpreterAssembler) {
   Node* accumulator = GetAccumulator();
   Node* relative_jump = BytecodeOperandUImmWord(0);
   Node* true_value = BooleanConstant(true);
-  CSA_ASSERT(this, TaggedIsNotSmi(accumulator));
-  CSA_ASSERT(this, IsBoolean(accumulator));
   JumpIfWordEqual(accumulator, true_value, relative_jump);
 }
 
@@ -2457,8 +2455,6 @@ IGNITION_HANDLER(JumpIfTrueConstant, InterpreterAssembler) {
   Node* index = BytecodeOperandIdx(0);
   Node* relative_jump = LoadAndUntagConstantPoolEntry(index);
   Node* true_value = BooleanConstant(true);
-  CSA_ASSERT(this, TaggedIsNotSmi(accumulator));
-  CSA_ASSERT(this, IsBoolean(accumulator));
   JumpIfWordEqual(accumulator, true_value, relative_jump);
 }
 
@@ -2471,8 +2467,6 @@ IGNITION_HANDLER(JumpIfFalse, InterpreterAssembler) {
   Node* accumulator = GetAccumulator();
   Node* relative_jump = BytecodeOperandUImmWord(0);
   Node* false_value = BooleanConstant(false);
-  CSA_ASSERT(this, TaggedIsNotSmi(accumulator));
-  CSA_ASSERT(this, IsBoolean(accumulator));
   JumpIfWordEqual(accumulator, false_value, relative_jump);
 }
 
@@ -2486,8 +2480,6 @@ IGNITION_HANDLER(JumpIfFalseConstant, InterpreterAssembler) {
   Node* index = BytecodeOperandIdx(0);
   Node* relative_jump = LoadAndUntagConstantPoolEntry(index);
   Node* false_value = BooleanConstant(false);
-  CSA_ASSERT(this, TaggedIsNotSmi(accumulator));
-  CSA_ASSERT(this, IsBoolean(accumulator));
   JumpIfWordEqual(accumulator, false_value, relative_jump);
 }
 

test/cctest/interpreter/bytecode_expectations/CompareBoolean.golden

+#
+# Autogenerated by generate-bytecode-expectations.
+#
+
+---
+wrap: yes
+
+---
+snippet: "
+  return (1 === true) ? 1 : 2;
+"
+frame size: 0
+parameter count: 1
+bytecode array length: 14
+bytecodes: [
+  /*   30 E> */ B(StackCheck),
+  /*   34 S> */ B(LdaSmi), I8(1),
+                B(JumpIfTrue), U8(4),
+                B(Jump), U8(6),
+                B(LdaSmi), I8(1),
+                B(Jump), U8(4),
+                B(LdaSmi), I8(2),
+  /*   63 S> */ B(Return),
+]
+constant pool: [
+]
+handlers: [
+]
+
+---
+snippet: "
+  return (false === 1) ? 1 : 2;
+"
+frame size: 0
+parameter count: 1
+bytecode array length: 14
+bytecodes: [
+  /*   30 E> */ B(StackCheck),
+  /*   34 S> */ B(LdaSmi), I8(1),
+                B(JumpIfFalse), U8(4),
+                B(Jump), U8(6),
+                B(LdaSmi), I8(1),
+                B(Jump), U8(4),
+                B(LdaSmi), I8(2),
+  /*   64 S> */ B(Return),
+]
+constant pool: [
+]
+handlers: [
+]
+
+---
+snippet: "
+  return (1 === true || 0 === false) ? 1 : 2;
+"
+frame size: 0
+parameter count: 1
+bytecode array length: 17
+bytecodes: [
+  /*   30 E> */ B(StackCheck),
+  /*   34 S> */ B(LdaSmi), I8(1),
+                B(JumpIfTrue), U8(7),
+                B(LdaZero),
+                B(JumpIfFalse), U8(4),
+                B(Jump), U8(6),
+                B(LdaSmi), I8(1),
+                B(Jump), U8(4),
+                B(LdaSmi), I8(2),
+  /*   78 S> */ B(Return),
+]
+constant pool: [
+]
+handlers: [
+]
+
+---
+snippet: "
+  if (1 === true || 1 === false) return 1;
+"
+frame size: 0
+parameter count: 1
+bytecode array length: 16
+bytecodes: [
+  /*   30 E> */ B(StackCheck),
+  /*   34 S> */ B(LdaSmi), I8(1),
+                B(JumpIfTrue), U8(8),
+                B(LdaSmi), I8(1),
+                B(JumpIfFalse), U8(4),
+                B(Jump), U8(5),
+  /*   65 S> */ B(LdaSmi), I8(1),
+  /*   75 S> */ B(Return),
+                B(LdaUndefined),
+  /*   75 S> */ B(Return),
+]
+constant pool: [
+]
+handlers: [
+]
+
+---
+snippet: "
+  if (!('false' === false)) return 1;
+"
+frame size: 0
+parameter count: 1
+bytecode array length: 10
+bytecodes: [
+  /*   30 E> */ B(StackCheck),
+  /*   34 S> */ B(LdaConstant), U8(0),
+                B(JumpIfFalse), U8(5),
+  /*   60 S> */ B(LdaSmi), I8(1),
+  /*   70 S> */ B(Return),
+                B(LdaUndefined),
+  /*   70 S> */ B(Return),
+]
+constant pool: [
+  ONE_BYTE_INTERNALIZED_STRING_TYPE ["false"],
+]
+handlers: [
+]
+

test/cctest/interpreter/test-bytecode-generator.cc

@@ -1029,6 +1029,26 @@ TEST(Typeof) {
                      LoadGolden("Typeof.golden")));
 }
 
+TEST(CompareBoolean) {
+  InitializedIgnitionHandleScope scope;
+  BytecodeExpectationsPrinter printer(CcTest::isolate());
+
+  const char* snippets[] = {
+      "return (1 === true) ? 1 : 2;\n",
+
+      "return (false === 1) ? 1 : 2;\n",
+
+      "return (1 === true || 0 === false) ? 1 : 2;\n",
+
+      "if (1 === true || 1 === false) return 1;\n",
+
+      "if (!('false' === false)) return 1;\n",
+  };
+
+  CHECK(CompareTexts(BuildActual(printer, snippets),
+                     LoadGolden("CompareBoolean.golden")));
+}
+
 TEST(CompareTypeOf) {
   InitializedIgnitionHandleScope scope;
   BytecodeExpectationsPrinter printer(CcTest::isolate());

test/js-perf-test/BytecodeHandlers/compare.js

@@ -16,6 +16,8 @@ addBenchmark('Number-StrictEquals-False', NumberStrictEqualsFalse);
 addBenchmark('String-StrictEquals-True', StringStrictEqualsTrue);
 addBenchmark('String-StrictEquals-False', StringStrictEqualsFalse);
 addBenchmark('SmiString-StrictEquals', MixedStrictEquals);
+addBenchmark('Boolean-StrictEquals-True', BooleanStrictEqualsTrue);
+addBenchmark('Boolean-StrictEquals-False', BooleanStrictEqualsTrue);
 addBenchmark('Smi-Equals-True', SmiEqualsTrue);
 addBenchmark('Smi-Equals-False', SmiEqualsFalse);
 addBenchmark('Number-Equals-True', NumberEqualsTrue);
@@ -61,6 +63,61 @@ function equals(a, b) {
   }
 }
 
+function testStrictEqualsBool(a) {
+  var ret;
+  for (var i = 0; i < 1000; ++i) {
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+    if (a === true || a === false) ret = true;
+  }
+  return ret;
+}
+
 // Relational comparison handlers are similar, so use one benchmark to measure
 // all of them.
 function relationalCompare(a, b) {
@@ -132,6 +189,14 @@ function StringEqualsTrue() {
  equals("abc", "abc");
 }
 
+function BooleanStrictEqualsTrue() {
+ testStrictEqualsBool(true);
+}
+
+function BooleanStrictEqualsFalse() {
+ testStrictEqualsBool("10");
+}
+
 function MixedEquals() {
  equals(10, "10");
 }

test/js-perf-test/JSTests.json

@@ -405,6 +405,9 @@
         {"name": "String-StrictEquals-True"},
         {"name": "String-StrictEquals-False"},
         {"name": "SmiString-StrictEquals"},
+        {"name": "Boolean-StrictEquals-True"},
+        {"name": "Boolean-StrictEquals-False"},
+        {"name": "SmiString-RelationalCompare"}
         {"name": "Smi-Equals-True"},
         {"name": "Smi-Equals-False"},
         {"name": "Number-Equals-True"},