x86 interrupt calling convention: only save xmm registers if the target supports SSE

[phil-opp]

Summary

The existing code always saves the xmm registers for 64-bit targets even if the target doesn't support SSE (which is common for kernels). Thus, the compiler inserts movaps instructions which lead to CPU exceptions when an interrupt handler is invoked.

This commit fixes this bug by returning a register set without xmm registers from getCalleeSavedRegs and getCallPreservedMask for such targets.

[tari]

LGTM. I thought we already handled the no-SSE case, but looks like only for the 32-bit subtarget.

[RKSimon]

Please can you confirm if this fixes PR26413 (and if possible add a test)?

[phil-opp]

PR26413 is a different problem: The generated code for targets with SSE support uses movaps instructions instead of movups instructions, which leads to alignment exceptions because the stack is only 8-byte aligned.

This is my first time hacking on LLVM, so I don't know how to test this.

[aaboud]

Looks Good to me.
Can you add a LIT test?

See test\CodeGen\X86\x86-64-intrcc.ll for reference.

[phil-opp]

I added a regression test.

(By the way, it seems like the test_isr_clobbers clobbers test of x86-64-intrcc.ll is broken, since the CHECK-SSE-NEXT commands are invalid.)

[aaboud]

(By the way, it seems like the test_isr_clobbers clobbers test of x86-64-intrcc.ll is broken, since the CHECK-SSE-NEXT commands are invalid.)

I am aware of that, and I uploaded a fix as part of D22044.

[phil-opp]

I don't have commit access. Could someone commit it for me?

[andreadb]

I only have a minor comment about the test.

Given how simple is function @test_isr_sse_clobbers, I wouldn't be surprised if the codegen with/without -O0 is the same.
If so, then you should be able to get rid of CHECK0; at the moment, CHECK and CHECK0 are basically equivalent classes of checks.

I also suggest to automatically generate CHECK lines using 'update_llc_test_checks.py' (it is up to you).
Since the body of @test_isr_sse_clobbers is very small, I don't expect to see many automatically generated CHECK lines anyway.

[phil-opp]

You were right: The generated code with and without -O0 is identical.

I removed the CHECK0 lines and regenerated the assertions using the python script.

[phil-opp]

Could someone commit this for me, please?

[andreadb]

In D29959#678801, @phil-opp wrote:

Could someone commit this for me, please?

Sure. I will commit this patch for you.

llvm/trunk/lib/Target/X86/X86CallingConv.td

def CSR_32_AllRegs_SSE : CalleeSavedRegs<(add CSR_32_AllRegs,
                                               (sequence "XMM%u", 0, 7))>;
 def CSR_32_AllRegs_AVX : CalleeSavedRegs<(add CSR_32_AllRegs,
                                               (sequence "YMM%u", 0, 7))>;
 def CSR_32_AllRegs_AVX512 : CalleeSavedRegs<(add CSR_32_AllRegs,
                                                  (sequence "ZMM%u", 0, 7),
                                                  (sequence "K%u", 0, 7))>;
 
 def CSR_64_AllRegs     : CalleeSavedRegs<(add CSR_64_MostRegs, RAX)>;
+def CSR_64_AllRegs_NoSSE : CalleeSavedRegs<(add RAX, RBX, RCX, RDX, RSI, RDI, R8, R9,
+                                                R10, R11, R12, R13, R14, R15, RBP)>;
 def CSR_64_AllRegs_AVX : CalleeSavedRegs<(sub (add CSR_64_MostRegs, RAX,
                                                    (sequence "YMM%u", 0, 15)),
                                               (sequence "XMM%u", 0, 15))>;
 def CSR_64_AllRegs_AVX512 : CalleeSavedRegs<(sub (add CSR_64_MostRegs, RAX,
                                                       (sequence "ZMM%u", 0, 31),
                                                       (sequence "K%u", 0, 7)),
                                                  (sequence "XMM%u", 0, 15))>;
 

llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp

if (CallsEHReturn)
       return CSR_64EHRet_SaveList;
     return CSR_64_SaveList;
   case CallingConv::X86_INTR:
     if (Is64Bit) {
       if (HasAVX512)
         return CSR_64_AllRegs_AVX512_SaveList;
       if (HasAVX)
         return CSR_64_AllRegs_AVX_SaveList;
+      if (HasSSE)
         return CSR_64_AllRegs_SaveList;
+      return CSR_64_AllRegs_NoSSE_SaveList;
     } else {
       if (HasAVX512)
         return CSR_32_AllRegs_AVX512_SaveList;
       if (HasAVX)
         return CSR_32_AllRegs_AVX_SaveList;
       if (HasSSE)
         return CSR_32_AllRegs_SSE_SaveList;
       return CSR_32_AllRegs_SaveList;
X86RegisterInfo::getCallPreservedMask(const MachineFunction &MF,
   case CallingConv::X86_64_SysV:
     return CSR_64_RegMask;
   case CallingConv::X86_INTR:
     if (Is64Bit) {
       if (HasAVX512)
         return CSR_64_AllRegs_AVX512_RegMask;
       if (HasAVX)
         return CSR_64_AllRegs_AVX_RegMask;
+      if (HasSSE)
         return CSR_64_AllRegs_RegMask;
+      return CSR_64_AllRegs_NoSSE_RegMask;
     } else {
       if (HasAVX512)
         return CSR_32_AllRegs_AVX512_RegMask;
       if (HasAVX)
         return CSR_32_AllRegs_AVX_RegMask;
       if (HasSSE)
         return CSR_32_AllRegs_SSE_RegMask;
       return CSR_32_AllRegs_RegMask;

llvm/trunk/test/CodeGen/X86/x86-64-intrcc-nosse.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=x86_64-unknown-unknown -mattr=-sse < %s | FileCheck %s
+
+%struct.interrupt_frame = type { i64, i64, i64, i64, i64 }
+
[e-mail] = appending global [1 x i8*] [i8* bitcast (void (%struct.interrupt_frame*, i64)* @test_isr_sse_clobbers to i8*)], section "llvm.metadata"
+
+; Clobbered SSE must not be saved when the target doesn't support SSE
+define x86_intrcc void @test_isr_sse_clobbers(%struct.interrupt_frame* %frame, i64 %ecode) {
+  ; CHECK-LABEL: test_isr_sse_clobbers:
+  ; CHECK:       # BB#0:
+  ; CHECK-NEXT:    cld
+  ; CHECK-NEXT:    #APP
+  ; CHECK-NEXT:    #NO_APP
+  ; CHECK-NEXT:    addq $8, %rsp
+  ; CHECK-NEXT:    iretq
+  call void asm sideeffect "", "~{xmm0},~{xmm6}"()
+  ret void
+}