Augment jump threading to handle redundant returns

Although the code generator already combines return instructions late
in the pipeline into a common site, there were still superfluous jumps
to that common site left in the code.

Change-Id: I06c885fb0ab6a2c078f9dabdc6616c6881f42c75
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2284984
Commit-Queue: Daniel Clifford <danno@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69133}

src/compiler/backend/instruction.h

@@ -1457,6 +1457,7 @@ class V8_EXPORT_PRIVATE InstructionBlock final
 
   bool must_deconstruct_frame() const { return must_deconstruct_frame_; }
   void mark_must_deconstruct_frame() { must_deconstruct_frame_ = true; }
+  void clear_must_deconstruct_frame() { must_deconstruct_frame_ = false; }
 
  private:
   Successors successors_;

src/compiler/backend/jump-threading.cc

@@ -75,6 +75,10 @@ bool JumpThreading::ComputeForwarding(Zone* local_zone,
   ZoneStack<RpoNumber> stack(local_zone);
   JumpThreadingState state = {false, *result, stack};
   state.Clear(code->InstructionBlockCount());
+  RpoNumber empty_deconstruct_frame_return_block = RpoNumber::Invalid();
+  int32_t empty_deconstruct_frame_return_size;
+  RpoNumber empty_no_deconstruct_frame_return_block = RpoNumber::Invalid();
+  int32_t empty_no_deconstruct_frame_return_size;
 
   // Iterate over the blocks forward, pushing the blocks onto the stack.
   for (auto const block : code->instruction_blocks()) {
@@ -117,6 +121,44 @@ bool JumpThreading::ComputeForwarding(Zone* local_zone,
               fw = code->InputRpo(instr, 0);
             }
             fallthru = false;
+          } else if (instr->IsRet()) {
+            TRACE("  ret\n");
+            if (fallthru) {
+              CHECK_IMPLIES(block->must_construct_frame(),
+                            block->must_deconstruct_frame());
+              // Only handle returns with immediate/constant operands, since
+              // they must always be the same for all returns in a function.
+              // Dynamic return values might use different registers at
+              // different return sites and therefore cannot be shared.
+              if (instr->InputAt(0)->IsImmediate()) {
+                int32_t return_size =
+                    ImmediateOperand::cast(instr->InputAt(0))->inline_value();
+                // Instructions can be shared only for blocks that share
+                // the same |must_deconstruct_frame| attribute.
+                if (block->must_deconstruct_frame()) {
+                  if (empty_deconstruct_frame_return_block ==
+                      RpoNumber::Invalid()) {
+                    empty_deconstruct_frame_return_block = block->rpo_number();
+                    empty_deconstruct_frame_return_size = return_size;
+                  } else if (empty_deconstruct_frame_return_size ==
+                             return_size) {
+                    fw = empty_deconstruct_frame_return_block;
+                    block->clear_must_deconstruct_frame();
+                  }
+                } else {
+                  if (empty_no_deconstruct_frame_return_block ==
+                      RpoNumber::Invalid()) {
+                    empty_no_deconstruct_frame_return_block =
+                        block->rpo_number();
+                    empty_no_deconstruct_frame_return_size = return_size;
+                  } else if (empty_no_deconstruct_frame_return_size ==
+                             return_size) {
+                    fw = empty_no_deconstruct_frame_return_block;
+                  }
+                }
+              }
+            }
+            fallthru = false;
           } else {
             // can't skip other instructions.
             TRACE("  other\n");
@@ -185,7 +227,8 @@ void JumpThreading::ApplyForwarding(Zone* local_zone,
       FlagsMode mode = FlagsModeField::decode(instr->opcode());
       if (mode == kFlags_branch || mode == kFlags_branch_and_poison) {
         fallthru = false;  // branches don't fall through to the next block.
-      } else if (instr->arch_opcode() == kArchJmp) {
+      } else if (instr->arch_opcode() == kArchJmp ||
+                 instr->arch_opcode() == kArchRet) {
         if (skip[block_num]) {
           // Overwrite a redundant jump with a nop.
           TRACE("jt-fw nop @%d\n", i);

test/cctest/compiler/test-jump-threading.cc

@@ -50,6 +50,15 @@ class TestCode : public HandleAndZoneScope {
     End();
     return pos;
   }
+  int Return(int size, bool defer = false, bool deconstruct_frame = false) {
+    Start(defer, deconstruct_frame);
+    InstructionOperand ops[] = {Immediate(size)};
+    sequence_.AddInstruction(Instruction::New(main_zone(), kArchRet, 0, nullptr,
+                                              1, ops, 0, nullptr));
+    int pos = static_cast<int>(sequence_.instructions().size() - 1);
+    End();
+    return pos;
+  }
   void Nop() {
     Start();
     sequence_.AddInstruction(Instruction::New(main_zone(), kArchNop));
@@ -88,11 +97,17 @@ class TestCode : public HandleAndZoneScope {
   InstructionOperand UseRpo(int num) {
     return sequence_.AddImmediate(Constant(RpoNumber::FromInt(num)));
   }
-  void Start(bool deferred = false) {
+  InstructionOperand Immediate(int num) {
+    return sequence_.AddImmediate(Constant(num));
+  }
+  void Start(bool deferred = false, bool deconstruct_frame = false) {
     if (current_ == nullptr) {
       current_ = main_zone()->New<InstructionBlock>(
           main_zone(), rpo_number_, RpoNumber::Invalid(), RpoNumber::Invalid(),
           RpoNumber::Invalid(), deferred, false);
+      if (deconstruct_frame) {
+        current_->mark_must_deconstruct_frame();
+      }
       blocks_.push_back(current_);
       sequence_.StartBlock(rpo_number_);
     }
@@ -628,6 +643,14 @@ void CheckJump(TestCode* code, int pos, int target) {
   CHECK_EQ(target, code->sequence_.InputRpo(instr, 0).ToInt());
 }
 
+void CheckRet(TestCode* code, int pos) {
+  Instruction* instr = code->sequence_.InstructionAt(pos);
+  CHECK_EQ(kArchRet, instr->arch_opcode());
+  CHECK_EQ(1, static_cast<int>(instr->InputCount()));
+  CHECK_EQ(0, static_cast<int>(instr->OutputCount()));
+  CHECK_EQ(0, static_cast<int>(instr->TempCount()));
+}
+
 void CheckNop(TestCode* code, int pos) {
   Instruction* instr = code->sequence_.InstructionAt(pos);
   CHECK_EQ(kArchNop, instr->arch_opcode());
@@ -763,6 +786,86 @@ TEST(Rewire_diamond) {
   }
 }
 
+TEST(RewireRet) {
+  TestCode code;
+
+  // B0
+  code.Branch(1, 2);
+  // B1
+  int j1 = code.Return(0);
+  // B2
+  int j2 = code.Return(0);
+  // B3
+  code.End();
+
+  int forward[] = {0, 1, 1, 3};
+  VerifyForwarding(&code, 4, forward);
+  ApplyForwarding(&code, 4, forward);
+
+  CheckRet(&code, j1);
+  CheckNop(&code, j2);
+}
+
+TEST(RewireRet1) {
+  TestCode code;
+
+  // B0
+  code.Branch(1, 2);
+  // B1
+  int j1 = code.Return(0);
+  // B2
+  int j2 = code.Return(0, true, true);
+  // B3
+  code.End();
+
+  int forward[] = {0, 1, 2, 3};
+  VerifyForwarding(&code, 4, forward);
+  ApplyForwarding(&code, 4, forward);
+
+  CheckRet(&code, j1);
+  CheckRet(&code, j2);
+}
+
+TEST(RewireRet2) {
+  TestCode code;
+
+  // B0
+  code.Branch(1, 2);
+  // B1
+  int j1 = code.Return(0, true, true);
+  // B2
+  int j2 = code.Return(0, true, true);
+  // B3
+  code.End();
+
+  int forward[] = {0, 1, 1, 3};
+  VerifyForwarding(&code, 4, forward);
+  ApplyForwarding(&code, 4, forward);
+
+  CheckRet(&code, j1);
+  CheckNop(&code, j2);
+}
+
+TEST(DifferentSizeRet) {
+  TestCode code;
+
+  // B0
+  code.Branch(1, 2);
+  // B1
+  int j1 = code.Return(0);
+  // B2
+  int j2 = code.Return(1);
+  // B3
+  code.End();
+
+  int forward[] = {0, 1, 2, 3};
+  VerifyForwarding(&code, 4, forward);
+  ApplyForwarding(&code, 4, forward);
+
+  CheckRet(&code, j1);
+  CheckRet(&code, j2);
+}
+
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8