Move branch inversion on ==0 into platform-agnostic reducer

This change is based on a discussion from
https://crrev.com/c/v8/v8/+/2053769/4/src/compiler/machine-operator-reducer.cc#1696
wherein Tobias suggested moving the folding away of ==0 operations out
of the platform-specific instruction selectors and into the
MachineOperatorReducer. I noticed that CommonOperatorReducer already
handles some very similar cases, so I have tried putting the ==0 folding
into CommonOperatorReducer instead. I'm happy to move it into
MachineOperatorReducer if that's better; I still don't have a very good
understanding of how roles are separated among reducers.

Change-Id: Ia0285bd9fafeef29d87cc88654bd6d355d467e8f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2076498
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66688}

src/compiler/backend/arm/instruction-selector-arm.cc

@@ -1896,16 +1896,6 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
 // Shared routine for word comparisons against zero.
 void InstructionSelector::VisitWordCompareZero(Node* user, Node* value,
                                                FlagsContinuation* cont) {
-  // Try to combine with comparisons against 0 by simply inverting the branch.
-  while (value->opcode() == IrOpcode::kWord32Equal && CanCover(user, value)) {
-    Int32BinopMatcher m(value);
-    if (!m.right().Is(0)) break;
-
-    user = value;
-    value = m.left().node();
-    cont->Negate();
-  }
-
   if (CanCover(user, value)) {
     switch (value->opcode()) {
       case IrOpcode::kWord32Equal:

src/compiler/backend/arm64/instruction-selector-arm64.cc

@@ -2395,15 +2395,6 @@ void VisitAtomicBinop(InstructionSelector* selector, Node* node,
 void InstructionSelector::VisitWordCompareZero(Node* user, Node* value,
                                                FlagsContinuation* cont) {
   Arm64OperandGenerator g(this);
-  // Try to combine with comparisons against 0 by simply inverting the branch.
-  while (value->opcode() == IrOpcode::kWord32Equal && CanCover(user, value)) {
-    Int32BinopMatcher m(value);
-    if (!m.right().Is(0)) break;
-
-    user = value;
-    value = m.left().node();
-    cont->Negate();
-  }
 
   // Try to match bit checks to create TBZ/TBNZ instructions.
   // Unlike the switch below, CanCover check is not needed here.

src/compiler/backend/ia32/instruction-selector-ia32.cc

@@ -1538,16 +1538,6 @@ void VisitPairAtomicBinOp(InstructionSelector* selector, Node* node,
 // Shared routine for word comparison with zero.
 void InstructionSelector::VisitWordCompareZero(Node* user, Node* value,
                                                FlagsContinuation* cont) {
-  // Try to combine with comparisons against 0 by simply inverting the branch.
-  while (value->opcode() == IrOpcode::kWord32Equal && CanCover(user, value)) {
-    Int32BinopMatcher m(value);
-    if (!m.right().Is(0)) break;
-
-    user = value;
-    value = m.left().node();
-    cont->Negate();
-  }
-
   if (CanCover(user, value)) {
     switch (value->opcode()) {
       case IrOpcode::kWord32Equal:

src/compiler/backend/mips/instruction-selector-mips.cc

@@ -1615,16 +1615,6 @@ void InstructionSelector::VisitStackPointerGreaterThan(
 // Shared routine for word comparisons against zero.
 void InstructionSelector::VisitWordCompareZero(Node* user, Node* value,
                                                FlagsContinuation* cont) {
-  // Try to combine with comparisons against 0 by simply inverting the branch.
-  while (value->opcode() == IrOpcode::kWord32Equal && CanCover(user, value)) {
-    Int32BinopMatcher m(value);
-    if (!m.right().Is(0)) break;
-
-    user = value;
-    value = m.left().node();
-    cont->Negate();
-  }
-
   if (CanCover(user, value)) {
     switch (value->opcode()) {
       case IrOpcode::kWord32Equal:

src/compiler/backend/mips64/instruction-selector-mips64.cc

@@ -2169,6 +2169,16 @@ void InstructionSelector::VisitStackPointerGreaterThan(
 void InstructionSelector::VisitWordCompareZero(Node* user, Node* value,
                                                FlagsContinuation* cont) {
   // Try to combine with comparisons against 0 by simply inverting the branch.
+  // This was already performed by CommonOperatorReducer, but it did not include
+  // the 64-bit comparison case because not all platforms handle 64-bit
+  // comparisons the same way.
+  //
+  // We actually must not include the 64bit case in CommonOperatorReducer, it
+  // would be unsound on platforms with pointer compression, since we got sloppy
+  // there about truncating 64 to 32 bit implicitly. That's an issue related to
+  // the general problem that it's unclear if the semantics of a Branch node is
+  // a 32 or 64 bit non-zero check. On most platforms, it seems to be a 32bit
+  // check while on Mips64 it's a 64bit check.
   while (CanCover(user, value)) {
     if (value->opcode() == IrOpcode::kWord32Equal) {
       Int32BinopMatcher m(value);

src/compiler/backend/ppc/instruction-selector-ppc.cc

@@ -1508,16 +1508,6 @@ void VisitFloat64Compare(InstructionSelector* selector, Node* node,
 // Shared routine for word comparisons against zero.
 void InstructionSelector::VisitWordCompareZero(Node* user, Node* value,
                                                FlagsContinuation* cont) {
-  // Try to combine with comparisons against 0 by simply inverting the branch.
-  while (value->opcode() == IrOpcode::kWord32Equal && CanCover(user, value)) {
-    Int32BinopMatcher m(value);
-    if (!m.right().Is(0)) break;
-
-    user = value;
-    value = m.left().node();
-    cont->Negate();
-  }
-
   if (CanCover(user, value)) {
     switch (value->opcode()) {
       case IrOpcode::kWord32Equal:

src/compiler/backend/s390/instruction-selector-s390.cc

@@ -1834,16 +1834,6 @@ void VisitLoadAndTest(InstructionSelector* selector, InstructionCode opcode,
 // Shared routine for word comparisons against zero.
 void InstructionSelector::VisitWordCompareZero(Node* user, Node* value,
                                                FlagsContinuation* cont) {
-  // Try to combine with comparisons against 0 by simply inverting the branch.
-  while (value->opcode() == IrOpcode::kWord32Equal && CanCover(user, value)) {
-    Int32BinopMatcher m(value);
-    if (!m.right().Is(0)) break;
-
-    user = value;
-    value = m.left().node();
-    cont->Negate();
-  }
-
   FlagsCondition fc = cont->condition();
   if (CanCover(user, value)) {
     switch (value->opcode()) {

src/compiler/backend/x64/instruction-selector-x64.cc

@@ -2063,16 +2063,6 @@ void VisitAtomicExchange(InstructionSelector* selector, Node* node,
 // Shared routine for word comparison against zero.
 void InstructionSelector::VisitWordCompareZero(Node* user, Node* value,
                                                FlagsContinuation* cont) {
-  // Try to combine with comparisons against 0 by simply inverting the branch.
-  while (value->opcode() == IrOpcode::kWord32Equal && CanCover(user, value)) {
-    Int32BinopMatcher m(value);
-    if (!m.right().Is(0)) break;
-
-    user = value;
-    value = m.left().node();
-    cont->Negate();
-  }
-
   if (CanCover(user, value)) {
     switch (value->opcode()) {
       case IrOpcode::kWord32Equal:

src/compiler/common-operator-reducer.cc

@@ -60,6 +60,9 @@ Reduction CommonOperatorReducer::Reduce(Node* node) {
     case IrOpcode::kDeoptimizeIf:
     case IrOpcode::kDeoptimizeUnless:
       return ReduceDeoptimizeConditional(node);
+    case IrOpcode::kTrapIf:
+    case IrOpcode::kTrapUnless:
+      return ReduceTrapConditional(node);
     case IrOpcode::kMerge:
       return ReduceMerge(node);
     case IrOpcode::kEffectPhi:
@@ -80,38 +83,77 @@ Reduction CommonOperatorReducer::Reduce(Node* node) {
   return NoChange();
 }
 
+Node* CommonOperatorReducer::GetInvertedConditionOrNull(Node* cond) {
+  switch (cond->opcode()) {
+    case IrOpcode::kBooleanNot:
+      return cond->InputAt(0);
+    case IrOpcode::kSelect:
+      // Check whether {cond} is a Select acting as a boolean not (i.e. true
+      // being returned in the false case and vice versa).
+      if (DecideCondition(broker(), cond->InputAt(1)) == Decision::kFalse &&
+          DecideCondition(broker(), cond->InputAt(2)) == Decision::kTrue) {
+        return cond->InputAt(0);
+      }
+      break;
+    case IrOpcode::kWord32Equal: {
+      // Check whether the comparison is against zero.
+      Uint32BinopMatcher m(cond);
+      if (m.right().Is(0)) {
+        return m.left().node();
+      }
+      break;
+    }
+    default:
+      break;
+  }
+
+  return nullptr;
+}
+
+base::Optional<CommonOperatorReducer::SimplifiedCondition>
+CommonOperatorReducer::TryGetSimplifiedCondition(Node* cond) {
+  Node* new_cond = cond;
+  bool is_inverted = false;
+  while (true) {
+    if (Node* next = GetInvertedConditionOrNull(new_cond)) {
+      new_cond = next;
+      is_inverted = !is_inverted;
+    } else {
+      if (cond == new_cond) return {};
+      return CommonOperatorReducer::SimplifiedCondition{new_cond, is_inverted};
+    }
+  }
+}
 
 Reduction CommonOperatorReducer::ReduceBranch(Node* node) {
   DCHECK_EQ(IrOpcode::kBranch, node->opcode());
   Node* const cond = node->InputAt(0);
-  // Swap IfTrue/IfFalse on {branch} if {cond} is a BooleanNot and use the input
-  // to BooleanNot as new condition for {branch}. Note we assume that {cond} was
-  // already properly optimized before we get here (as guaranteed by the graph
-  // reduction logic). The same applies if {cond} is a Select acting as boolean
-  // not (i.e. true being returned in the false case and vice versa).
-  if (cond->opcode() == IrOpcode::kBooleanNot ||
-      (cond->opcode() == IrOpcode::kSelect &&
-       DecideCondition(broker(), cond->InputAt(1)) == Decision::kFalse &&
-       DecideCondition(broker(), cond->InputAt(2)) == Decision::kTrue)) {
-    for (Node* const use : node->uses()) {
-      switch (use->opcode()) {
-        case IrOpcode::kIfTrue:
-          NodeProperties::ChangeOp(use, common()->IfFalse());
-          break;
-        case IrOpcode::kIfFalse:
-          NodeProperties::ChangeOp(use, common()->IfTrue());
-          break;
-        default:
-          UNREACHABLE();
+  // Swap IfTrue/IfFalse on {branch} if {cond} is a BooleanNot or similar, and
+  // use the input to BooleanNot as new condition for {branch}. Note we assume
+  // that {cond} was already properly optimized before we get here (as
+  // guaranteed by the graph reduction logic).
+  if (auto simplified = TryGetSimplifiedCondition(cond)) {
+    if (simplified->is_inverted) {
+      for (Node* const use : node->uses()) {
+        switch (use->opcode()) {
+          case IrOpcode::kIfTrue:
+            NodeProperties::ChangeOp(use, common()->IfFalse());
+            break;
+          case IrOpcode::kIfFalse:
+            NodeProperties::ChangeOp(use, common()->IfTrue());
+            break;
+          default:
+            UNREACHABLE();
+        }
       }
+      // Negate the hint for {branch}.
+      NodeProperties::ChangeOp(
+          node, common()->Branch(NegateBranchHint(BranchHintOf(node->op()))));
     }
     // Update the condition of {branch}. No need to mark the uses for revisit,
     // since we tell the graph reducer that the {branch} was changed and the
     // graph reduction logic will ensure that the uses are revisited properly.
-    node->ReplaceInput(0, cond->InputAt(0));
-    // Negate the hint for {branch}.
-    NodeProperties::ChangeOp(
-        node, common()->Branch(NegateBranchHint(BranchHintOf(node->op()))));
+    node->ReplaceInput(0, simplified->condition);
     return Changed(node);
   }
   Decision const decision = DecideCondition(broker(), cond);
@@ -141,17 +183,19 @@ Reduction CommonOperatorReducer::ReduceDeoptimizeConditional(Node* node) {
   Node* frame_state = NodeProperties::GetValueInput(node, 1);
   Node* effect = NodeProperties::GetEffectInput(node);
   Node* control = NodeProperties::GetControlInput(node);
-  // Swap DeoptimizeIf/DeoptimizeUnless on {node} if {cond} is a BooleaNot
-  // and use the input to BooleanNot as new condition for {node}.  Note we
-  // assume that {cond} was already properly optimized before we get here
-  // (as guaranteed by the graph reduction logic).
-  if (condition->opcode() == IrOpcode::kBooleanNot) {
-    NodeProperties::ReplaceValueInput(node, condition->InputAt(0), 0);
-    NodeProperties::ChangeOp(
-        node,
-        condition_is_true
-            ? common()->DeoptimizeIf(p.kind(), p.reason(), p.feedback())
-            : common()->DeoptimizeUnless(p.kind(), p.reason(), p.feedback()));
+  // Swap DeoptimizeIf/DeoptimizeUnless on {node} if {cond} is a BooleanNot or
+  // similar, and use the input to BooleanNot as new condition for {node}.  Note
+  // we assume that {cond} was already properly optimized before we get here (as
+  // guaranteed by the graph reduction logic).
+  if (auto simplified = TryGetSimplifiedCondition(condition)) {
+    NodeProperties::ReplaceValueInput(node, simplified->condition, 0);
+    if (simplified->is_inverted) {
+      NodeProperties::ChangeOp(
+          node,
+          condition_is_true
+              ? common()->DeoptimizeIf(p.kind(), p.reason(), p.feedback())
+              : common()->DeoptimizeUnless(p.kind(), p.reason(), p.feedback()));
+    }
     return Changed(node);
   }
   Decision const decision = DecideCondition(broker(), condition);
@@ -169,6 +213,28 @@ Reduction CommonOperatorReducer::ReduceDeoptimizeConditional(Node* node) {
   return Replace(dead());
 }
 
+Reduction CommonOperatorReducer::ReduceTrapConditional(Node* node) {
+  DCHECK(node->opcode() == IrOpcode::kTrapIf ||
+         node->opcode() == IrOpcode::kTrapUnless);
+  bool condition_is_true = node->opcode() == IrOpcode::kTrapUnless;
+  TrapId trap_id = TrapIdOf(node->op());
+  Node* condition = NodeProperties::GetValueInput(node, 0);
+  // Swap TrapIf/TrapUnless on {node} if {cond} is a BooleanNot or similar,
+  // and use the input to BooleanNot as new condition for {node}.  Note we
+  // assume that {cond} was already properly optimized before we get here
+  // (as guaranteed by the graph reduction logic).
+  if (auto simplified = TryGetSimplifiedCondition(condition)) {
+    NodeProperties::ReplaceValueInput(node, simplified->condition, 0);
+    if (simplified->is_inverted) {
+      NodeProperties::ChangeOp(node, condition_is_true
+                                         ? common()->TrapIf(trap_id)
+                                         : common()->TrapUnless(trap_id));
+    }
+    return Changed(node);
+  }
+  return NoChange();
+}
+
 Reduction CommonOperatorReducer::ReduceMerge(Node* node) {
   DCHECK_EQ(IrOpcode::kMerge, node->opcode());
   //

src/compiler/common-operator-reducer.h

@@ -36,6 +36,7 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final
  private:
   Reduction ReduceBranch(Node* node);
   Reduction ReduceDeoptimizeConditional(Node* node);
+  Reduction ReduceTrapConditional(Node* node);
   Reduction ReduceMerge(Node* node);
   Reduction ReduceEffectPhi(Node* node);
   Reduction ReducePhi(Node* node);
@@ -47,6 +48,17 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final
   Reduction Change(Node* node, Operator const* op, Node* a);
   Reduction Change(Node* node, Operator const* op, Node* a, Node* b);
 
+  // If the condition can be more simply represented by an inverted value,
+  // returns the node for that inverted value. Otherwise returns null.
+  Node* GetInvertedConditionOrNull(Node* cond);
+
+  struct SimplifiedCondition {
+    Node* condition;
+    bool is_inverted;
+  };
+  // Removes as many inversions as possible from a condition.
+  base::Optional<SimplifiedCondition> TryGetSimplifiedCondition(Node* cond);
+
   Graph* graph() const { return graph_; }
   JSHeapBroker* broker() const { return broker_; }
   CommonOperatorBuilder* common() const { return common_; }

src/wasm/baseline/arm/liftoff-assembler-arm.h

@@ -264,7 +264,7 @@ void LiftoffAssembler::PatchPrepareStackFrame(int offset, int frame_size) {
     patching_assembler.PadWithNops();
 
     // Now generate the OOL code.
-    AllocateStackSpace(bytes);
+    AllocateStackSpace(frame_size);
     // Jump back to the start of the function (from {pc_offset()} to {offset +
     // liftoff::kPatchInstructionsRequired * kInstrSize}).
     int func_start_offset =

test/unittests/compiler/arm/instruction-selector-arm-unittest.cc

@@ -402,131 +402,6 @@ TEST_P(InstructionSelectorDPITest, BranchWithShiftByImmediate) {
   }
 }
 
-
-TEST_P(InstructionSelectorDPITest, BranchIfZeroWithParameters) {
-  const DPI dpi = GetParam();
-  StreamBuilder m(this, MachineType::Int32(), MachineType::Int32(),
-                  MachineType::Int32());
-  RawMachineLabel a, b;
-  m.Branch(m.Word32Equal((m.*dpi.constructor)(m.Parameter(0), m.Parameter(1)),
-                         m.Int32Constant(0)),
-           &a, &b);
-  m.Bind(&a);
-  m.Return(m.Int32Constant(1));
-  m.Bind(&b);
-  m.Return(m.Int32Constant(0));
-  Stream s = m.Build();
-  ASSERT_EQ(1U, s.size());
-  EXPECT_EQ(dpi.test_arch_opcode, s[0]->arch_opcode());
-  EXPECT_EQ(kMode_Operand2_R, s[0]->addressing_mode());
-  EXPECT_EQ(kFlags_branch, s[0]->flags_mode());
-  EXPECT_EQ(kEqual, s[0]->flags_condition());
-}
-
-
-TEST_P(InstructionSelectorDPITest, BranchIfNotZeroWithParameters) {
-  const DPI dpi = GetParam();
-  StreamBuilder m(this, MachineType::Int32(), MachineType::Int32(),
-                  MachineType::Int32());
-  RawMachineLabel a, b;
-  m.Branch(
-      m.Word32NotEqual((m.*dpi.constructor)(m.Parameter(0), m.Parameter(1)),
-                       m.Int32Constant(0)),
-      &a, &b);
-  m.Bind(&a);
-  m.Return(m.Int32Constant(1));
-  m.Bind(&b);
-  m.Return(m.Int32Constant(0));
-  Stream s = m.Build();
-  ASSERT_EQ(1U, s.size());
-  EXPECT_EQ(dpi.test_arch_opcode, s[0]->arch_opcode());
-  EXPECT_EQ(kMode_Operand2_R, s[0]->addressing_mode());
-  EXPECT_EQ(kFlags_branch, s[0]->flags_mode());
-  EXPECT_EQ(kNotEqual, s[0]->flags_condition());
-}
-
-
-TEST_P(InstructionSelectorDPITest, BranchIfZeroWithImmediate) {
-  const DPI dpi = GetParam();
-  TRACED_FOREACH(int32_t, imm, kImmediates) {
-    StreamBuilder m(this, MachineType::Int32(), MachineType::Int32());
-    RawMachineLabel a, b;
-    m.Branch(m.Word32Equal(
-                 (m.*dpi.constructor)(m.Parameter(0), m.Int32Constant(imm)),
-                 m.Int32Constant(0)),
-             &a, &b);
-    m.Bind(&a);
-    m.Return(m.Int32Constant(1));
-    m.Bind(&b);
-    m.Return(m.Int32Constant(0));
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(dpi.test_arch_opcode, s[0]->arch_opcode());
-    EXPECT_EQ(kMode_Operand2_I, s[0]->addressing_mode());
-    EXPECT_EQ(kFlags_branch, s[0]->flags_mode());
-    EXPECT_EQ(kEqual, s[0]->flags_condition());
-  }
-  TRACED_FOREACH(int32_t, imm, kImmediates) {
-    StreamBuilder m(this, MachineType::Int32(), MachineType::Int32());
-    RawMachineLabel a, b;
-    m.Branch(m.Word32Equal(
-                 (m.*dpi.constructor)(m.Int32Constant(imm), m.Parameter(0)),
-                 m.Int32Constant(0)),
-             &a, &b);
-    m.Bind(&a);
-    m.Return(m.Int32Constant(1));
-    m.Bind(&b);
-    m.Return(m.Int32Constant(0));
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(dpi.test_arch_opcode, s[0]->arch_opcode());
-    EXPECT_EQ(kMode_Operand2_I, s[0]->addressing_mode());
-    EXPECT_EQ(kFlags_branch, s[0]->flags_mode());
-    EXPECT_EQ(kEqual, s[0]->flags_condition());
-  }
-}
-
-
-TEST_P(InstructionSelectorDPITest, BranchIfNotZeroWithImmediate) {
-  const DPI dpi = GetParam();
-  TRACED_FOREACH(int32_t, imm, kImmediates) {
-    StreamBuilder m(this, MachineType::Int32(), MachineType::Int32());
-    RawMachineLabel a, b;
-    m.Branch(m.Word32NotEqual(
-                 (m.*dpi.constructor)(m.Parameter(0), m.Int32Constant(imm)),
-                 m.Int32Constant(0)),
-             &a, &b);
-    m.Bind(&a);
-    m.Return(m.Int32Constant(1));
-    m.Bind(&b);
-    m.Return(m.Int32Constant(0));
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(dpi.test_arch_opcode, s[0]->arch_opcode());
-    EXPECT_EQ(kMode_Operand2_I, s[0]->addressing_mode());
-    EXPECT_EQ(kFlags_branch, s[0]->flags_mode());
-    EXPECT_EQ(kNotEqual, s[0]->flags_condition());
-  }
-  TRACED_FOREACH(int32_t, imm, kImmediates) {
-    StreamBuilder m(this, MachineType::Int32(), MachineType::Int32());
-    RawMachineLabel a, b;
-    m.Branch(m.Word32NotEqual(
-                 (m.*dpi.constructor)(m.Int32Constant(imm), m.Parameter(0)),
-                 m.Int32Constant(0)),
-             &a, &b);
-    m.Bind(&a);
-    m.Return(m.Int32Constant(1));
-    m.Bind(&b);
-    m.Return(m.Int32Constant(0));
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(dpi.test_arch_opcode, s[0]->arch_opcode());
-    EXPECT_EQ(kMode_Operand2_I, s[0]->addressing_mode());
-    EXPECT_EQ(kFlags_branch, s[0]->flags_mode());
-    EXPECT_EQ(kNotEqual, s[0]->flags_condition());
-  }
-}
-
 INSTANTIATE_TEST_SUITE_P(InstructionSelectorTest, InstructionSelectorDPITest,
                          ::testing::ValuesIn(kDPIs));
 
@@ -980,50 +855,6 @@ TEST_P(InstructionSelectorODPITest, BranchWithImmediate) {
   }
 }
 
-
-TEST_P(InstructionSelectorODPITest, BranchIfZeroWithParameters) {
-  const ODPI odpi = GetParam();
-  StreamBuilder m(this, MachineType::Int32(), MachineType::Int32(),
-                  MachineType::Int32());
-  RawMachineLabel a, b;
-  Node* n = (m.*odpi.constructor)(m.Parameter(0), m.Parameter(1));
-  m.Branch(m.Word32Equal(m.Projection(1, n), m.Int32Constant(0)), &a, &b);
-  m.Bind(&a);
-  m.Return(m.Projection(0, n));
-  m.Bind(&b);
-  m.Return(m.Int32Constant(0));
-  Stream s = m.Build();
-  ASSERT_EQ(1U, s.size());
-  EXPECT_EQ(odpi.arch_opcode, s[0]->arch_opcode());
-  EXPECT_EQ(kMode_Operand2_R, s[0]->addressing_mode());
-  EXPECT_EQ(4U, s[0]->InputCount());
-  EXPECT_EQ(1U, s[0]->OutputCount());
-  EXPECT_EQ(kFlags_branch, s[0]->flags_mode());
-  EXPECT_EQ(kNotOverflow, s[0]->flags_condition());
-}
-
-
-TEST_P(InstructionSelectorODPITest, BranchIfNotZeroWithParameters) {
-  const ODPI odpi = GetParam();
-  StreamBuilder m(this, MachineType::Int32(), MachineType::Int32(),
-                  MachineType::Int32());
-  RawMachineLabel a, b;
-  Node* n = (m.*odpi.constructor)(m.Parameter(0), m.Parameter(1));
-  m.Branch(m.Word32NotEqual(m.Projection(1, n), m.Int32Constant(0)), &a, &b);
-  m.Bind(&a);
-  m.Return(m.Projection(0, n));
-  m.Bind(&b);
-  m.Return(m.Int32Constant(0));
-  Stream s = m.Build();
-  ASSERT_EQ(1U, s.size());
-  EXPECT_EQ(odpi.arch_opcode, s[0]->arch_opcode());
-  EXPECT_EQ(kMode_Operand2_R, s[0]->addressing_mode());
-  EXPECT_EQ(4U, s[0]->InputCount());
-  EXPECT_EQ(1U, s[0]->OutputCount());
-  EXPECT_EQ(kFlags_branch, s[0]->flags_mode());
-  EXPECT_EQ(kOverflow, s[0]->flags_condition());
-}
-
 INSTANTIATE_TEST_SUITE_P(InstructionSelectorTest, InstructionSelectorODPITest,
                          ::testing::ValuesIn(kODPIs));
 
@@ -1952,8 +1783,6 @@ TEST_F(InstructionSelectorTest, Float64Sqrt) {
 const Comparison kBinopCmpZeroRightInstructions[] = {
     {&RawMachineAssembler::Word32Equal, "Word32Equal", kEqual, kNotEqual,
      kEqual},
-    {&RawMachineAssembler::Word32NotEqual, "Word32NotEqual", kNotEqual, kEqual,
-     kNotEqual},
     {&RawMachineAssembler::Int32LessThan, "Int32LessThan", kNegative,
      kPositiveOrZero, kNegative},
     {&RawMachineAssembler::Int32GreaterThanOrEqual, "Int32GreaterThanOrEqual",
@@ -1966,8 +1795,6 @@ const Comparison kBinopCmpZeroRightInstructions[] = {
 const Comparison kBinopCmpZeroLeftInstructions[] = {
     {&RawMachineAssembler::Word32Equal, "Word32Equal", kEqual, kNotEqual,
      kEqual},
-    {&RawMachineAssembler::Word32NotEqual, "Word32NotEqual", kNotEqual, kEqual,
-     kNotEqual},
     {&RawMachineAssembler::Int32GreaterThan, "Int32GreaterThan", kNegative,
      kPositiveOrZero, kNegative},
     {&RawMachineAssembler::Int32LessThanOrEqual, "Int32LessThanOrEqual",

test/unittests/compiler/arm64/instruction-selector-arm64-unittest.cc

@@ -1166,22 +1166,10 @@ const TestAndBranch kTestAndBranchMatchers32[] = {
           -> Node* { return m.Word32And(x, m.Int32Constant(mask)); },
       "if (x and mask)", kArm64TestAndBranch32, MachineType::Int32()},
      kNotEqual},
-    {{[](InstructionSelectorTest::StreamBuilder& m, Node* x,
-         uint32_t mask) -> Node* {
-        return m.Word32BinaryNot(m.Word32And(x, m.Int32Constant(mask)));
-      },
-      "if not (x and mask)", kArm64TestAndBranch32, MachineType::Int32()},
-     kEqual},
     {{[](InstructionSelectorTest::StreamBuilder& m, Node* x, uint32_t mask)
           -> Node* { return m.Word32And(m.Int32Constant(mask), x); },
       "if (mask and x)", kArm64TestAndBranch32, MachineType::Int32()},
      kNotEqual},
-    {{[](InstructionSelectorTest::StreamBuilder& m, Node* x,
-         uint32_t mask) -> Node* {
-        return m.Word32BinaryNot(m.Word32And(m.Int32Constant(mask), x));
-      },
-      "if not (mask and x)", kArm64TestAndBranch32, MachineType::Int32()},
-     kEqual},
     // Branch on the result of '(x and mask) == mask'. This tests that a bit is
     // set rather than cleared which is why conditions are inverted.
     {{[](InstructionSelectorTest::StreamBuilder& m, Node* x,
@@ -1191,13 +1179,6 @@ const TestAndBranch kTestAndBranchMatchers32[] = {
       },
       "if ((x and mask) == mask)", kArm64TestAndBranch32, MachineType::Int32()},
      kNotEqual},
-    {{[](InstructionSelectorTest::StreamBuilder& m, Node* x,
-         uint32_t mask) -> Node* {
-        return m.Word32BinaryNot(m.Word32Equal(
-            m.Word32And(x, m.Int32Constant(mask)), m.Int32Constant(mask)));
-      },
-      "if ((x and mask) != mask)", kArm64TestAndBranch32, MachineType::Int32()},
-     kEqual},
     {{[](InstructionSelectorTest::StreamBuilder& m, Node* x,
          uint32_t mask) -> Node* {
         return m.Word32Equal(m.Int32Constant(mask),
@@ -1205,13 +1186,6 @@ const TestAndBranch kTestAndBranchMatchers32[] = {
       },
       "if (mask == (x and mask))", kArm64TestAndBranch32, MachineType::Int32()},
      kNotEqual},
-    {{[](InstructionSelectorTest::StreamBuilder& m, Node* x,
-         uint32_t mask) -> Node* {
-        return m.Word32BinaryNot(m.Word32Equal(
-            m.Int32Constant(mask), m.Word32And(x, m.Int32Constant(mask))));
-      },
-      "if (mask != (x and mask))", kArm64TestAndBranch32, MachineType::Int32()},
-     kEqual},
     // Same as above but swap 'mask' and 'x'.
     {{[](InstructionSelectorTest::StreamBuilder& m, Node* x,
          uint32_t mask) -> Node* {
@@ -1220,27 +1194,13 @@ const TestAndBranch kTestAndBranchMatchers32[] = {
       },
       "if ((mask and x) == mask)", kArm64TestAndBranch32, MachineType::Int32()},
      kNotEqual},
-    {{[](InstructionSelectorTest::StreamBuilder& m, Node* x,
-         uint32_t mask) -> Node* {
-        return m.Word32BinaryNot(m.Word32Equal(
-            m.Word32And(m.Int32Constant(mask), x), m.Int32Constant(mask)));
-      },
-      "if ((mask and x) != mask)", kArm64TestAndBranch32, MachineType::Int32()},
-     kEqual},
     {{[](InstructionSelectorTest::StreamBuilder& m, Node* x,
          uint32_t mask) -> Node* {
         return m.Word32Equal(m.Int32Constant(mask),
                              m.Word32And(m.Int32Constant(mask), x));
       },
       "if (mask == (mask and x))", kArm64TestAndBranch32, MachineType::Int32()},
-     kNotEqual},
-    {{[](InstructionSelectorTest::StreamBuilder& m, Node* x,
-         uint32_t mask) -> Node* {
-        return m.Word32BinaryNot(m.Word32Equal(
-            m.Int32Constant(mask), m.Word32And(m.Int32Constant(mask), x)));
-      },
-      "if (mask != (mask and x))", kArm64TestAndBranch32, MachineType::Int32()},
-     kEqual}};
+     kNotEqual}};
 
 using InstructionSelectorTestAndBranchTest =
     InstructionSelectorTestWithParam<TestAndBranch>;
@@ -1466,30 +1426,7 @@ TEST_F(InstructionSelectorTest, Word32EqualZeroAndBranchWithOneBitMask) {
     uint32_t mask = 1 << bit;
     StreamBuilder m(this, MachineType::Int32(), MachineType::Int32());
     RawMachineLabel a, b;
-    m.Branch(m.Word32Equal(m.Word32And(m.Int32Constant(mask), m.Parameter(0)),
-                           m.Int32Constant(0)),
-             &a, &b);
-    m.Bind(&a);
-    m.Return(m.Int32Constant(1));
-    m.Bind(&b);
-    m.Return(m.Int32Constant(0));
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kArm64TestAndBranch32, s[0]->arch_opcode());
-    EXPECT_EQ(kEqual, s[0]->flags_condition());
-    EXPECT_EQ(4U, s[0]->InputCount());
-    EXPECT_EQ(InstructionOperand::IMMEDIATE, s[0]->InputAt(1)->kind());
-    EXPECT_EQ(bit, s.ToInt32(s[0]->InputAt(1)));
-  }
-
-  TRACED_FORRANGE(int, bit, 0, 31) {
-    uint32_t mask = 1 << bit;
-    StreamBuilder m(this, MachineType::Int32(), MachineType::Int32());
-    RawMachineLabel a, b;
-    m.Branch(
-        m.Word32NotEqual(m.Word32And(m.Int32Constant(mask), m.Parameter(0)),
-                         m.Int32Constant(0)),
-        &a, &b);
+    m.Branch(m.Word32And(m.Int32Constant(mask), m.Parameter(0)), &a, &b);
     m.Bind(&a);
     m.Return(m.Int32Constant(1));
     m.Bind(&b);
@@ -1524,27 +1461,6 @@ TEST_F(InstructionSelectorTest, Word64EqualZeroAndBranchWithOneBitMask) {
     EXPECT_EQ(InstructionOperand::IMMEDIATE, s[0]->InputAt(1)->kind());
     EXPECT_EQ(bit, s.ToInt64(s[0]->InputAt(1)));
   }
-
-  TRACED_FORRANGE(int, bit, 0, 63) {
-    uint64_t mask = uint64_t{1} << bit;
-    StreamBuilder m(this, MachineType::Int64(), MachineType::Int64());
-    RawMachineLabel a, b;
-    m.Branch(
-        m.Word64NotEqual(m.Word64And(m.Int64Constant(mask), m.Parameter(0)),
-                         m.Int64Constant(0)),
-        &a, &b);
-    m.Bind(&a);
-    m.Return(m.Int64Constant(1));
-    m.Bind(&b);
-    m.Return(m.Int64Constant(0));
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kArm64TestAndBranch, s[0]->arch_opcode());
-    EXPECT_EQ(kNotEqual, s[0]->flags_condition());
-    EXPECT_EQ(4U, s[0]->InputCount());
-    EXPECT_EQ(InstructionOperand::IMMEDIATE, s[0]->InputAt(1)->kind());
-    EXPECT_EQ(bit, s.ToInt64(s[0]->InputAt(1)));
-  }
 }
 
 TEST_F(InstructionSelectorTest, CompareAgainstZeroAndBranch) {
@@ -1584,73 +1500,20 @@ TEST_F(InstructionSelectorTest, CompareAgainstZeroAndBranch) {
 }
 
 TEST_F(InstructionSelectorTest, EqualZeroAndBranch) {
-  {
-    StreamBuilder m(this, MachineType::Int32(), MachineType::Int32());
-    RawMachineLabel a, b;
-    Node* p0 = m.Parameter(0);
-    m.Branch(m.Word32Equal(p0, m.Int32Constant(0)), &a, &b);
-    m.Bind(&a);
-    m.Return(m.Int32Constant(1));
-    m.Bind(&b);
-    m.Return(m.Int32Constant(0));
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kArm64CompareAndBranch32, s[0]->arch_opcode());
-    EXPECT_EQ(kEqual, s[0]->flags_condition());
-    EXPECT_EQ(3U, s[0]->InputCount());
-    EXPECT_EQ(s.ToVreg(p0), s.ToVreg(s[0]->InputAt(0)));
-  }
-
-  {
-    StreamBuilder m(this, MachineType::Int32(), MachineType::Int32());
-    RawMachineLabel a, b;
-    Node* p0 = m.Parameter(0);
-    m.Branch(m.Word32NotEqual(p0, m.Int32Constant(0)), &a, &b);
-    m.Bind(&a);
-    m.Return(m.Int32Constant(1));
-    m.Bind(&b);
-    m.Return(m.Int32Constant(0));
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kArm64CompareAndBranch32, s[0]->arch_opcode());
-    EXPECT_EQ(kNotEqual, s[0]->flags_condition());
-    EXPECT_EQ(3U, s[0]->InputCount());
-    EXPECT_EQ(s.ToVreg(p0), s.ToVreg(s[0]->InputAt(0)));
-  }
-
-  {
-    StreamBuilder m(this, MachineType::Int64(), MachineType::Int64());
-    RawMachineLabel a, b;
-    Node* p0 = m.Parameter(0);
-    m.Branch(m.Word64Equal(p0, m.Int64Constant(0)), &a, &b);
-    m.Bind(&a);
-    m.Return(m.Int64Constant(1));
-    m.Bind(&b);
-    m.Return(m.Int64Constant(0));
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kArm64CompareAndBranch, s[0]->arch_opcode());
-    EXPECT_EQ(kEqual, s[0]->flags_condition());
-    EXPECT_EQ(3U, s[0]->InputCount());
-    EXPECT_EQ(s.ToVreg(p0), s.ToVreg(s[0]->InputAt(0)));
-  }
-
-  {
-    StreamBuilder m(this, MachineType::Int64(), MachineType::Int64());
-    RawMachineLabel a, b;
-    Node* p0 = m.Parameter(0);
-    m.Branch(m.Word64NotEqual(p0, m.Int64Constant(0)), &a, &b);
-    m.Bind(&a);
-    m.Return(m.Int64Constant(1));
-    m.Bind(&b);
-    m.Return(m.Int64Constant(0));
-    Stream s = m.Build();
-    ASSERT_EQ(1U, s.size());
-    EXPECT_EQ(kArm64CompareAndBranch, s[0]->arch_opcode());
-    EXPECT_EQ(kNotEqual, s[0]->flags_condition());
-    EXPECT_EQ(3U, s[0]->InputCount());
-    EXPECT_EQ(s.ToVreg(p0), s.ToVreg(s[0]->InputAt(0)));
-  }
+  StreamBuilder m(this, MachineType::Int64(), MachineType::Int64());
+  RawMachineLabel a, b;
+  Node* p0 = m.Parameter(0);
+  m.Branch(m.Word64Equal(p0, m.Int64Constant(0)), &a, &b);
+  m.Bind(&a);
+  m.Return(m.Int64Constant(1));
+  m.Bind(&b);
+  m.Return(m.Int64Constant(0));
+  Stream s = m.Build();
+  ASSERT_EQ(1U, s.size());
+  EXPECT_EQ(kArm64CompareAndBranch, s[0]->arch_opcode());
+  EXPECT_EQ(kEqual, s[0]->flags_condition());
+  EXPECT_EQ(3U, s[0]->InputCount());
+  EXPECT_EQ(s.ToVreg(p0), s.ToVreg(s[0]->InputAt(0)));
 }
 
 // -----------------------------------------------------------------------------
@@ -4632,7 +4495,16 @@ TEST_F(InstructionSelectorTest, CompareAgainstZero32) {
     StreamBuilder m(this, MachineType::Int32(), MachineType::Int32());
     Node* const param = m.Parameter(0);
     RawMachineLabel a, b;
-    m.Branch((m.*cmp.mi.constructor)(param, m.Int32Constant(0)), &a, &b);
+    if (cmp.mi.constructor_name == std::string("Word32Equal")) {
+      // This case is inverted in an earlier reducer.
+      continue;
+    }
+    if (cmp.mi.constructor_name == std::string("Word32NotEqual")) {
+      // Simulate the result of an earlier reduction.
+      m.Branch(param, &a, &b);
+    } else {
+      m.Branch((m.*cmp.mi.constructor)(param, m.Int32Constant(0)), &a, &b);
+    }
     m.Bind(&a);
     m.Return(m.Int32Constant(1));
     m.Bind(&b);
@@ -4660,6 +4532,11 @@ TEST_F(InstructionSelectorTest, CompareAgainstZero64) {
     StreamBuilder m(this, MachineType::Int64(), MachineType::Int64());
     Node* const param = m.Parameter(0);
     RawMachineLabel a, b;
+    if (cmp.mi.constructor_name == std::string("Word64NotEqual")) {
+      // This case is inverted in an earlier reducer, making it equivalent to
+      // Word64Equal.
+      continue;
+    }
     m.Branch((m.*cmp.mi.constructor)(param, m.Int64Constant(0)), &a, &b);
     m.Bind(&a);
     m.Return(m.Int64Constant(1));

test/unittests/compiler/common-operator-reducer-unittest.cc

@@ -182,6 +182,26 @@ TEST_F(CommonOperatorReducerTest, BranchWithSelect) {
   }
 }
 
+TEST_F(CommonOperatorReducerTest, BranchWithEqualsZero) {
+  Node* const value = Parameter(0);
+  TRACED_FOREACH(BranchHint, hint, kBranchHints) {
+    Node* const control = graph()->start();
+    Node* const branch = graph()->NewNode(
+        common()->Branch(hint),
+        graph()->NewNode(machine()->Word32Equal(), Uint32Constant(0), value),
+        control);
+    Node* const if_true = graph()->NewNode(common()->IfTrue(), branch);
+    Node* const if_false = graph()->NewNode(common()->IfFalse(), branch);
+    Reduction const r = Reduce(branch);
+    ASSERT_TRUE(r.Changed());
+    EXPECT_EQ(branch, r.replacement());
+    EXPECT_THAT(branch, IsBranch(value, control));
+    EXPECT_THAT(if_false, IsIfTrue(branch));
+    EXPECT_THAT(if_true, IsIfFalse(branch));
+    EXPECT_EQ(NegateBranchHint(hint), BranchHintOf(branch->op()));
+  }
+}
+
 // -----------------------------------------------------------------------------
 // Merge