[turbofan] Fix DCHECK in CommonOperatorReducer::DecideCondition

Bug: chromium:1408606
Change-Id: Ic2f41bd4b41c662ec2b075c3abe1b7a2d909e60a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4194727
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Reviewed-by: Darius Mercadier <dmercadier@chromium.org>
Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
Auto-Submit: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85486}

src/compiler/common-operator-reducer.cc

@@ -19,12 +19,10 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
-CommonOperatorReducer::CommonOperatorReducer(Editor* editor, Graph* graph,
-                                             JSHeapBroker* broker,
-                                             CommonOperatorBuilder* common,
-                                             MachineOperatorBuilder* machine,
-                                             Zone* temp_zone,
-                                             BranchSemantics branch_semantics)
+CommonOperatorReducer::CommonOperatorReducer(
+    Editor* editor, Graph* graph, JSHeapBroker* broker,
+    CommonOperatorBuilder* common, MachineOperatorBuilder* machine,
+    Zone* temp_zone, BranchSemantics default_branch_semantics)
     : AdvancedReducer(editor),
       graph_(graph),
       broker_(broker),
@@ -32,7 +30,7 @@ CommonOperatorReducer::CommonOperatorReducer(Editor* editor, Graph* graph,
       machine_(machine),
       dead_(graph->NewNode(common->Dead())),
       zone_(temp_zone),
-      branch_semantics_(branch_semantics) {
+      default_branch_semantics_(default_branch_semantics) {
   NodeProperties::SetType(dead_, Type::None());
 }
 
@@ -67,16 +65,17 @@ Reduction CommonOperatorReducer::Reduce(Node* node) {
   return NoChange();
 }
 
-Decision CommonOperatorReducer::DecideCondition(Node* const cond) {
+Decision CommonOperatorReducer::DecideCondition(
+    Node* const cond, BranchSemantics branch_semantics) {
   Node* unwrapped = SkipValueIdentities(cond);
   switch (unwrapped->opcode()) {
     case IrOpcode::kInt32Constant: {
-      DCHECK_EQ(branch_semantics_, BranchSemantics::kMachine);
+      DCHECK_EQ(branch_semantics, BranchSemantics::kMachine);
       Int32Matcher m(unwrapped);
       return m.ResolvedValue() ? Decision::kTrue : Decision::kFalse;
     }
     case IrOpcode::kHeapConstant: {
-      if (branch_semantics_ == BranchSemantics::kMachine) {
+      if (branch_semantics == BranchSemantics::kMachine) {
         return Decision::kTrue;
       }
       HeapObjectMatcher m(unwrapped);
@@ -91,6 +90,7 @@ Decision CommonOperatorReducer::DecideCondition(Node* const cond) {
 
 Reduction CommonOperatorReducer::ReduceBranch(Node* node) {
   DCHECK_EQ(IrOpcode::kBranch, node->opcode());
+  BranchSemantics branch_semantics = BranchSemanticsOf(node);
   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
@@ -99,8 +99,10 @@ Reduction CommonOperatorReducer::ReduceBranch(Node* node) {
   // not (i.e. true being returned in the false case and vice versa).
   if (cond->opcode() == IrOpcode::kBooleanNot ||
       (cond->opcode() == IrOpcode::kSelect &&
-       DecideCondition(cond->InputAt(1)) == Decision::kFalse &&
-       DecideCondition(cond->InputAt(2)) == Decision::kTrue)) {
+       DecideCondition(cond->InputAt(1), branch_semantics) ==
+           Decision::kFalse &&
+       DecideCondition(cond->InputAt(2), branch_semantics) ==
+           Decision::kTrue)) {
     for (Node* const use : node->uses()) {
       switch (use->opcode()) {
         case IrOpcode::kIfTrue:
@@ -122,7 +124,7 @@ Reduction CommonOperatorReducer::ReduceBranch(Node* node) {
         node, common()->Branch(NegateBranchHint(BranchHintOf(node->op()))));
     return Changed(node);
   }
-  Decision const decision = DecideCondition(cond);
+  Decision const decision = DecideCondition(cond, branch_semantics);
   if (decision == Decision::kUnknown) return NoChange();
   Node* const control = node->InputAt(1);
   for (Node* const use : node->uses()) {
@@ -161,7 +163,8 @@ Reduction CommonOperatorReducer::ReduceDeoptimizeConditional(Node* node) {
                   : common()->DeoptimizeUnless(p.reason(), p.feedback()));
     return Changed(node);
   }
-  Decision const decision = DecideCondition(condition);
+  Decision const decision =
+      DecideCondition(condition, default_branch_semantics_);
   if (decision == Decision::kUnknown) return NoChange();
   if (condition_is_true == (decision == Decision::kTrue)) {
     ReplaceWithValue(node, dead(), effect, control);
@@ -393,7 +396,7 @@ Reduction CommonOperatorReducer::ReduceSelect(Node* node) {
   Node* const vtrue = node->InputAt(1);
   Node* const vfalse = node->InputAt(2);
   if (vtrue == vfalse) return Replace(vtrue);
-  switch (DecideCondition(cond)) {
+  switch (DecideCondition(cond, default_branch_semantics_)) {
     case Decision::kTrue:
       return Replace(vtrue);
     case Decision::kFalse:
@@ -470,7 +473,7 @@ Reduction CommonOperatorReducer::ReduceSwitch(Node* node) {
 Reduction CommonOperatorReducer::ReduceStaticAssert(Node* node) {
   DCHECK_EQ(IrOpcode::kStaticAssert, node->opcode());
   Node* const cond = node->InputAt(0);
-  Decision decision = DecideCondition(cond);
+  Decision decision = DecideCondition(cond, default_branch_semantics_);
   if (decision == Decision::kTrue) {
     RelaxEffectsAndControls(node);
     return Changed(node);
@@ -484,7 +487,7 @@ Reduction CommonOperatorReducer::ReduceTrapConditional(Node* trap) {
          trap->opcode() == IrOpcode::kTrapUnless);
   bool trapping_condition = trap->opcode() == IrOpcode::kTrapIf;
   Node* const cond = trap->InputAt(0);
-  Decision decision = DecideCondition(cond);
+  Decision decision = DecideCondition(cond, default_branch_semantics_);
 
   if (decision == Decision::kUnknown) {
     return NoChange();

src/compiler/common-operator-reducer.h

@@ -27,7 +27,7 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final
   CommonOperatorReducer(Editor* editor, Graph* graph, JSHeapBroker* broker,
                         CommonOperatorBuilder* common,
                         MachineOperatorBuilder* machine, Zone* temp_zone,
-                        BranchSemantics branch_semantics);
+                        BranchSemantics default_branch_semantics);
   ~CommonOperatorReducer() final = default;
 
   const char* reducer_name() const override { return "CommonOperatorReducer"; }
@@ -50,7 +50,12 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final
   Reduction Change(Node* node, Operator const* op, Node* a, Node* b);
 
   // Helper to determine if conditions are true or false.
-  Decision DecideCondition(Node* const cond);
+  Decision DecideCondition(Node* const cond, BranchSemantics branch_semantics);
+  BranchSemantics BranchSemanticsOf(const Node* branch) {
+    BranchSemantics bs = BranchParametersOf(branch->op()).semantics();
+    if (bs != BranchSemantics::kUnspecified) return bs;
+    return default_branch_semantics_;
+  }
 
   Graph* graph() const { return graph_; }
   JSHeapBroker* broker() const { return broker_; }
@@ -64,7 +69,7 @@ class V8_EXPORT_PRIVATE CommonOperatorReducer final
   MachineOperatorBuilder* const machine_;
   Node* const dead_;
   Zone* zone_;
-  BranchSemantics branch_semantics_;
+  BranchSemantics default_branch_semantics_;
 };
 
 }  // namespace compiler

test/mjsunit/regress/regress-1408606.js

@@ -0,0 +1,18 @@
+// Copyright 2023 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --allow-natives-syntax
+
+var v0 = new Uint8ClampedArray();
+function f0(cnt) {
+  Object.defineProperty(v0, cnt == 0 ? "subarray" : "property", {
+  })
+  v0[-7] = -1.5;
+  v0[-7];
+}
+%PrepareFunctionForOptimization(f0);
+f0(0);
+f0();
+%OptimizeFunctionOnNextCall(f0);
+f0();