Update is_clobbered_in_inst to affect only clobbers, not all defs.

Author: cfallin

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

@@ -1094,7 +1094,6 @@ impl ABIMachineSpec for AArch64MachineDeps {
                 caller_conv: call_conv,
                 callee_conv: call_conv,
                 callee_pop_size: 0,
-                has_non_abi_defs: false,
             }),
         });
         insts

cranelift/codegen/src/isa/aarch64/inst/mod.rs

@@ -979,8 +979,6 @@ impl MachInst for Inst {
     fn is_included_in_clobbers(&self) -> bool {
         let (caller, callee) = match self {
             Inst::Args { .. } => return false,
-            Inst::Call { info } if info.has_non_abi_defs => return true,
-            Inst::CallInd { info } if info.has_non_abi_defs => return true,
             Inst::Call { info } => (info.caller_conv, info.callee_conv),
             Inst::CallInd { info } => (info.caller_conv, info.callee_conv),
             _ => return true,

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

@@ -614,7 +614,6 @@ impl ABIMachineSpec for Riscv64MachineDeps {
                 caller_conv: call_conv,
                 callee_conv: call_conv,
                 callee_pop_size: 0,
-                has_non_abi_defs: false,
             }),
         });
         insts

cranelift/codegen/src/isa/s390x/inst/mod.rs

@@ -1069,9 +1069,7 @@ impl MachInst for Inst {
         // registers.
         match self {
             &Inst::Args { .. } => false,
-            &Inst::Call { ref info, .. } => {
-                info.caller_conv != info.callee_conv || info.has_non_abi_defs
-            }
+            &Inst::Call { ref info, .. } => info.caller_conv != info.callee_conv,
             &Inst::CallInd { ref info, .. } => info.caller_conv != info.callee_conv,
             &Inst::ElfTlsGetOffset { .. } => false,
             _ => true,

cranelift/codegen/src/isa/s390x/lower/isle.rs

@@ -999,7 +999,6 @@ impl IsleContext<'_, '_, MInst, S390xBackend> {
             callee_pop_size,
             caller_conv: self.lower_ctx.abi().call_conv(self.lower_ctx.sigs()),
             callee_conv: self.lower_ctx.sigs()[abi].call_conv(),
-            has_non_abi_defs: false,
         }
     }
 }

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

@@ -877,7 +877,6 @@ impl ABIMachineSpec for X64ABIMachineSpec {
             defs: smallvec![],
             clobbers: Self::get_regs_clobbered_by_call(call_conv),
             callee_pop_size,
-            has_non_abi_defs: false,
             callee_conv: call_conv,
             caller_conv: call_conv,
         })));

cranelift/codegen/src/machinst/abi.rs

@@ -620,10 +620,6 @@ pub struct CallInfo<T> {
     /// caller, if any. (Used for popping stack arguments with the `tail`
     /// calling convention.)
     pub callee_pop_size: u32,
-    /// Do the defs have any definitions outside of the ABI-implied
-    /// clobbers? If so, this instruction needs to be considered when
-    /// computing the function body's clobbered registers.
-    pub has_non_abi_defs: bool,
 }
 
 impl<T> CallInfo<T> {
@@ -638,7 +634,6 @@ impl<T> CallInfo<T> {
             caller_conv: call_conv,
             callee_conv: call_conv,
             callee_pop_size: 0,
-            has_non_abi_defs: false,
         }
     }
 
@@ -652,7 +647,6 @@ impl<T> CallInfo<T> {
             caller_conv: self.caller_conv,
             callee_conv: self.callee_conv,
             callee_pop_size: self.callee_pop_size,
-            has_non_abi_defs: self.has_non_abi_defs,
         }
     }
 }
@@ -2036,9 +2030,6 @@ pub struct CallSite<M: ABIMachineSpec> {
     caller_conv: isa::CallConv,
     /// The settings controlling this compilation.
     flags: settings::Flags,
-    /// Has any defs that are not constrained to ABI-specified
-    /// registers.
-    has_non_abi_defs: bool,
 
     _mach: PhantomData<M>,
 }
@@ -2072,7 +2063,6 @@ impl<M: ABIMachineSpec> CallSite<M> {
             is_tail_call,
             caller_conv,
             flags,
-            has_non_abi_defs: false,
             _mach: PhantomData,
         }
     }
@@ -2096,7 +2086,6 @@ impl<M: ABIMachineSpec> CallSite<M> {
             is_tail_call: IsTailCall::No,
             caller_conv,
             flags,
-            has_non_abi_defs: false,
             _mach: PhantomData,
         }
     }
@@ -2120,7 +2109,6 @@ impl<M: ABIMachineSpec> CallSite<M> {
             is_tail_call,
             caller_conv,
             flags,
-            has_non_abi_defs: false,
             _mach: PhantomData,
         }
     }
@@ -2373,7 +2361,6 @@ impl<M: ABIMachineSpec> CallSite<M> {
                                 vreg: into_reg,
                                 location: RetLocation::Stack(amode, ty),
                             });
-                            self.has_non_abi_defs = true;
                             into_regs.push(into_reg.to_reg());
                         }
                     }
@@ -2474,7 +2461,6 @@ impl<M: ABIMachineSpec> CallSite<M> {
                 callee_conv: call_conv,
                 caller_conv: self.caller_conv,
                 callee_pop_size,
-                has_non_abi_defs: self.has_non_abi_defs,
             },
         )
         .into_iter()

cranelift/codegen/src/machinst/mod.rs

@@ -112,7 +112,8 @@ pub trait MachInst: Clone + Debug {
     /// Is this an "args" pseudoinst?
     fn is_args(&self) -> bool;
 
-    /// Should this instruction be included in the clobber-set?
+    /// Should this instruction's clobber-list be included in the
+    /// clobber-set?
     fn is_included_in_clobbers(&self) -> bool;
 
     /// Does this instruction access memory?

cranelift/codegen/src/machinst/vcode.rs

@@ -672,16 +672,6 @@ impl<I: VCodeInst> VCode<I> {
         }
 
         for (i, range) in self.operand_ranges.iter() {
-            // Skip this instruction if not "included in clobbers" as
-            // per the MachInst. (Some backends use this to implement
-            // ABI specifics; e.g., excluding calls of the same ABI as
-            // the current function from clobbers, because by
-            // definition everything clobbered by the call can be
-            // clobbered by this function without saving as well.)
-            if !self.insts[i].is_included_in_clobbers() {
-                continue;
-            }
-
             let operands = &self.operands[range.clone()];
             let allocs = &regalloc.allocs[range];
             for (operand, alloc) in operands.iter().zip(allocs.iter()) {
@@ -693,8 +683,28 @@ impl<I: VCodeInst> VCode<I> {
             }
 
             // Also add explicitly-clobbered registers.
-            if let Some(&inst_clobbered) = self.clobbers.get(&InsnIndex::new(i)) {
-                clobbered.union_from(inst_clobbered);
+            //
+            // Skip merging this instruction's clobber list if not
+            // "included in clobbers" as per the MachInst. (Some
+            // backends use this to implement ABI specifics; e.g.,
+            // excluding calls of the same ABI as the current function
+            // from clobbers, because by definition everything
+            // clobbered by the call can be clobbered by this function
+            // without saving as well.
+            //
+            // This is important for a particular optimization: when
+            // some registers are "half-clobbered", e.g. vector/float
+            // registers on aarch64, we want them to be seen as
+            // clobbered by regalloc so it avoids carrying values
+            // across calls in these registers but not seen as
+            // clobbered by prologue generation here (because the
+            // actual half-clobber implied by the clobber list fits
+            // within the clobbers that we allow without
+            // clobber-saves).
+            if self.insts[i].is_included_in_clobbers() {
+                if let Some(&inst_clobbered) = self.clobbers.get(&InsnIndex::new(i)) {
+                    clobbered.union_from(inst_clobbered);
+                }
             }
         }
 

cranelift/filetests/filetests/isa/aarch64/tail-call-conv.clif

@@ -385,21 +385,13 @@ block0:
 ;   stp x23, x24, [sp, #-16]!
 ;   stp x21, x22, [sp, #-16]!
 ;   stp x19, x20, [sp, #-16]!
-;   stp d14, d15, [sp, #-16]!
-;   stp d12, d13, [sp, #-16]!
-;   stp d10, d11, [sp, #-16]!
-;   stp d8, d9, [sp, #-16]!
 ;   sub sp, sp, #240
 ; block0:
 ;   mov x8, sp
 ;   load_ext_name x12, TestCase(%tail_callee_stack_rets)+0
 ;   blr x12
 ;   ldr x2, [sp, #232]
 ;   add sp, sp, #240
-;   ldp d8, d9, [sp], #16
-;   ldp d10, d11, [sp], #16
-;   ldp d12, d13, [sp], #16
-;   ldp d14, d15, [sp], #16
 ;   ldp x19, x20, [sp], #16
 ;   ldp x21, x22, [sp], #16
 ;   ldp x23, x24, [sp], #16
@@ -417,15 +409,11 @@ block0:
 ;   stp x23, x24, [sp, #-0x10]!
 ;   stp x21, x22, [sp, #-0x10]!
 ;   stp x19, x20, [sp, #-0x10]!
-;   stp d14, d15, [sp, #-0x10]!
-;   stp d12, d13, [sp, #-0x10]!
-;   stp d10, d11, [sp, #-0x10]!
-;   stp d8, d9, [sp, #-0x10]!
 ;   sub sp, sp, #0xf0
-; block1: ; offset 0x30
+; block1: ; offset 0x20
 ;   mov x8, sp
-;   ldr x12, #0x3c
-;   b #0x44
+;   ldr x12, #0x2c
+;   b #0x34
 ;   .byte 0x00, 0x00, 0x00, 0x00 ; reloc_external Abs8 %tail_callee_stack_rets 0
 ;   .byte 0x00, 0x00, 0x00, 0x00
 ;   blr x12
@@ -461,10 +449,6 @@ block0:
 ;   stur x9, [sp, #0xe8]
 ;   ldur x2, [sp, #0xe8]
 ;   add sp, sp, #0xf0
-;   ldp d8, d9, [sp], #0x10
-;   ldp d10, d11, [sp], #0x10
-;   ldp d12, d13, [sp], #0x10
-;   ldp d14, d15, [sp], #0x10
 ;   ldp x19, x20, [sp], #0x10
 ;   ldp x21, x22, [sp], #0x10
 ;   ldp x23, x24, [sp], #0x10
@@ -648,10 +632,6 @@ block0:
 ;   stp x23, x24, [sp, #-16]!
 ;   stp x21, x22, [sp, #-16]!
 ;   stp x19, x20, [sp, #-16]!
-;   stp d14, d15, [sp, #-16]!
-;   stp d12, d13, [sp, #-16]!
-;   stp d10, d11, [sp, #-16]!
-;   stp d8, d9, [sp, #-16]!
 ;   sub sp, sp, #400
 ; block0:
 ;   movz x2, #10
@@ -705,10 +685,6 @@ block0:
 ;   blr x10
 ;   ldr x2, [sp, #392]
 ;   add sp, sp, #400
-;   ldp d8, d9, [sp], #16
-;   ldp d10, d11, [sp], #16
-;   ldp d12, d13, [sp], #16
-;   ldp d14, d15, [sp], #16
 ;   ldp x19, x20, [sp], #16
 ;   ldp x21, x22, [sp], #16
 ;   ldp x23, x24, [sp], #16
@@ -726,12 +702,8 @@ block0:
 ;   stp x23, x24, [sp, #-0x10]!
 ;   stp x21, x22, [sp, #-0x10]!
 ;   stp x19, x20, [sp, #-0x10]!
-;   stp d14, d15, [sp, #-0x10]!
-;   stp d12, d13, [sp, #-0x10]!
-;   stp d10, d11, [sp, #-0x10]!
-;   stp d8, d9, [sp, #-0x10]!
 ;   sub sp, sp, #0x190
-; block1: ; offset 0x30
+; block1: ; offset 0x20
 ;   mov x2, #0xa
 ;   mov x3, #0xf
 ;   mov x4, #0x14
@@ -779,8 +751,8 @@ block0:
 ;   stur x20, [sp, #0x90]
 ;   stur x22, [sp, #0x98]
 ;   add x8, sp, #0xa0
-;   ldr x10, #0xf4
-;   b #0xfc
+;   ldr x10, #0xe4
+;   b #0xec
 ;   .byte 0x00, 0x00, 0x00, 0x00 ; reloc_external Abs8 %tail_callee_stack_args_and_rets 0
 ;   .byte 0x00, 0x00, 0x00, 0x00
 ;   blr x10
@@ -817,10 +789,6 @@ block0:
 ;   str x9, [sp, #0x188]
 ;   ldr x2, [sp, #0x188]
 ;   add sp, sp, #0x190
-;   ldp d8, d9, [sp], #0x10
-;   ldp d10, d11, [sp], #0x10
-;   ldp d12, d13, [sp], #0x10
-;   ldp d14, d15, [sp], #0x10
 ;   ldp x19, x20, [sp], #0x10
 ;   ldp x21, x22, [sp], #0x10
 ;   ldp x23, x24, [sp], #0x10