From 0cb08095e6bc4c2c4dfb7e13863938f966bb5f76 Mon Sep 17 00:00:00 2001
From: Chris Fallin <chris@cfallin.org>
Date: Mon, 4 Apr 2022 16:22:05 -0700
Subject: [PATCH] Make multiple defs of one vreg possible on one instruction.
 (#38)

Currently, if this is done, overlapping liveranges are created, and we
hit an assert that ensures our non-overlapping and
built-in-reverse-order invariants during liverange construction.

One could argue that multiple defs of a single vreg don't make a ton of
sense -- which def's value is valid after the instruction? -- but if
they all get the same alloc, then the answer is "whatever was put in
that alloc", and this is just a case of an instruction being a bit
over-eager when listing its registers.

This can arise in practice when operand lists come from combinations or
concatenations: for example, in Cranelift's s390x backend, there is a
"Loop" pseudo-instruction, and the operands of the Loop are the operands
of all the sub-instructions.

It seems more logically cohesive overall to say that one can state an
operand as many times as one likes; so this PR makes it so.
---
 src/ion/liveranges.rs | 42 +++++++++++++++++++++++++++++-------------
 src/lib.rs            | 18 ++++++++++++++++++
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/src/ion/liveranges.rs b/src/ion/liveranges.rs
index 2aa7a7d..af99393 100644
--- a/src/ion/liveranges.rs
+++ b/src/ion/liveranges.rs
@@ -176,7 +176,11 @@ impl<'a, F: Function> Env<'a, F> {
     /// Mark `range` as live for the given `vreg`.
     ///
     /// Returns the liverange that contains the given range.
-    pub fn add_liverange_to_vreg(&mut self, vreg: VRegIndex, range: CodeRange) -> LiveRangeIndex {
+    pub fn add_liverange_to_vreg(
+        &mut self,
+        vreg: VRegIndex,
+        mut range: CodeRange,
+    ) -> LiveRangeIndex {
         trace!("add_liverange_to_vreg: vreg {:?} range {:?}", vreg, range);
 
         // Invariant: as we are building liveness information, we
@@ -190,18 +194,30 @@ impl<'a, F: Function> Env<'a, F> {
         // array, then reverse them at the end of
         // `compute_liveness()`.
 
-        debug_assert!(
-            self.vregs[vreg.index()].ranges.is_empty()
-                || range.to
-                    <= self.ranges[self.vregs[vreg.index()]
-                        .ranges
-                        .last()
-                        .unwrap()
-                        .index
-                        .index()]
-                    .range
-                    .from
-        );
+        if !self.vregs[vreg.index()].ranges.is_empty() {
+            let last_range_index = self.vregs[vreg.index()].ranges.last().unwrap().index;
+            let last_range = self.ranges[last_range_index.index()].range;
+            if self.func.allow_multiple_vreg_defs() {
+                if last_range.contains(&range) {
+                    // Special case (may occur when multiple defs of pinned
+                    // physical regs occur): if this new range overlaps the
+                    // existing range, return it.
+                    return last_range_index;
+                }
+                // If this range's end falls in the middle of the last
+                // range, truncate it to be contiguous so we can merge
+                // below.
+                if range.to >= last_range.from && range.to <= last_range.to {
+                    range.to = last_range.from;
+                }
+            }
+            debug_assert!(
+                range.to <= last_range.from,
+                "range {:?}, last_range {:?}",
+                range,
+                last_range
+            );
+        }
 
         if self.vregs[vreg.index()].ranges.is_empty()
             || range.to
diff --git a/src/lib.rs b/src/lib.rs
index ef350a0..b9edcf3 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1011,6 +1011,24 @@ pub trait Function {
     fn multi_spillslot_named_by_last_slot(&self) -> bool {
         false
     }
+
+    // -----------
+    // Misc config
+    // -----------
+
+    /// Allow a single instruction to define a vreg multiple times. If
+    /// allowed, the semantics are as if the definition occurs only
+    /// once, and all defs will get the same alloc. This flexibility is
+    /// meant to allow the embedder to more easily aggregate operands
+    /// together in macro/pseudoinstructions, or e.g. add additional
+    /// clobbered vregs without taking care to deduplicate. This may be
+    /// particularly useful when referring to physical registers via
+    /// pinned vregs. It is optional functionality because a strict mode
+    /// (at most one def per vreg) is also useful for finding bugs in
+    /// other applications.
+    fn allow_multiple_vreg_defs(&self) -> bool {
+        false
+    }
 }
 
 /// A position before or after an instruction at which we can make an