From b4c6bfd371fe6bdead48ce80bd135233f96243f6 Mon Sep 17 00:00:00 2001
From: Sean Stangl <sean.stangl@gmail.com>
Date: Wed, 22 Jan 2020 09:14:41 -0700
Subject: [PATCH] When splitting a const, insert prior to the terminal branch
 group. (#1325)

* When splitting a const, insert prior to the terminal branch group. Closes #1159

Given code like the following, on x86_64, which does not have i128 registers:

    ebb0(v0: i64):
        v1 = iconst.i128 0
        v2 = icmp_imm eq v0, 1
        brnz v2, ebb1
        jump ebb2(v1)

It would be split to:

    ebb0(v0: i64):
        v1 = iconst.i128 0
        v2 = icmp_imm eq v0, 1
        brnz v2, ebb1
        v3, v4 = isplit.i128 v1
        jump ebb2(v3, v4)

But that fails basic-block invariants. This patch changes that to:

    ebb0(v0: i64):
        v1 = iconst.i128 0
        v2 = icmp_imm eq v0, 1
        v3, v4 = isplit.i128 v1
        brnz v2, ebb1
        jump ebb2(v3, v4)

* Add isplit-bb.clif testcase
---
 cranelift/codegen/src/ir/layout.rs            | 14 ++++++++++
 cranelift/codegen/src/legalizer/split.rs      | 15 ++++++++--
 .../filetests/verifier/isplit-bb.clif         | 28 +++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)
 create mode 100644 cranelift/filetests/filetests/verifier/isplit-bb.clif

diff --git a/cranelift/codegen/src/ir/layout.rs b/cranelift/codegen/src/ir/layout.rs
index 7b624a92d2..b090b8d819 100644
--- a/cranelift/codegen/src/ir/layout.rs
+++ b/cranelift/codegen/src/ir/layout.rs
@@ -4,6 +4,7 @@
 //! determined by the `Layout` data structure defined in this module.
 
 use crate::entity::SecondaryMap;
+use crate::ir::dfg::DataFlowGraph;
 use crate::ir::progpoint::{ExpandedProgramPoint, ProgramOrder};
 use crate::ir::{Ebb, Inst};
 use crate::packed_option::PackedOption;
@@ -573,6 +574,19 @@ impl Layout {
         self.insts[inst].prev.expand()
     }
 
+    /// Fetch the first instruction in an ebb's terminal branch group.
+    pub fn canonical_branch_inst(&self, dfg: &DataFlowGraph, ebb: Ebb) -> Option<Inst> {
+        // Basic blocks permit at most two terminal branch instructions.
+        // If two, the former is conditional and the latter is unconditional.
+        let last = self.last_inst(ebb)?;
+        if let Some(prev) = self.prev_inst(last) {
+            if dfg[prev].opcode().is_branch() {
+                return Some(prev);
+            }
+        }
+        Some(last)
+    }
+
     /// Insert `inst` before the instruction `before` in the same EBB.
     pub fn insert_inst(&mut self, inst: Inst, before: Inst) {
         debug_assert_eq!(self.inst_ebb(inst), None);
diff --git a/cranelift/codegen/src/legalizer/split.rs b/cranelift/codegen/src/legalizer/split.rs
index a3fa29f267..405b18e4f7 100644
--- a/cranelift/codegen/src/legalizer/split.rs
+++ b/cranelift/codegen/src/legalizer/split.rs
@@ -189,6 +189,17 @@ fn perform_repairs(pos: &mut FuncCursor, cfg: &ControlFlowGraph, mut repairs: Ve
 
             // Split the old argument, possibly causing more repairs to be scheduled.
             pos.goto_inst(inst);
+            #[cfg(feature = "basic-blocks")]
+            {
+                let inst_ebb = pos.func.layout.inst_ebb(inst).expect("inst in ebb");
+
+                // Insert split values prior to the terminal branch group.
+                let dfg = &pos.func.dfg;
+                let canonical = pos.func.layout.canonical_branch_inst(dfg, inst_ebb);
+                if let Some(first_branch) = canonical {
+                    pos.goto_inst(first_branch);
+                }
+            }
             let (lo, hi) = split_value(pos, old_arg, repair.concat, &mut repairs);
 
             // The `lo` part replaces the original argument.
@@ -248,8 +259,8 @@ fn split_value(
             }
         }
         ValueDef::Param(ebb, num) => {
-            // This is an EBB parameter. We can split the parameter value unless this is the entry
-            // block.
+            // This is an EBB parameter.
+            // We can split the parameter value unless this is the entry block.
             if pos.func.layout.entry_block() != Some(ebb) {
                 reuse = Some(split_ebb_param(pos, ebb, num, value, concat, repairs));
             }
diff --git a/cranelift/filetests/filetests/verifier/isplit-bb.clif b/cranelift/filetests/filetests/verifier/isplit-bb.clif
new file mode 100644
index 0000000000..ba789706a1
--- /dev/null
+++ b/cranelift/filetests/filetests/verifier/isplit-bb.clif
@@ -0,0 +1,28 @@
+test compile
+target x86_64
+
+function u0:0(i128, i128, i64) -> i128 system_v {
+    ebb0(v0: i128, v1: i128, v2: i64):
+        trap user0
+
+    ebb1:
+        v10 = iconst.i64 0
+        v11 = iconst.i64 0
+        v17 = iconcat v10, v11
+        v12 = iconst.i64 0
+        v13 = iconst.i64 0 
+        v20 = iconcat v12, v13
+        trap user0
+
+    ebb79:
+        v425 = iconst.i64 0
+        v426 = icmp_imm eq v425, 1
+        brnz v426, ebb80
+        jump ebb85(v20, v17)
+
+    ebb80:
+        trap user0
+
+    ebb85(v462: i128, v874: i128):
+        trap user0
+}