[X86] Enable compilation of user interrupt handlers.

[tianqing]

Summary

Add __uintr_frame structure and use UIRET instruction for functions with
x86 interrupt calling convention when UINTR is present.

[craig.topper]

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

[LuoYuanke]

In D99708#2663989, @craig.topper wrote:

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

[hjl.tools]

In D99708#2664076, @LuoYuanke wrote:

In D99708#2663989, @craig.topper wrote:

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler in the source, there is no need for another
attribute. We discussed that kernel might need to use UINTR instructions. We decided that kernel could use inline asm
statements if needed.

[craig.topper]

In D99708#2664164, @hjl.tools wrote:

In D99708#2664076, @LuoYuanke wrote:

In D99708#2663989, @craig.topper wrote:

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler in the source, there is no need for another
attribute. We discussed that kernel might need to use UINTR instructions. We decided that kernel could use inline asm
statements if needed.

So if write kernel code and compile with -march=haswell today, I get IRET. If tomorrow I change my command line to -march=sapphirerapids, now my kernel interrupt code generates user interrupt instructions. That seems surprising.

[hjl.tools]

In D99708#2664218, @craig.topper wrote:

In D99708#2664164, @hjl.tools wrote:

In D99708#2664076, @LuoYuanke wrote:

In D99708#2663989, @craig.topper wrote:

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler in the source, there is no need for another
attribute. We discussed that kernel might need to use UINTR instructions. We decided that kernel could use inline asm
statements if needed.

So if write kernel code and compile with -march=haswell today, I get IRET. If tomorrow I change my command line to -march=sapphirerapids, now my kernel interrupt code generates user interrupt instructions. That seems surprising.

-mcmodel=kernel should disable uiret.:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99870

[craig.topper]

In D99708#2664351, @hjl.tools wrote:

In D99708#2664218, @craig.topper wrote:

In D99708#2664164, @hjl.tools wrote:

In D99708#2664076, @LuoYuanke wrote:

In D99708#2663989, @craig.topper wrote:

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler in the source, there is no need for another
attribute. We discussed that kernel might need to use UINTR instructions. We decided that kernel could use inline asm
statements if needed.

So if write kernel code and compile with -march=haswell today, I get IRET. If tomorrow I change my command line to -march=sapphirerapids, now my kernel interrupt code generates user interrupt instructions. That seems surprising.

-mcmodel=kernel should disable uiret.:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99870

That makes sense. Can we put that in this patch?

[hjl.tools]

In D99708#2664372, @craig.topper wrote:

In D99708#2664351, @hjl.tools wrote:

In D99708#2664218, @craig.topper wrote:

In D99708#2664164, @hjl.tools wrote:

In D99708#2664076, @LuoYuanke wrote:

In D99708#2663989, @craig.topper wrote:

A user interrupt is different than a regular interrupt right? It doesn't make sense that we would change the behavior of the interrupt calling convention just because the the user interrupt instructions are enabled. That would occur just from passing a -march for a newer CPU wouldn't it?

Maybe need support another attribute "attribute ((user_interrupt))" for functions? However this is what gcc does (https://gcc.godbolt.org/z/8ojTMG6bT).

Since there won't be both user interrupt handler and kernel interrupt handler in the source, there is no need for another
attribute. We discussed that kernel might need to use UINTR instructions. We decided that kernel could use inline asm
statements if needed.

So if write kernel code and compile with -march=haswell today, I get IRET. If tomorrow I change my command line to -march=sapphirerapids, now my kernel interrupt code generates user interrupt instructions. That seems surprising.

-mcmodel=kernel should disable uiret.:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99870

That makes sense. Can we put that in this patch?

The feedback is that don't enable UINTR for kernel build.

[tianqing]

Update handling of -mcmodel=kernel.

[LuoYuanke]

LGMT. Thank you!

[craig.topper]

llvm/test/CodeGen/X86/x86-64-intrcc-uintr.ll

2

Please use update_llc_test_checks.py

[tianqing]

Does anyone has further comments?

[LuoYuanke]

LGTM. But wait one or two days to see if there is more comments from Craig and HJ.

clang/lib/Headers/uintrintrin.h

 /*===------------------ uintrintrin.h - UINTR intrinsics -------------------===
  *
  * Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
  * See https://llvm.org/LICENSE.txt for license information.
  * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
  *
  *===-----------------------------------------------------------------------===
  */
 
 #ifndef __X86GPRINTRIN_H
 #error "Never use <uintrintrin.h> directly; include <x86gprintrin.h> instead."
 #endif
 
 #ifndef __UINTRINTRIN_H
 #define __UINTRINTRIN_H
 
 /* Define the default attributes for the functions in this file */
 #define __DEFAULT_FN_ATTRS \
   __attribute__((__always_inline__, __nodebug__, __target__("uintr")))
 
 #ifdef __x86_64__
 
+struct __uintr_frame
+{
+  unsigned long long rip;
+  unsigned long long rflags;
+  unsigned long long rsp;
+};
+
 /// Clears the user interrupt flag (UIF). Its effect takes place immediately: a
 ///    user interrupt cannot be delivered on the instruction boundary following
 ///    CLUI. Can be executed only if CR4.UINT = 1, the logical processor is in
 ///    64-bit mode, and software is not executing inside an enclave; otherwise,
 ///    each causes an invalid-opcode exception. Causes a transactional abort if
 ///    executed inside a transactional region; the abort loads EAX as it would
 ///    had it been due to an execution of CLI.
 ///
 /// \headerfile <x86gprintrin.h>
 ///
 /// This intrinsic corresponds to the <c> CLUI </c> instruction.
 ///
 /// \operation
 ///   UIF := 0
 /// \endoperation
 static __inline__ void __DEFAULT_FN_ATTRS
 _clui (void)
 {
   __builtin_ia32_clui();
 }
 
 /// Sets the user interrupt flag (UIF). Its effect takes place immediately; a
 ///    user interrupt may be delivered on the instruction boundary following
 ///    STUI. Can be executed only if CR4.UINT = 1, the logical processor is in
 ///    64-bit mode, and software is not executing inside an enclave; otherwise,
 ///    each causes an invalid-opcode exception. Causes a transactional abort if
 ///    executed inside a transactional region; the abort loads EAX as it would
 ///    had it been due to an execution of STI.
 ///
 /// \headerfile <x86gprintrin.h>
 ///
 /// This intrinsic corresponds to the <c> STUI </c> instruction.
 ///
 /// \operation
 ///   UIF := 1
 /// \endoperation
 static __inline__ void __DEFAULT_FN_ATTRS
 _stui (void)
 {
   __builtin_ia32_stui();
 }
 
 /// Get the current value of the user interrupt flag (UIF). Can be executed
 ///    regardless of CPL and inside a transactional region. Can be executed only
 ///    if CR4.UINT = 1, the logical processor is in 64-bit mode, and software is
 ///    not executing inside an enclave; otherwise, it causes an invalid-opcode
 ///    exception.
 ///

 ///   OF := 0
 ///   PF := 0
 ///   SF := 0
 ///   dst := CF
 /// \endoperation
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
 _testui (void)
 {
   return __builtin_ia32_testui();
 }
 
 /// Send interprocessor user interrupt. Can be executed only if
 ///    CR4.UINT = IA32_UINT_TT[0] = 1, the logical processor is in 64-bit mode,
 ///    and software is not executing inside an enclave; otherwise, it causes an
 ///    invalid-opcode exception. May be executed at any privilege level, all of
 ///    its memory accesses are performed with supervisor privilege.
 ///

 ///       // ID tempUPID.NDST[15:8]
 ///       SendOrdinaryIPI(tempUPID.NV, tempUPID.NDST[15:8])
 ///     FI
 ///   FI
 /// \endoperation
 static __inline__ void __DEFAULT_FN_ATTRS
 _senduipi (unsigned long long __a)
 {
   __builtin_ia32_senduipi(__a);
 }
 
 #endif /* __x86_64__ */
 
 #undef __DEFAULT_FN_ATTRS
 
 #endif /* __UINTRINTRIN_H */

llvm/lib/Target/X86/X86ExpandPseudo.cpp

case X86::EH_RETURN64: {
     return true;
   }
   case X86::IRET: {
     // Adjust stack to erase error code
     int64_t StackAdj = MBBI->getOperand(0).getImm();
     X86FL->emitSPUpdate(MBB, MBBI, DL, StackAdj, true);
     // Replace pseudo with machine iret
     BuildMI(MBB, MBBI, DL,
-            TII->get(STI->is64Bit() ? X86::IRET64 : X86::IRET32));
+            TII->get(STI->is64Bit()
+                         ? (STI->hasUINTR() ? X86::UIRET : X86::IRET64)
+                         : X86::IRET32));
     MBB.erase(MBBI);
     return true;
   }
   case X86::RET: {
     // Adjust stack to erase error code
     int64_t StackAdj = MBBI->getOperand(0).getImm();
     MachineInstrBuilder MIB;
     if (StackAdj == 0) {

llvm/test/CodeGen/X86/x86-64-intrcc-uintr.ll

+; RUN: llc < %s | FileCheck %s
+; RUN: llc -O0 < %s | FileCheck %s -check-prefix=CHECK0
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.__uintr_frame = type { i64, i64, i64 }
+
+; #include <x86gprintrin.h>
+;
+; void
+; __attribute__ ((interrupt))
+; test_uintr_isr_cc_empty(struct __uintr_frame *frame, unsigned long long uirrv)
+; {
+; }
+
+define dso_local x86_intrcc void @test_uintr_isr_cc_empty(%struct.__uintr_frame* nocapture byval(%struct.__uintr_frame) %frame, i64 %uirrv) #0 {
+; CHECK-LABEL: test_uintr_isr_cc_empty:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    cld
+; CHECK-NEXT:    addq $16, %rsp
+; CHECK-NEXT:    uiret
+;
+; CHECK0-LABEL: test_uintr_isr_cc_empty:
+; CHECK0:       # %bb.0: # %entry
+; CHECK0-NEXT:    pushq %rax
+; CHECK0-NEXT:    cld
+; CHECK0-NEXT:    addq $16, %rsp
+; CHECK0-NEXT:    uiret
+entry:
+  ret void
+}
+
+; unsigned long long g_rip;
+; unsigned long long g_rflags;
+; unsigned long long g_rsp;
+; unsigned long long g_uirrv;
+;
+; void
+; __attribute__((interrupt))
+; test_uintr_isr_cc_args(struct __uintr_frame *frame, unsigned long long uirrv)
+; {
+;   g_rip = frame->rip;
+;   g_rflags = frame->rflags;
+;   g_rsp = frame->rsp;
+;   g_uirrv = uirrv;
+; }
+@g_rip = dso_local local_unnamed_addr global i64 0, align 8
+@g_rflags = dso_local local_unnamed_addr global i64 0, align 8
+@g_rsp = dso_local local_unnamed_addr global i64 0, align 8
+@g_uirrv = dso_local local_unnamed_addr global i64 0, align 8
+
+define dso_local x86_intrcc void @test_uintr_isr_cc_args(%struct.__uintr_frame* nocapture readonly byval(%struct.__uintr_frame) %frame, i64 %uirrv) #0 {
+; CHECK-LABEL: test_uintr_isr_cc_args:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    pushq %rax
+; CHECK-NEXT:    pushq %rdx
+; CHECK-NEXT:    pushq %rcx
+; CHECK-NEXT:    cld
+; CHECK-NEXT:    movq 32(%rsp), %rax
+; CHECK-NEXT:    movq 40(%rsp), %rcx
+; CHECK-NEXT:    movq 48(%rsp), %rdx
+; CHECK-NEXT:    movq %rcx, g_rip(%rip)
+; CHECK-NEXT:    movq %rdx, g_rflags(%rip)
+; CHECK-NEXT:    movq 56(%rsp), %rcx
+; CHECK-NEXT:    movq %rcx, g_rsp(%rip)
+; CHECK-NEXT:    movq %rax, g_uirrv(%rip)
+; CHECK-NEXT:    popq %rcx
+; CHECK-NEXT:    popq %rdx
+; CHECK-NEXT:    popq %rax
+; CHECK-NEXT:    addq $16, %rsp
+; CHECK-NEXT:    uiret
+;
+; CHECK0-LABEL: test_uintr_isr_cc_args:
+; CHECK0:       # %bb.0: # %entry
+; CHECK0-NEXT:    pushq %rax
+; CHECK0-NEXT:    pushq %rax
+; CHECK0-NEXT:    pushq %rdx
+; CHECK0-NEXT:    pushq %rcx
+; CHECK0-NEXT:    cld
+; CHECK0-NEXT:    movq 32(%rsp), %rax
+; CHECK0-NEXT:    leaq 40(%rsp), %rcx
+; CHECK0-NEXT:    movq (%rcx), %rdx
+; CHECK0-NEXT:    movq %rdx, g_rip(%rip)
+; CHECK0-NEXT:    movq 8(%rcx), %rdx
+; CHECK0-NEXT:    movq %rdx, g_rflags(%rip)
+; CHECK0-NEXT:    movq 16(%rcx), %rcx
+; CHECK0-NEXT:    movq %rcx, g_rsp(%rip)
+; CHECK0-NEXT:    movq %rax, g_uirrv(%rip)
+; CHECK0-NEXT:    popq %rcx
+; CHECK0-NEXT:    popq %rdx
+; CHECK0-NEXT:    popq %rax
+; CHECK0-NEXT:    addq $16, %rsp
+; CHECK0-NEXT:    uiret
+entry:
+  %rip = getelementptr inbounds %struct.__uintr_frame, %struct.__uintr_frame* %frame, i64 0, i32 0
+  %0 = load i64, i64* %rip, align 8
+  store i64 %0, i64* @g_rip, align 8
+  %rflags = getelementptr inbounds %struct.__uintr_frame, %struct.__uintr_frame* %frame, i64 0, i32 1
+  %1 = load i64, i64* %rflags, align 8
+  store i64 %1, i64* @g_rflags, align 8
+  %rsp = getelementptr inbounds %struct.__uintr_frame, %struct.__uintr_frame* %frame, i64 0, i32 2
+  %2 = load i64, i64* %rsp, align 8
+  store i64 %2, i64* @g_rsp, align 8
+  store i64 %uirrv, i64* @g_uirrv, align 8
+  ret void
+}
+
+attributes #0 = { nofree norecurse nounwind willreturn "disable-tail-calls"="true" "frame-pointer"="none" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+uintr" "tune-cpu"="generic" }