From 5a23f975fccaadef1cd132fc65dc750e3d37e19a Mon Sep 17 00:00:00 2001
From: Jakob Stoklund Olesen <jolesen@mozilla.com>
Date: Tue, 13 Jun 2017 15:43:14 -0700
Subject: [PATCH] Extract spill insertion into a reload::insert_spill function.

Make sure that spill instructions are generated in the same way
everywhere, including adding encoding and updating live ranges.
---
 filetests/regalloc/spill.cton       | 13 ++++++-
 lib/cretonne/src/regalloc/reload.rs | 54 +++++++++++++++++++++--------
 2 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/filetests/regalloc/spill.cton b/filetests/regalloc/spill.cton
index d3da74ddf0..a43ba4cf8f 100644
--- a/filetests/regalloc/spill.cton
+++ b/filetests/regalloc/spill.cton
@@ -17,6 +17,7 @@ isa riscv enable_e
 ; That is in order:
 ; 1. The argument v1.
 ; 2. The link register.
+; 3. The first computed value, v2
 function %pyramid(i32) -> i32 {
 ebb0(v1: i32):
 ; check: $ebb0($(rv1=$V): i32, $(rlink=$V): i32)
@@ -24,6 +25,9 @@ ebb0(v1: i32):
     ; nextln: $(link=$V) = spill $rlink
     ; not: spill
     v2 = iadd_imm v1, 12
+    ; check: $(r1v2=$V) = iadd_imm
+    ; nextln: $v2 = spill $r1v2
+    ; not: spill
     v3 = iadd_imm v2, 12
     v4 = iadd_imm v3, 12
     v5 = iadd_imm v4, 12
@@ -35,7 +39,10 @@ ebb0(v1: i32):
     v11 = iadd_imm v10, 12
     v12 = iadd_imm v11, 12
     v13 = iadd_imm v12, 12
-    v32 = iadd v12, v13
+    v14 = iadd_imm v13, 12
+    v33 = iadd v13, v14
+    ; check: iadd $v13
+    v32 = iadd v33, v12
     v31 = iadd v32, v11
     v30 = iadd v31, v10
     v29 = iadd v30, v9
@@ -46,7 +53,11 @@ ebb0(v1: i32):
     v24 = iadd v25, v4
     v23 = iadd v24, v3
     v22 = iadd v23, v2
+    ; check: $(r2v2=$V) = fill $v2
+    ; check: $v22 = iadd $v23, $r2v2
     v21 = iadd v22, v1
+    ; check: $(r2v1=$V) = fill $v1
+    ; check: $v21 = iadd $v22, $r2v1
     ; check: $(rlink2=$V) = fill $link
     return v21
     ; check: return $v21, $rlink2
diff --git a/lib/cretonne/src/regalloc/reload.rs b/lib/cretonne/src/regalloc/reload.rs
index 9786f45b67..dd623de2b5 100644
--- a/lib/cretonne/src/regalloc/reload.rs
+++ b/lib/cretonne/src/regalloc/reload.rs
@@ -13,7 +13,7 @@ use dominator_tree::DominatorTree;
 use entity_map::EntityMap;
 use ir::{Ebb, Inst, Value, Function, Signature, DataFlowGraph};
 use ir::layout::{Cursor, CursorPosition};
-use ir::{InstBuilder, ArgumentType, ArgumentLoc};
+use ir::{InstBuilder, Opcode, ArgumentType, ArgumentLoc};
 use isa::RegClass;
 use isa::{TargetIsa, Encoding, EncInfo, RecipeConstraints, ConstraintKind};
 use regalloc::affinity::Affinity;
@@ -167,16 +167,14 @@ impl<'a> Context<'a> {
                         // An incoming register parameter was spilled. Replace the parameter value
                         // with a temporary register value that is immediately spilled.
                         let reg = func.dfg.replace_ebb_arg(arg.value, abi.value_type);
-                        func.dfg.ins(&mut pos).with_result(arg.value).spill(reg);
-                        let spill = func.dfg.value_def(arg.value).unwrap_inst();
-                        *func.encodings.ensure(spill) = self.isa
-                            .encode(&func.dfg, &func.dfg[spill], abi.value_type)
-                            .expect("Can't encode spill");
-                        // Update live ranges.
-                        self.liveness.move_def_locally(arg.value, spill);
                         let affinity = Affinity::abi(abi, self.isa);
                         self.liveness.create_dead(reg, ebb, affinity);
-                        self.liveness.extend_locally(reg, ebb, spill, pos.layout);
+                        self.insert_spill(ebb,
+                                          arg.value,
+                                          reg,
+                                          &mut pos,
+                                          &mut func.encodings,
+                                          &mut func.dfg);
                     }
                 }
                 ArgumentLoc::Stack(_) => {
@@ -270,13 +268,8 @@ impl<'a> Context<'a> {
             if lv.affinity.is_stack() && op.kind != ConstraintKind::Stack {
                 let value_type = dfg.value_type(lv.value);
                 let reg = dfg.replace_result(lv.value, value_type);
-                dfg.ins(pos).with_result(lv.value).spill(reg);
-                let spill = dfg.value_def(lv.value).unwrap_inst();
-
-                // Create a live range for reg.
                 self.liveness.create_dead(reg, inst, Affinity::new(op));
-                self.liveness.extend_locally(reg, ebb, spill, &pos.layout);
-                self.liveness.move_def_locally(lv.value, spill);
+                self.insert_spill(ebb, lv.value, reg, pos, encodings, dfg);
             }
         }
     }
@@ -337,4 +330,35 @@ impl<'a> Context<'a> {
             }
         }
     }
+
+    /// Insert a spill at `pos` and update data structures.
+    ///
+    /// - Insert `stack = spill reg` at `pos`, and assign an encoding.
+    /// - Move the `stack` live range starting point to the new instruction.
+    /// - Extend the `reg` live range to reach the new instruction.
+    fn insert_spill(&mut self,
+                    ebb: Ebb,
+                    stack: Value,
+                    reg: Value,
+                    pos: &mut Cursor,
+                    encodings: &mut EntityMap<Inst, Encoding>,
+                    dfg: &mut DataFlowGraph) {
+        let ty = dfg.value_type(reg);
+
+        // Insert spill instruction. Use the low-level `Unary` constructor because it returns an
+        // instruction reference directly rather than a result value (which we know is equal to
+        // `stack`).
+        let (inst, _) = dfg.ins(pos)
+            .with_result(stack)
+            .Unary(Opcode::Spill, ty, reg);
+
+        // Give it an encoding.
+        *encodings.ensure(inst) = self.isa
+            .encode(dfg, &dfg[inst], ty)
+            .expect("Can't encode spill");
+
+        // Update live ranges.
+        self.liveness.move_def_locally(stack, inst);
+        self.liveness.extend_locally(reg, ebb, inst, pos.layout);
+    }
 }