On 1 April 2021 20:31:13 CEST, Bernhard Reutner-Fischer <rep.dot....@gmail.com>
wrote:
>On 1 April 2021 18:54:07 CEST, "H.J. Lu via Gcc-patches"
><gcc-patches@gcc.gnu.org> wrote:
>>Since uiret should be used only for user interrupt handler return,
>>don't
>>generate uiret in interrupt handler return with -mcmodel=kernel even
>if
>>UINTR is enabled.
>>
>>gcc/
>>
>> PR target/99870
>> * config/i386/i386.md (interrupt_return): Don't generate uiret
>> for -mcmodel=kernel.
>>
>>gcc/testsuite/
>>
>> * gcc.target/i386/pr99870.c: New test.
>>---
>> gcc/config/i386/i386.md | 3 ++-
>> gcc/testsuite/gcc.target/i386/pr99870.c | 19 +++++++++++++++++++
>> 2 files changed, 21 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.target/i386/pr99870.c
>>
>>diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
>>index 9ff35d9a607..1055b4187b2 100644
>>--- a/gcc/config/i386/i386.md
>>+++ b/gcc/config/i386/i386.md
>>@@ -13885,7 +13885,8 @@ (define_insn "interrupt_return"
>> (unspec [(const_int 0)] UNSPEC_INTERRUPT_RETURN)]
>> "reload_completed"
>> {
>>- return TARGET_64BIT ? (TARGET_UINTR ? "uiret" : "iretq") : "iret";
>>+ return TARGET_64BIT ? ((TARGET_UINTR && ix86_cmodel != CM_KERNEL)
>>+ ? "uiret" : "iretq") : "iret";
>> })
>>
>>;; Used by x86_machine_dependent_reorg to avoid penalty on single byte
>>RET
>>diff --git a/gcc/testsuite/gcc.target/i386/pr99870.c
>>b/gcc/testsuite/gcc.target/i386/pr99870.c
>>new file mode 100644
>>index 00000000000..0dafa001ba9
>>--- /dev/null
>>+++ b/gcc/testsuite/gcc.target/i386/pr99870.c
>>@@ -0,0 +1,19 @@
>>+/* { dg-do compile } */
>>+/* { dg-options "-O2 -march=sapphirerapids -mgeneral-regs-only" } */
>>+/* { dg-additional-options "-mcmodel=kernel" { target { ! ia32 } } }
>>*/
>>+
>>+void
>>+__attribute__((interrupt))
>>+fn (void *frame)
Oh and of course that named param should trigger an unused parm warning making
the test fail as is.
Why wasn't that flagged? Isn't this warning on by default?
>>+{
>>+}
>>+
>>+typedef void (*fn_t) (void *) __attribute__((interrupt));
>>+
>>+fn_t fns[] =
>>+{
>>+ fn,
>>+};
>>+
>>+/* { dg-final { scan-assembler-times "\\tiret" 1 { target ia32 } } }
>
>This matches ia32 and the others as well (x32 and amd64 or x86_64)
>doesn't it.
>>*/
>>+/* { dg-final { scan-assembler-times "\\tiretq" 1 { target { ! ia32 }
>>} } } */
>
>So this is superfluous without a trailing anchor for the first, fwiw.
>Thanks,
