chore: cherry-pick 13ffdf63a471 from v8 (#34881)

* chore: [18-x-y] cherry-pick 13ffdf63a471 from v8 * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
electron · Jul 12, 2022 · ecb9afd · ecb9afd
1 parent a70431f
commit ecb9afd
Show file tree

Hide file tree

Showing 2 changed files with 170 additions and 0 deletions.
diff --git a/patches/v8/.patches b/patches/v8/.patches
@@ -10,3 +10,4 @@ revert_fix_cppgc_removed_deleted_cstors_in_cppheapcreateparams.patch
 cherry-pick-723ed8a9cfff.patch
 cherry-pick-44c4e56fea2c.patch
 version_10_2_154_10_cherry-pick.patch
+cherry-pick-13ffdf63a471.patch
diff --git a/patches/v8/cherry-pick-13ffdf63a471.patch b/patches/v8/cherry-pick-13ffdf63a471.patch
@@ -0,0 +1,169 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Tobias Tebbi <tebbi@chromium.org>
+Date: Fri, 17 Jun 2022 10:14:44 +0000
+Subject: Merged: [compiler] make CanCover() transitive
+
+In addition to checking that a node is owned, CanCover() also needs to
+check if there are any side-effects in between the current node and
+the merged node. When merging inputs of inputs, this check was done
+with the wrong side-effect level of the in-between node.
+We partially fixed this before with `CanCoverTransitively`.
+This CL addresses the issue by always comparing to the side-effect
+level of the node from which we started, making `CanCoverTransitively`
+superfluous.
+
+Bug: chromium:1336869
+(cherry picked from commit 6048f754931e0971cab58fb0de785482d175175b)
+
+Change-Id: I63cf6bfdd40c2c55921db4a2ac737612bb7da4e4
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3722095
+Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
+Cr-Commit-Position: refs/branch-heads/10.2@{#19}
+Cr-Branched-From: 374091f382e88095694c1283cbdc2acddc1b1417-refs/heads/10.2.154@{#1}
+Cr-Branched-From: f0c353f6315eeb2212ba52478983a3b3af07b5b1-refs/heads/main@{#79976}
+
+diff --git a/src/compiler/backend/instruction-selector.cc b/src/compiler/backend/instruction-selector.cc
+index 528aa543688041b816087efa02dec81ad8c9e3b1..b0e589738a0a1d7fb9d7f8eeb7677a5d7d0df022 100644
+--- a/src/compiler/backend/instruction-selector.cc
++++ b/src/compiler/backend/instruction-selector.cc
+@@ -283,7 +283,7 @@ Instruction* InstructionSelector::Emit(Instruction* instr) {
+
+ bool InstructionSelector::CanCover(Node* user, Node* node) const {
+   // 1. Both {user} and {node} must be in the same basic block.
+-  if (schedule()->block(node) != schedule()->block(user)) {
++  if (schedule()->block(node) != current_block_) {
+     return false;
+   }
+   // 2. Pure {node}s must be owned by the {user}.
+@@ -291,7 +291,7 @@ bool InstructionSelector::CanCover(Node* user, Node* node) const {
+     return node->OwnedBy(user);
+   }
+   // 3. Impure {node}s must match the effect level of {user}.
+-  if (GetEffectLevel(node) != GetEffectLevel(user)) {
++  if (GetEffectLevel(node) != current_effect_level_) {
+     return false;
+   }
+   // 4. Only {node} must have value edges pointing to {user}.
+@@ -303,21 +303,6 @@ bool InstructionSelector::CanCover(Node* user, Node* node) const {
+   return true;
+ }
+
+-bool InstructionSelector::CanCoverTransitively(Node* user, Node* node,
+-                                               Node* node_input) const {
+-  if (CanCover(user, node) && CanCover(node, node_input)) {
+-    // If {node} is pure, transitivity might not hold.
+-    if (node->op()->HasProperty(Operator::kPure)) {
+-      // If {node_input} is pure, the effect levels do not matter.
+-      if (node_input->op()->HasProperty(Operator::kPure)) return true;
+-      // Otherwise, {user} and {node_input} must have the same effect level.
+-      return GetEffectLevel(user) == GetEffectLevel(node_input);
+-    }
+-    return true;
+-  }
+-  return false;
+-}
+-
+ bool InstructionSelector::IsOnlyUserOfNodeInSameBlock(Node* user,
+                                                       Node* node) const {
+   BasicBlock* bb_user = schedule()->block(user);
+@@ -1206,6 +1191,7 @@ void InstructionSelector::VisitBlock(BasicBlock* block) {
+   int effect_level = 0;
+   for (Node* const node : *block) {
+     SetEffectLevel(node, effect_level);
++    current_effect_level_ = effect_level;
+     if (node->opcode() == IrOpcode::kStore ||
+         node->opcode() == IrOpcode::kUnalignedStore ||
+         node->opcode() == IrOpcode::kCall ||
+@@ -1223,6 +1209,7 @@ void InstructionSelector::VisitBlock(BasicBlock* block) {
+   // control input should be on the same effect level as the last node.
+   if (block->control_input() != nullptr) {
+     SetEffectLevel(block->control_input(), effect_level);
++    current_effect_level_ = effect_level;
+   }
+
+   auto FinishEmittedInstructions = [&](Node* node, int instruction_start) {
+diff --git a/src/compiler/backend/instruction-selector.h b/src/compiler/backend/instruction-selector.h
+index b33de8e8569ebce59eebfec6eef12ed1f1874ddc..167cdc1f0c4962d55f3f9b40b7865737fc652bb0 100644
+--- a/src/compiler/backend/instruction-selector.h
++++ b/src/compiler/backend/instruction-selector.h
+@@ -423,15 +423,12 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
+   // Used in pattern matching during code generation.
+   // Check if {node} can be covered while generating code for the current
+   // instruction. A node can be covered if the {user} of the node has the only
+-  // edge and the two are in the same basic block.
+-  // Before fusing two instructions a and b, it is useful to check that
+-  // CanCover(a, b) holds. If this is not the case, code for b must still be
+-  // generated for other users, and fusing is unlikely to improve performance.
++  // edge, the two are in the same basic block, and there are no side-effects
++  // in-between. The last check is crucial for soundness.
++  // For pure nodes, CanCover(a,b) is checked to avoid duplicated execution:
++  // If this is not the case, code for b must still be generated for other
++  // users, and fusing is unlikely to improve performance.
+   bool CanCover(Node* user, Node* node) const;
+-  // CanCover is not transitive.  The counter example are Nodes A,B,C such that
+-  // CanCover(A, B) and CanCover(B,C) and B is pure: The the effect level of A
+-  // and B might differ. CanCoverTransitively does the additional checks.
+-  bool CanCoverTransitively(Node* user, Node* node, Node* node_input) const;
+
+   // Used in pattern matching during code generation.
+   // This function checks that {node} and {user} are in the same basic block,
+@@ -756,6 +753,7 @@ class V8_EXPORT_PRIVATE InstructionSelector final {
+   BoolVector defined_;
+   BoolVector used_;
+   IntVector effect_level_;
++  int current_effect_level_;
+   IntVector virtual_registers_;
+   IntVector virtual_register_rename_;
+   InstructionScheduler* scheduler_;
+diff --git a/src/compiler/backend/loong64/instruction-selector-loong64.cc b/src/compiler/backend/loong64/instruction-selector-loong64.cc
+index 4f03f99acd5b859b6fc095a34dea66ec058bcd4b..6b2d25f5dc1d912c1183e1b349bd29615024fa48 100644
+--- a/src/compiler/backend/loong64/instruction-selector-loong64.cc
++++ b/src/compiler/backend/loong64/instruction-selector-loong64.cc
+@@ -1446,7 +1446,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) {
+   if (CanCover(node, value)) {
+     switch (value->opcode()) {
+       case IrOpcode::kWord64Sar: {
+-        if (CanCoverTransitively(node, value, value->InputAt(0)) &&
++        if (CanCover(value, value->InputAt(0)) &&
+             TryEmitExtendingLoad(this, value, node)) {
+           return;
+         } else {
+diff --git a/src/compiler/backend/mips64/instruction-selector-mips64.cc b/src/compiler/backend/mips64/instruction-selector-mips64.cc
+index 4f5738ddaddba03aaa0ebbc351fe18d766e04d00..1e29e0ed730be49f4a09484d6c4db3ffca92355e 100644
+--- a/src/compiler/backend/mips64/instruction-selector-mips64.cc
++++ b/src/compiler/backend/mips64/instruction-selector-mips64.cc
+@@ -1532,7 +1532,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) {
+   if (CanCover(node, value)) {
+     switch (value->opcode()) {
+       case IrOpcode::kWord64Sar: {
+-        if (CanCoverTransitively(node, value, value->InputAt(0)) &&
++        if (CanCover(value, value->InputAt(0)) &&
+             TryEmitExtendingLoad(this, value, node)) {
+           return;
+         } else {
+diff --git a/src/compiler/backend/riscv64/instruction-selector-riscv64.cc b/src/compiler/backend/riscv64/instruction-selector-riscv64.cc
+index b6ced36f796b922df19e8bbb70db1bb6c33a2149..3b744795e5cd83ba4a8eff23cf4e607ffa0ca1c5 100644
+--- a/src/compiler/backend/riscv64/instruction-selector-riscv64.cc
++++ b/src/compiler/backend/riscv64/instruction-selector-riscv64.cc
+@@ -1428,7 +1428,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) {
+   if (CanCover(node, value)) {
+     switch (value->opcode()) {
+       case IrOpcode::kWord64Sar: {
+-        if (CanCoverTransitively(node, value, value->InputAt(0)) &&
++        if (CanCover(value, value->InputAt(0)) &&
+             TryEmitExtendingLoad(this, value, node)) {
+           return;
+         } else {
+diff --git a/src/compiler/backend/x64/instruction-selector-x64.cc b/src/compiler/backend/x64/instruction-selector-x64.cc
+index 3aec65221fb03e0d4143d08f5156b8a3f192e82c..5b8064010419d2749cddc0302ba63dc20f6e58df 100644
+--- a/src/compiler/backend/x64/instruction-selector-x64.cc
++++ b/src/compiler/backend/x64/instruction-selector-x64.cc
+@@ -1802,7 +1802,7 @@ void InstructionSelector::VisitTruncateInt64ToInt32(Node* node) {
+       case IrOpcode::kWord64Shr: {
+         Int64BinopMatcher m(value);
+         if (m.right().Is(32)) {
+-          if (CanCoverTransitively(node, value, value->InputAt(0)) &&
++          if (CanCover(value, value->InputAt(0)) &&
+               TryMatchLoadWord64AndShiftRight(this, value, kX64Movl)) {
+             return EmitIdentity(node);
+           }