[turbofan] Add proper conversions in RedundancyElimination

Change-Id: Ia832abb79894dfde290a8127534b161d6fcc8178
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4197350
Reviewed-by: Darius Mercadier <dmercadier@chromium.org>
Commit-Queue: Nico Hartmann <nicohartmann@chromium.org>
Cr-Commit-Position: refs/heads/main@{#85504}

src/compiler/pipeline.cc

@@ -1805,7 +1805,8 @@ struct EarlyOptimizationPhase {
     SimplifiedOperatorReducer simple_reducer(&graph_reducer, data->jsgraph(),
                                              data->broker(),
                                              BranchSemantics::kMachine);
-    RedundancyElimination redundancy_elimination(&graph_reducer, temp_zone);
+    RedundancyElimination redundancy_elimination(&graph_reducer,
+                                                 data->jsgraph(), temp_zone);
     ValueNumberingReducer value_numbering(temp_zone, data->graph()->zone());
     MachineOperatorReducer machine_reducer(
         &graph_reducer, data->jsgraph(),
@@ -1920,7 +1921,8 @@ struct LoadEliminationPhase {
         &graph_reducer, data->jsgraph(), temp_zone, BranchElimination::kEARLY);
     DeadCodeElimination dead_code_elimination(&graph_reducer, data->graph(),
                                               data->common(), temp_zone);
-    RedundancyElimination redundancy_elimination(&graph_reducer, temp_zone);
+    RedundancyElimination redundancy_elimination(&graph_reducer,
+                                                 data->jsgraph(), temp_zone);
     LoadElimination load_elimination(&graph_reducer, data->jsgraph(),
                                      temp_zone);
     CheckpointElimination checkpoint_elimination(&graph_reducer);

src/compiler/redundancy-elimination.cc

@@ -4,6 +4,7 @@
 
 #include "src/compiler/redundancy-elimination.h"
 
+#include "src/compiler/js-graph.h"
 #include "src/compiler/node-properties.h"
 #include "src/compiler/simplified-operator.h"
 
@@ -11,8 +12,12 @@ namespace v8 {
 namespace internal {
 namespace compiler {
 
-RedundancyElimination::RedundancyElimination(Editor* editor, Zone* zone)
+RedundancyElimination::RedundancyElimination(Editor* editor, JSGraph* jsgraph,
-    : AdvancedReducer(editor), node_checks_(zone), zone_(zone) {}
+                                             Zone* zone)
+    : AdvancedReducer(editor),
+      node_checks_(zone),
+      jsgraph_(jsgraph),
+      zone_(zone) {}
 
 RedundancyElimination::~RedundancyElimination() = default;
 
@@ -133,8 +138,43 @@ RedundancyElimination::EffectPathChecks::AddCheck(Zone* zone,
 
 namespace {
 
+struct Subsumption {
+  enum class Kind {
+    kNone,
+    kImplicit,
+    kWithConversion,
+  };
+
+  static Subsumption None() { return Subsumption(Kind::kNone, nullptr); }
+  static Subsumption Implicit() {
+    return Subsumption(Kind::kImplicit, nullptr);
+  }
+  static Subsumption WithConversion(const Operator* conversion_op) {
+    return Subsumption(Kind::kWithConversion, conversion_op);
+  }
+
+  bool IsNone() const { return kind_ == Kind::kNone; }
+  bool IsImplicit() const { return kind_ == Kind::kImplicit; }
+  bool IsWithConversion() const { return kind_ == Kind::kWithConversion; }
+  const Operator* conversion_operator() const {
+    DCHECK(IsWithConversion());
+    return conversion_op_;
+  }
+
+ private:
+  Subsumption(Kind kind, const Operator* conversion_op)
+      : kind_(kind), conversion_op_(conversion_op) {
+    DCHECK_EQ(kind_ == Kind::kWithConversion, conversion_op_ != nullptr);
+  }
+
+  Kind kind_;
+  const Operator* conversion_op_;
+};
+
 // Does check {a} subsume check {b}?
-bool CheckSubsumes(Node const* a, Node const* b) {
+Subsumption CheckSubsumes(Node const* a, Node const* b,
+                          MachineOperatorBuilder* machine) {
+  Subsumption subsumption = Subsumption::Implicit();
   if (a->op() != b->op()) {
     if (a->opcode() == IrOpcode::kCheckInternalizedString &&
         b->opcode() == IrOpcode::kCheckString) {
@@ -149,14 +189,24 @@ bool CheckSubsumes(Node const* a, Node const* b) {
                b->opcode() == IrOpcode::kCheckedTaggedToArrayIndex) {
       // CheckedTaggedSignedToInt32(node) implies
       // CheckedTaggedToArrayIndex(node)
+      if (machine->Is64()) {
+        // On 64 bit architectures, ArrayIndex is 64 bit.
+        subsumption =
+            Subsumption::WithConversion(machine->ChangeInt32ToInt64());
+      }
     } else if (a->opcode() == IrOpcode::kCheckedTaggedToInt32 &&
                b->opcode() == IrOpcode::kCheckedTaggedToArrayIndex) {
       // CheckedTaggedToInt32(node) implies CheckedTaggedToArrayIndex(node)
+      if (machine->Is64()) {
+        // On 64 bit architectures, ArrayIndex is 64 bit.
+        subsumption =
+            Subsumption::WithConversion(machine->ChangeInt32ToInt64());
+      }
     } else if (a->opcode() == IrOpcode::kCheckReceiver &&
                b->opcode() == IrOpcode::kCheckReceiverOrNullOrUndefined) {
       // CheckReceiver(node) implies CheckReceiverOrNullOrUndefined(node)
     } else if (a->opcode() != b->opcode()) {
-      return false;
+      return Subsumption::None();
     } else {
       switch (a->opcode()) {
         case IrOpcode::kCheckBounds:
@@ -189,7 +239,7 @@ bool CheckSubsumes(Node const* a, Node const* b) {
           const CheckMinusZeroParameters& bp =
               CheckMinusZeroParametersOf(b->op());
           if (ap.mode() != bp.mode()) {
-            return false;
+            return Subsumption::None();
           }
           break;
         }
@@ -203,20 +253,20 @@ bool CheckSubsumes(Node const* a, Node const* b) {
           // for Number, in which case {b} will be subsumed no matter what.
           if (ap.mode() != bp.mode() &&
               ap.mode() != CheckTaggedInputMode::kNumber) {
-            return false;
+            return Subsumption::None();
           }
           break;
         }
         default:
           DCHECK(!IsCheckedWithFeedback(a->op()));
-          return false;
+          return Subsumption::None();
       }
     }
   }
   for (int i = a->op()->ValueInputCount(); --i >= 0;) {
-    if (a->InputAt(i) != b->InputAt(i)) return false;
+    if (a->InputAt(i) != b->InputAt(i)) return Subsumption::None();
   }
-  return true;
+  return subsumption;
 }
 
 bool TypeSubsumes(Node* node, Node* replacement) {
@@ -232,11 +282,19 @@ bool TypeSubsumes(Node* node, Node* replacement) {
 
 }  // namespace
 
-Node* RedundancyElimination::EffectPathChecks::LookupCheck(Node* node) const {
+Node* RedundancyElimination::EffectPathChecks::LookupCheck(
+    Node* node, JSGraph* jsgraph) const {
   for (Check const* check = head_; check != nullptr; check = check->next) {
-    if (CheckSubsumes(check->node, node) && TypeSubsumes(node, check->node)) {
+    Subsumption subsumption =
+        CheckSubsumes(check->node, node, jsgraph->machine());
+    if (!subsumption.IsNone() && TypeSubsumes(node, check->node)) {
       DCHECK(!check->node->IsDead());
-      return check->node;
+      Node* result = check->node;
+      if (subsumption.IsWithConversion()) {
+        result = jsgraph->graph()->NewNode(subsumption.conversion_operator(),
+                                           result);
+      }
+      return result;
     }
   }
   return nullptr;
@@ -276,7 +334,7 @@ Reduction RedundancyElimination::ReduceCheckNode(Node* node) {
   // because we will have to recompute anyway once we compute the predecessor.
   if (checks == nullptr) return NoChange();
   // See if we have another check that dominates us.
-  if (Node* check = checks->LookupCheck(node)) {
+  if (Node* check = checks->LookupCheck(node, jsgraph_)) {
     ReplaceWithValue(node, check);
     return Replace(check);
   }

src/compiler/redundancy-elimination.h

@@ -6,14 +6,17 @@
 #define V8_COMPILER_REDUNDANCY_ELIMINATION_H_
 
 #include "src/compiler/graph-reducer.h"
+#include "src/compiler/machine-operator.h"
 
 namespace v8 {
 namespace internal {
 namespace compiler {
 
+class JSGraph;
+
 class V8_EXPORT_PRIVATE RedundancyElimination final : public AdvancedReducer {
  public:
-  RedundancyElimination(Editor* editor, Zone* zone);
+  RedundancyElimination(Editor* editor, JSGraph* jsgraph, Zone* zone);
   ~RedundancyElimination() final;
   RedundancyElimination(const RedundancyElimination&) = delete;
   RedundancyElimination& operator=(const RedundancyElimination&) = delete;
@@ -37,7 +40,7 @@ class V8_EXPORT_PRIVATE RedundancyElimination final : public AdvancedReducer {
     void Merge(EffectPathChecks const* that);
 
     EffectPathChecks const* AddCheck(Zone* zone, Node* node) const;
-    Node* LookupCheck(Node* node) const;
+    Node* LookupCheck(Node* node, JSGraph* jsgraph) const;
     Node* LookupBoundsCheckFor(Node* node) const;
 
    private:
@@ -74,6 +77,7 @@ class V8_EXPORT_PRIVATE RedundancyElimination final : public AdvancedReducer {
   Zone* zone() const { return zone_; }
 
   PathChecksForEffectNodes node_checks_;
+  JSGraph* jsgraph_;
   Zone* const zone_;
 };
 

test/unittests/compiler/redundancy-elimination-unittest.cc

@@ -6,6 +6,7 @@
 
 #include "src/codegen/tick-counter.h"
 #include "src/compiler/feedback-source.h"
+#include "src/compiler/js-graph.h"
 #include "test/unittests/compiler/graph-reducer-unittest.h"
 #include "test/unittests/compiler/graph-unittest.h"
 #include "test/unittests/compiler/node-test-utils.h"
@@ -22,8 +23,12 @@ class RedundancyEliminationTest : public GraphTest {
  public:
   explicit RedundancyEliminationTest(int num_parameters = 4)
       : GraphTest(num_parameters),
-        reducer_(&editor_, zone()),
+        javascript_(zone()),
-        simplified_(zone()) {
+        simplified_(zone()),
+        machine_(zone()),
+        jsgraph_(isolate(), graph(), common(), &javascript_, &simplified_,
+                 &machine_),
+        reducer_(&editor_, &jsgraph_, zone()) {
     // Initialize the {reducer_} state for the Start node.
     reducer_.Reduce(graph()->start());
 
@@ -51,8 +56,11 @@ class RedundancyEliminationTest : public GraphTest {
   NiceMock<MockAdvancedReducerEditor> editor_;
   std::vector<FeedbackSource> vector_slot_pairs_;
   FeedbackSource feedback2_;
-  RedundancyElimination reducer_;
+  JSOperatorBuilder javascript_;
   SimplifiedOperatorBuilder simplified_;
+  MachineOperatorBuilder machine_;
+  JSGraph jsgraph_;
+  RedundancyElimination reducer_;
 };
 
 namespace {