Don't relax a branch to have different input constraints.

When relaxing a branch, restrict the set of candidate encodings to those which
have the same input constraints as the original encoding choice. This prevents
situations where relaxation prefers a non-REX-prefixed encoding over a REX
prefixed one because the end of the instruction can be one byte closer to the
destination, in a situation where the encoding needs to be REX-prefixed
because of one of the operand registers.

This also makes the Context class perform encoding verification after
relaxation, to catch similar problems in the future.

Fixes #256.

lib/cretonne/src/binemit/relaxation.rs

@@ -152,13 +152,23 @@ fn relax_branch(
     if let Some(enc) = isa.legal_encodings(dfg, &dfg[inst], ctrl_type).find(
         |&enc| {
             let range = encinfo.branch_range(enc).expect("Branch with no range");
-            let in_range = range.contains(offset, dest_offset);
+            if !range.contains(offset, dest_offset) {
-            dbg!(
+                dbg!("  trying [{}]: out of range", encinfo.display(enc));
-                "  trying [{}]: {}",
+                false
-                encinfo.display(enc),
+            } else if encinfo.operand_constraints(enc) !=
-                if in_range { "OK" } else { "out of range" }
+                       encinfo.operand_constraints(cur.func.encodings[inst])
-            );
+            {
-            in_range
+                // Conservatively give up if the encoding has different constraints
+                // than the original, so that we don't risk picking a new encoding
+                // which the existing operands don't satisfy. We can't check for
+                // validity directly because we don't have a RegDiversions active so
+                // we don't know which registers are actually in use.
+                dbg!("  trying [{}]: constraints differ", encinfo.display(enc));
+                false
+            } else {
+                dbg!("  trying [{}]: OK", encinfo.display(enc));
+                true
+            }
         },
     )
     {

lib/cretonne/src/context.rs

@@ -131,6 +131,20 @@ impl Context {
         }
     }
 
+    /// Run the locations verifier on the function.
+    pub fn verify_locations<'a>(&self, isa: &TargetIsa) -> verifier::Result {
+        verifier::verify_locations(isa, &self.func, None)
+    }
+
+    /// Run the locations verifier only if the `enable_verifier` setting is true.
+    pub fn verify_locations_if<'a>(&self, isa: &TargetIsa) -> CtonResult {
+        if isa.flags().enable_verifier() {
+            self.verify_locations(isa).map_err(Into::into)
+        } else {
+            Ok(())
+        }
+    }
+
     /// Perform pre-legalization rewrites on the function.
     pub fn preopt(&mut self, isa: &TargetIsa) -> CtonResult {
         do_preopt(&mut self.func);
@@ -212,13 +226,16 @@ impl Context {
     /// Insert prologue and epilogues after computing the stack frame layout.
     pub fn prologue_epilogue(&mut self, isa: &TargetIsa) -> CtonResult {
         isa.prologue_epilogue(&mut self.func)?;
-        self.verify_if(isa)
+        self.verify_if(isa)?;
+        self.verify_locations_if(isa)?;
+        Ok(())
     }
 
     /// Run the branch relaxation pass and return the final code size.
     pub fn relax_branches(&mut self, isa: &TargetIsa) -> Result<CodeOffset, CtonError> {
         let code_size = relax_branches(&mut self.func, isa)?;
         self.verify_if(isa)?;
+        self.verify_locations_if(isa)?;
 
         Ok(code_size)
     }

lib/cretonne/src/isa/constraints.rs

@@ -13,6 +13,7 @@ use ir::{Function, ValueLoc, Inst};
 use regalloc::RegDiversions;
 
 /// Register constraint for a single value operand or instruction result.
+#[derive(PartialEq, Debug)]
 pub struct OperandConstraint {
     /// The kind of constraint.
     pub kind: ConstraintKind,
@@ -53,7 +54,7 @@ impl OperandConstraint {
 }
 
 /// The different kinds of operand constraints.
-#[derive(Clone, Copy, PartialEq, Eq)]
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
 pub enum ConstraintKind {
     /// This operand or result must be a register from the given register class.
     Reg,
@@ -89,7 +90,7 @@ pub enum ConstraintKind {
 }
 
 /// Value operand constraints for an encoding recipe.
-#[derive(Clone)]
+#[derive(PartialEq, Clone)]
 pub struct RecipeConstraints {
     /// Constraints for the instruction's fixed value operands.
     ///
@@ -160,7 +161,7 @@ impl RecipeConstraints {
 /// - Intel uses the address of the instruction following the branch, `origin = 2` for a 2-byte
 ///   branch instruction.
 /// - ARM's A32 encoding uses the address of the branch instruction + 8 bytes, `origin = 8`.
-#[derive(Clone, Copy)]
+#[derive(Clone, Copy, Debug)]
 pub struct BranchRange {
     /// Offset in bytes from the address of the branch instruction to the origin used for computing
     /// the branch displacement. This is the destination of a branch that encodes a 0 displacement.