Merge pull request #2363 from cfallin/extend-only-if-abi

Do value-extensions at ABI boundaries only when ABI requires it.

cranelift/codegen/src/ir/extfunc.rs

@@ -256,8 +256,19 @@ impl fmt::Display for AbiParam {
 
 /// Function argument extension options.
 ///
-/// On some architectures, small integer function arguments are extended to the width of a
-/// general-purpose register.
+/// On some architectures, small integer function arguments and/or return values are extended to
+/// the width of a general-purpose register.
+///
+/// This attribute specifies how an argument or return value should be extended *if the platform
+/// and ABI require it*. Because the frontend (CLIF generator) does not know anything about the
+/// particulars of the target's ABI, and the CLIF should be platform-independent, these attributes
+/// specify *how* to extend (according to the signedness of the original program) rather than
+/// *whether* to extend.
+///
+/// For example, on x86-64, the SystemV ABI does not require extensions of narrow values, so these
+/// `ArgumentExtension` attributes are ignored; but in the Baldrdash (SpiderMonkey) ABI on the same
+/// platform, all narrow values *are* extended, so these attributes may lead to extra
+/// zero/sign-extend instructions in the generated machine code.
 #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash)]
 #[cfg_attr(feature = "enable-serde", derive(Serialize, Deserialize))]
 pub enum ArgumentExtension {

cranelift/codegen/src/isa/aarch64/abi.rs

@@ -728,6 +728,19 @@ impl ABIMachineSpec for AArch64MachineDeps {
         }
         caller_saved
     }
+
+    fn get_ext_mode(
+        call_conv: isa::CallConv,
+        specified: ir::ArgumentExtension,
+    ) -> ir::ArgumentExtension {
+        if call_conv.extends_baldrdash() {
+            // Baldrdash (SpiderMonkey) always extends args and return values to the full register.
+            specified
+        } else {
+            // No other supported ABI on AArch64 does so.
+            ir::ArgumentExtension::None
+        }
+    }
 }
 
 /// Is this type supposed to be seen on this machine? E.g. references of the

cranelift/codegen/src/isa/arm32/abi.rs

@@ -445,6 +445,13 @@ impl ABIMachineSpec for Arm32MachineDeps {
         }
         caller_saved
     }
+
+    fn get_ext_mode(
+        _call_conv: isa::CallConv,
+        specified: ir::ArgumentExtension,
+    ) -> ir::ArgumentExtension {
+        specified
+    }
 }
 
 fn is_callee_save(r: RealReg) -> bool {

cranelift/codegen/src/isa/x64/abi.rs

@@ -591,6 +591,19 @@ impl ABIMachineSpec for X64ABIMachineSpec {
 
         caller_saved
     }
+
+    fn get_ext_mode(
+        call_conv: isa::CallConv,
+        specified: ir::ArgumentExtension,
+    ) -> ir::ArgumentExtension {
+        if call_conv.extends_baldrdash() {
+            // Baldrdash (SpiderMonkey) always extends args and return values to the full register.
+            specified
+        } else {
+            // No other supported ABI on x64 does so.
+            ir::ArgumentExtension::None
+        }
+    }
 }
 
 impl From<StackAMode> for SyntheticAmode {

cranelift/codegen/src/isa/x86/abi.rs

@@ -144,8 +144,13 @@ impl ArgAssigner for Args {
             return ValueConversion::VectorSplit.into();
         }
 
-        // Small integers are extended to the size of a pointer register.
-        if ty.is_int() && ty.bits() < u16::from(self.pointer_bits) {
+        // Small integers are extended to the size of a pointer register, but
+        // only in ABIs that require this. The Baldrdash (SpiderMonkey) ABI
+        // does, but our other supported ABIs on x86 do not.
+        if ty.is_int()
+            && ty.bits() < u16::from(self.pointer_bits)
+            && self.call_conv.extends_baldrdash()
+        {
             match arg.extension {
                 ArgumentExtension::None => {}
                 ArgumentExtension::Uext => return ValueConversion::Uext(self.pointer_type).into(),

cranelift/codegen/src/machinst/abi_impl.rs

@@ -369,6 +369,16 @@ pub trait ABIMachineSpec {
     /// Get all caller-save registers, that is, registers that we expect
     /// not to be saved across a call to a callee with the given ABI.
     fn get_regs_clobbered_by_call(call_conv_of_callee: isa::CallConv) -> Vec<Writable<Reg>>;
+
+    /// Get the needed extension mode, given the mode attached to the argument
+    /// in the signature and the calling convention. The input (the attribute in
+    /// the signature) specifies what extension type should be done *if* the ABI
+    /// requires extension to the full register; this method's return value
+    /// indicates whether the extension actually *will* be done.
+    fn get_ext_mode(
+        call_conv: isa::CallConv,
+        specified: ir::ArgumentExtension,
+    ) -> ir::ArgumentExtension;
 }
 
 /// ABI information shared between body (callee) and caller.
@@ -770,6 +780,7 @@ impl<M: ABIMachineSpec> ABICallee for ABICalleeImpl<M> {
             &ABIArg::Reg(r, ty, ext, ..) => {
                 let from_bits = ty_bits(ty) as u8;
                 let dest_reg = Writable::from_reg(r.to_reg());
+                let ext = M::get_ext_mode(self.sig.call_conv, ext);
                 match (ext, from_bits) {
                     (ArgumentExtension::Uext, n) | (ArgumentExtension::Sext, n)
                         if n < word_bits =>
@@ -793,6 +804,7 @@ impl<M: ABIMachineSpec> ABICallee for ABICalleeImpl<M> {
                 // backends (aarch64 and x64) enforce a 128MB limit.
                 let off = i32::try_from(off)
                     .expect("Argument stack offset greater than 2GB; should hit impl limit first");
+                let ext = M::get_ext_mode(self.sig.call_conv, ext);
                 // Trash the from_reg; it should be its last use.
                 match (ext, from_bits) {
                     (ArgumentExtension::Uext, n) | (ArgumentExtension::Sext, n)
@@ -1211,27 +1223,28 @@ impl<M: ABIMachineSpec> ABICaller for ABICallerImpl<M> {
         let word_rc = M::word_reg_class();
         let word_bits = M::word_bits() as usize;
         match &self.sig.args[idx] {
-            &ABIArg::Reg(reg, ty, ext, _)
-                if ext != ir::ArgumentExtension::None && ty_bits(ty) < word_bits =>
-            {
-                assert_eq!(word_rc, reg.get_class());
-                let signed = match ext {
-                    ir::ArgumentExtension::Uext => false,
-                    ir::ArgumentExtension::Sext => true,
-                    _ => unreachable!(),
-                };
-                ctx.emit(M::gen_extend(
-                    Writable::from_reg(reg.to_reg()),
-                    from_reg,
-                    signed,
-                    ty_bits(ty) as u8,
-                    word_bits as u8,
-                ));
-            }
-            &ABIArg::Reg(reg, ty, _, _) => {
-                ctx.emit(M::gen_move(Writable::from_reg(reg.to_reg()), from_reg, ty));
+            &ABIArg::Reg(reg, ty, ext, _) => {
+                let ext = M::get_ext_mode(self.sig.call_conv, ext);
+                if ext != ir::ArgumentExtension::None && ty_bits(ty) < word_bits {
+                    assert_eq!(word_rc, reg.get_class());
+                    let signed = match ext {
+                        ir::ArgumentExtension::Uext => false,
+                        ir::ArgumentExtension::Sext => true,
+                        _ => unreachable!(),
+                    };
+                    ctx.emit(M::gen_extend(
+                        Writable::from_reg(reg.to_reg()),
+                        from_reg,
+                        signed,
+                        ty_bits(ty) as u8,
+                        word_bits as u8,
+                    ));
+                } else {
+                    ctx.emit(M::gen_move(Writable::from_reg(reg.to_reg()), from_reg, ty));
+                }
             }
             &ABIArg::Stack(off, mut ty, ext, _) => {
+                let ext = M::get_ext_mode(self.sig.call_conv, ext);
                 if ext != ir::ArgumentExtension::None && ty_bits(ty) < word_bits {
                     assert_eq!(word_rc, from_reg.get_class());
                     let signed = match ext {

cranelift/filetests/filetests/isa/aarch64/call.clif

@@ -1,4 +1,5 @@
 test compile
+set enable_probestack=false
 target aarch64
 
 function %f1(i64) -> i64 {
@@ -18,7 +19,7 @@ block0(v0: i64):
 ; nextln:  ret
 
 function %f2(i32) -> i64 {
-    fn0 = %g(i32 uext) -> i64
+    fn0 = %g(i32 uext) -> i64 baldrdash_system_v
 
 block0(v0: i32):
     v1 = call fn0(v0)
@@ -27,27 +28,22 @@ block0(v0: i32):
 
 ; check:  stp fp, lr, [sp, #-16]!
 ; nextln:  mov fp, sp
-; nextln:  mov w0, w0
+; check:  mov w0, w0
 ; nextln:  ldr x1, 8 ; b 12 ; data
 ; nextln:  blr x1
-; nextln:  mov sp, fp
+; check:  mov sp, fp
 ; nextln:  ldp fp, lr, [sp], #16
 ; nextln:  ret
 
-function %f3(i32) -> i32 uext {
+function %f3(i32) -> i32 uext baldrdash_system_v {
 block0(v0: i32):
     return v0
 }
 
-; check:  stp fp, lr, [sp, #-16]!
-; nextln:  mov fp, sp
-; nextln:  mov w0, w0
-; nextln:  mov sp, fp
-; nextln:  ldp fp, lr, [sp], #16
-; nextln:  ret
+; check:  mov w0, w0
 
 function %f4(i32) -> i64 {
-    fn0 = %g(i32 sext) -> i64
+    fn0 = %g(i32 sext) -> i64 baldrdash_system_v
 
 block0(v0: i32):
     v1 = call fn0(v0)
@@ -56,24 +52,19 @@ block0(v0: i32):
 
 ; check:  stp fp, lr, [sp, #-16]!
 ; nextln:  mov fp, sp
-; nextln:  sxtw x0, w0
+; check:  sxtw x0, w0
 ; nextln:  ldr x1, 8 ; b 12 ; data
 ; nextln:  blr x1
-; nextln:  mov sp, fp
+; check:  mov sp, fp
 ; nextln:  ldp fp, lr, [sp], #16
 ; nextln:  ret
 
-function %f5(i32) -> i32 sext {
+function %f5(i32) -> i32 sext baldrdash_system_v {
 block0(v0: i32):
     return v0
 }
 
-; check:  stp fp, lr, [sp, #-16]!
-; nextln:  mov fp, sp
-; nextln:  sxtw x0, w0
-; nextln:  mov sp, fp
-; nextln:  ldp fp, lr, [sp], #16
-; nextln:  ret
+; check:  sxtw x0, w0
 
 function %f6(i8) -> i64 {
     fn0 = %g(i32, i32, i32, i32, i32, i32, i32, i32, i8 sext) -> i64
@@ -97,8 +88,7 @@ block0(v0: i8):
 ; nextln:  movz x5, #42
 ; nextln:  movz x6, #42
 ; nextln:  movz x7, #42
-; nextln:  sxtb x8, w8
-; nextln:  stur x8, [sp]
+; nextln:  sturb w8, [sp]
 ; nextln:  ldr x8, 8 ; b 12 ; data
 ; nextln:  blr x8
 ; nextln:  add sp, sp, #16
@@ -125,8 +115,7 @@ block0(v0: i8):
 ; nextln:  movz x5, #42
 ; nextln:  movz x6, #42
 ; nextln:  movz x7, #42
-; nextln:  sxtb x9, w9
-; nextln:  stur x9, [x8]
+; nextln:  sturb w9, [x8]
 ; nextln:  mov sp, fp
 ; nextln:  ldp fp, lr, [sp], #16
 ; nextln:  ret