From ee76e01efc88db29e1eefdeed9c29cd441c92493 Mon Sep 17 00:00:00 2001
From: Benjamin Bouvier <public@benj.me>
Date: Wed, 19 Aug 2020 14:52:42 +0200
Subject: [PATCH] machinst: fix the pinned reg hack;

The pinned register hack didn't work because the GetPinnedReg is marked
as having side-effects, so that GVN wouldn't try to common it out.

This commit tweaks the function used during lowering to vcode, so that
the GetPinnedReg opcode is specially handled. It's a bit lame, but it
makes the hack work again.

Also, use_input needs to be a no-op for real registers.
---
 cranelift/codegen/src/inst_predicates.rs |  8 +++++---
 cranelift/codegen/src/machinst/lower.rs  | 12 +++++++-----
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/cranelift/codegen/src/inst_predicates.rs b/cranelift/codegen/src/inst_predicates.rs
index 9afe2ff6e1..6830fcda1c 100644
--- a/cranelift/codegen/src/inst_predicates.rs
+++ b/cranelift/codegen/src/inst_predicates.rs
@@ -41,9 +41,11 @@ pub fn has_side_effect(func: &Function, inst: Inst) -> bool {
     trivially_has_side_effects(opcode) || is_load_with_defined_trapping(opcode, data)
 }
 
-/// Does the given instruction have any side-effect as per [has_side_effect], or else is a load?
-pub fn has_side_effect_or_load(func: &Function, inst: Inst) -> bool {
-    has_side_effect(func, inst) || func.dfg[inst].opcode().can_load()
+/// Does the given instruction have any side-effect as per [has_side_effect], or else is a load,
+/// but not the get_pinned_reg opcode?
+pub fn has_side_effect_or_load_not_get_pinned_reg(func: &Function, inst: Inst) -> bool {
+    let op = func.dfg[inst].opcode();
+    op != Opcode::GetPinnedReg && (has_side_effect(func, inst) || op.can_load())
 }
 
 /// Is the given instruction a constant value (`iconst`, `fconst`, `bconst`) that can be
diff --git a/cranelift/codegen/src/machinst/lower.rs b/cranelift/codegen/src/machinst/lower.rs
index c4cbbd820d..b421f79254 100644
--- a/cranelift/codegen/src/machinst/lower.rs
+++ b/cranelift/codegen/src/machinst/lower.rs
@@ -4,7 +4,7 @@
 
 use crate::entity::SecondaryMap;
 use crate::fx::{FxHashMap, FxHashSet};
-use crate::inst_predicates::{has_side_effect_or_load, is_constant_64bit};
+use crate::inst_predicates::{has_side_effect_or_load_not_get_pinned_reg, is_constant_64bit};
 use crate::ir::instructions::BranchInfo;
 use crate::ir::types::I64;
 use crate::ir::{
@@ -372,7 +372,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
         for bb in f.layout.blocks() {
             cur_color += 1;
             for inst in f.layout.block_insts(bb) {
-                let side_effect = has_side_effect_or_load(f, inst);
+                let side_effect = has_side_effect_or_load_not_get_pinned_reg(f, inst);
 
                 // Assign colors. A new color is chosen *after* any side-effecting instruction.
                 inst_colors[inst] = InstColor::new(cur_color);
@@ -800,14 +800,14 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
                 debug!(" -> src inst {}", src_inst);
                 debug!(
                     " -> has side effect: {}",
-                    has_side_effect_or_load(self.f, src_inst)
+                    has_side_effect_or_load_not_get_pinned_reg(self.f, src_inst)
                 );
                 debug!(
                     " -> our color is {:?}, src inst is {:?}",
                     self.inst_color(at_inst),
                     self.inst_color(src_inst)
                 );
-                if !has_side_effect_or_load(self.f, src_inst)
+                if !has_side_effect_or_load_not_get_pinned_reg(self.f, src_inst)
                     || self.inst_color(at_inst) == self.inst_color(src_inst)
                 {
                     Some((src_inst, result_idx))
@@ -989,7 +989,9 @@ impl<'func, I: VCodeInst> LowerCtx for Lower<'func, I> {
 
     fn use_input_reg(&mut self, input: LowerInput) {
         debug!("use_input_reg: vreg {:?} is needed", input.reg);
-        self.vreg_needed[input.reg.get_index()] = true;
+        if input.reg.is_virtual() {
+            self.vreg_needed[input.reg.get_index()] = true;
+        }
     }
 
     fn is_reg_needed(&self, ir_inst: Inst, reg: Reg) -> bool {