From 2f320e56a132f5d7d479e3a9f52a4992a38ee050 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <git@yorickpeterse.com>
Date: Thu, 4 Apr 2024 17:42:17 +0200
Subject: [PATCH] Don't generate goto's for dropping immediates

When "dropping" an immediate value such as an Int, we no longer generate
an explicit goto to connect the current and next basic block. By doing
so, the clean up pass simply removes the entire block if no other
instructions are in the block, preventing us from generating many basic
blocks that simply contain a goto to the next block.

While this doesn't really affect performance, it makes the LLVM IR
smaller and makes it easier to visualize the MIR IR.

Changelog: fixed
---
 compiler/src/mir/passes.rs     |  5 ++---
 compiler/src/mir/specialize.rs | 18 +++++++++---------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/compiler/src/mir/passes.rs b/compiler/src/mir/passes.rs
index 55c511546..6a66eebac 100644
--- a/compiler/src/mir/passes.rs
+++ b/compiler/src/mir/passes.rs
@@ -4407,9 +4407,8 @@ impl<'a> LowerMethod<'a> {
 /// 2. Basic blocks that implicitly flow into another block are updated to end
 ///    with a goto to said block.
 ///
-/// These changes make it a bit easier to generate code from a MIR graph, as the
-/// order in which we process blocks (besides starting with the root block) no
-/// longer matters. It also makes the graph less noisy when visualising it.
+/// These changes make it easier to visualize the MIR code, and result in a
+/// smaller IR to pass to LLVM.
 pub(crate) fn clean_up_basic_blocks(mir: &mut Mir) {
     for method in mir.methods.values_mut() {
         let blocks = &method.body.blocks;
diff --git a/compiler/src/mir/specialize.rs b/compiler/src/mir/specialize.rs
index 60870932f..c7533ca59 100644
--- a/compiler/src/mir/specialize.rs
+++ b/compiler/src/mir/specialize.rs
@@ -1128,7 +1128,7 @@ impl<'a, 'b, 'c> ExpandDrop<'a, 'b, 'c> {
 
         match typ.shape(self.db, self.shapes) {
             Shape::Int | Shape::Float | Shape::Nil | Shape::Boolean => {
-                self.ignore_value(block_id, after_id, loc);
+                self.ignore_value(block_id, after_id);
             }
             Shape::Mut | Shape::Ref => {
                 self.drop_reference(block_id, after_id, val, loc);
@@ -1137,7 +1137,7 @@ impl<'a, 'b, 'c> ExpandDrop<'a, 'b, 'c> {
                 self.drop_atomic(block_id, after_id, val, loc);
             }
             Shape::Owned if typ.is_permanent(self.db) => {
-                self.ignore_value(block_id, after_id, loc);
+                self.ignore_value(block_id, after_id);
             }
             Shape::Owned => {
                 self.drop_owned(block_id, after_id, val, ins.dropper, loc);
@@ -1145,13 +1145,13 @@ impl<'a, 'b, 'c> ExpandDrop<'a, 'b, 'c> {
         }
     }
 
-    fn ignore_value(
-        &mut self,
-        before_id: BlockId,
-        after_id: BlockId,
-        location: LocationId,
-    ) {
-        self.block_mut(before_id).goto(after_id, location);
+    fn ignore_value(&mut self, before_id: BlockId, after_id: BlockId) {
+        // We don't generate a goto() here because:
+        //
+        // 1. If there are other instructions in the current block, the cleanup
+        //    phase connects the current and next block explicitly for us.
+        // 2. If the current block is empty, this prevents a redundant basic
+        //    block that only contains a goto to the next block.
         self.method.body.add_edge(before_id, after_id);
     }