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

@@ -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);

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));
             }

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
+}