On Thu, Dec 10, 2020 at 10:20 PM H.J. Lu <hjl.to...@gmail.com> wrote: > > User interrupt handler stack frame is similar to exception interrupt > handler stack frame. Instead of error code, the second argument is > user interrupt request register vector. > > gcc/ > > PR target/98219 > * config/i386/uintrintrin.h (__uintr_frame): Remove uirrv. > > gcc/testsuite/ > > PR target/98219 > * gcc.dg/guality/pr98219-1.c: New test. > * gcc.dg/guality/pr98219-2.c: Likewise. > * gcc.dg/torture/pr98219-1.c: Likewise. > * gcc.dg/torture/pr98219-2.c: Likewise. > * gcc.target/i386/uintr-2.c: Scan "add[lq] $8, %[er]sp". > (foo): Add an unsigned long long argument. > (UINTR_hanlder): Likewise. > * gcc.target/i386/uintr-3.c: Scan "add[lq] $8, %[er]sp". > (UINTR_hanlder): Add an unsigned long long argument. > * gcc.target/i386/uintr-4.c (UINTR_hanlder): Likewise. > * gcc.target/i386/uintr-5.c (UINTR_hanlder): Likewise.
OK with the fixes, described inline. Thanks, Uros. > --- > gcc/config/i386/uintrintrin.h | 3 -- > gcc/testsuite/gcc.dg/guality/pr98219-1.c | 48 ++++++++++++++++++ > gcc/testsuite/gcc.dg/guality/pr98219-2.c | 63 ++++++++++++++++++++++++ > gcc/testsuite/gcc.dg/torture/pr98219-1.c | 44 +++++++++++++++++ > gcc/testsuite/gcc.dg/torture/pr98219-2.c | 59 ++++++++++++++++++++++ > gcc/testsuite/gcc.target/i386/uintr-2.c | 5 +- > gcc/testsuite/gcc.target/i386/uintr-3.c | 4 +- > gcc/testsuite/gcc.target/i386/uintr-4.c | 4 +- > gcc/testsuite/gcc.target/i386/uintr-5.c | 2 +- > 9 files changed, 223 insertions(+), 9 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/guality/pr98219-1.c > create mode 100644 gcc/testsuite/gcc.dg/guality/pr98219-2.c > create mode 100644 gcc/testsuite/gcc.dg/torture/pr98219-1.c > create mode 100644 gcc/testsuite/gcc.dg/torture/pr98219-2.c > > diff --git a/gcc/config/i386/uintrintrin.h b/gcc/config/i386/uintrintrin.h > index 991f6427971..4606caf8582 100644 > --- a/gcc/config/i386/uintrintrin.h > +++ b/gcc/config/i386/uintrintrin.h > @@ -38,9 +38,6 @@ > > struct __uintr_frame > { > - /* The position of the most significant bit set in user-interrupt > - request register. */ > - unsigned long long uirrv; > /* RIP of the interrupted user process. */ > unsigned long long rip; > /* RFLAGS of the interrupted user process. */ > diff --git a/gcc/testsuite/gcc.dg/guality/pr98219-1.c > b/gcc/testsuite/gcc.dg/guality/pr98219-1.c > new file mode 100644 > index 00000000000..8d695080fd8 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/guality/pr98219-1.c > @@ -0,0 +1,48 @@ > +/* { dg-do run { target { { i?86-*-* x86_64-*-* } && { ! { ia32 } } } } } */ No need for extra curly braces after !, so: ... & { ! ia32 } ... here and in other tests. > +/* { dg-options "-g -muintr -mgeneral-regs-only" } */ > + > +#include <x86gprintrin.h> > + > +extern void exit (int); > + > +#define UIRRV 0x12345670 > +#define RIP 0x12345671 > +#define RFLAGS 0x12345672 > +#define RSP 0x12345673 > + > +#define STRING(x) XSTRING(x) > +#define XSTRING(x) #x > +#define ASMNAME(cname) ASMNAME2 (__USER_LABEL_PREFIX__, cname) > +#define ASMNAME2(prefix, cname) XSTRING (prefix) cname > + > +__attribute__((interrupt, used)) > +void > +fn (struct __uintr_frame *frame, unsigned long long uirrv) Please define and use typedef unsigned int uword_t __attribute__ ((mode (__word__))); as is the case in interrupt-3.c. Also, the convention is: void __attribute_((...)) fn (...) Please also fix in other tests. > +{ > + if (UIRRV != uirrv) /* BREAK */ > + __builtin_abort (); > + if (RIP != frame->rip) > + __builtin_abort (); > + if (RFLAGS != frame->rflags) > + __builtin_abort (); > + if (RSP != frame->rsp) > + __builtin_abort (); > + > + exit (0); > +} > + > +int > +main () > +{ > + asm ("push $" STRING (RSP) "; \ > + push $" STRING (RFLAGS) "; \ > + push $" STRING (RIP) "; \ > + push $" STRING (UIRRV) "; \ > + jmp " ASMNAME ("fn")); > + return 0; > +} > + > +/* { dg-final { gdb-test 22 "uirrv" "0x12345670" } } */ > +/* { dg-final { gdb-test 22 "frame->rip" "0x12345671" } } */ > +/* { dg-final { gdb-test 22 "frame->rflags" "0x12345672" } } */ > +/* { dg-final { gdb-test 22 "frame->rsp" "0x12345673" } } */ > diff --git a/gcc/testsuite/gcc.dg/guality/pr98219-2.c > b/gcc/testsuite/gcc.dg/guality/pr98219-2.c > new file mode 100644 > index 00000000000..c0e48c981de > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/guality/pr98219-2.c > @@ -0,0 +1,63 @@ > +/* { dg-do run { target { { i?86-*-* x86_64-*-* } && { ! { ia32 } } } } } */ > +/* { dg-options "-g -muintr -mgeneral-regs-only" } */ > + > +#include <x86gprintrin.h> > + > +extern void exit (int); > +typedef int aligned __attribute__((aligned(64))); > + > +#define UIRRV 0x12345670 > +#define RIP 0x12345671 > +#define RFLAGS 0x12345672 > +#define RSP 0x12345673 > + > +#define STRING(x) XSTRING(x) > +#define XSTRING(x) #x > +#define ASMNAME(cname) ASMNAME2 (__USER_LABEL_PREFIX__, cname) > +#define ASMNAME2(prefix, cname) XSTRING (prefix) cname > + > +int > +check_int (int *i, int align) > +{ > + *i = 20; > + if ((((ptrdiff_t) i) & (align - 1)) != 0) > + __builtin_abort (); > + return *i; > +} > + > +__attribute__((interrupt, used)) > +__attribute__((interrupt, used)) Two __attribute__s? Also, please follow convention as suggested above. > +void > +fn (struct __uintr_frame *frame, unsigned long long uirrv) > +{ > + aligned i; > + if (check_int (&i, __alignof__(i)) != i) > + __builtin_abort (); > + > + if (UIRRV != uirrv) /* BREAK */ > + __builtin_abort (); > + if (RIP != frame->rip) > + __builtin_abort (); > + if (RFLAGS != frame->rflags) > + __builtin_abort (); > + if (RSP != frame->rsp) > + __builtin_abort (); > + > + exit (0); > +} > + > +int > +main () > +{ > + asm ("push $" STRING (RSP) "; \ > + push $" STRING (RFLAGS) "; \ > + push $" STRING (RIP) "; \ > + push $" STRING (UIRRV) "; \ > + jmp " ASMNAME ("fn")); > + return 0; > +} > + > +/* { dg-final { gdb-test 34 "uirrv" "0x12345670" } } */ > +/* { dg-final { gdb-test 34 "frame->rip" "0x12345671" } } */ > +/* { dg-final { gdb-test 34 "frame->rflags" "0x12345672" } } */ > +/* { dg-final { gdb-test 34 "frame->rsp" "0x12345673" } } */ > diff --git a/gcc/testsuite/gcc.dg/torture/pr98219-1.c > b/gcc/testsuite/gcc.dg/torture/pr98219-1.c > new file mode 100644 > index 00000000000..c78495cc1c0 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr98219-1.c > @@ -0,0 +1,44 @@ > +/* { dg-do run { target { { i?86-*-* x86_64-*-* } && { ! { ia32 } } } } } */ > +/* { dg-skip-if "PR81210 sp not aligned to 16 bytes" { *-*-darwin* } } */ > +/* { dg-options "-muintr -mgeneral-regs-only" } */ > + > +#include <x86gprintrin.h> > + > +extern void exit (int); > + > +#define UIRRV 0x12345670 > +#define RIP 0x12345671 > +#define RFLAGS 0x12345672 > +#define RSP 0x12345673 > + > +#define STRING(x) XSTRING(x) > +#define XSTRING(x) #x > +#define ASMNAME(cname) ASMNAME2 (__USER_LABEL_PREFIX__, cname) > +#define ASMNAME2(prefix, cname) XSTRING (prefix) cname > + > +__attribute__((interrupt, used)) > +void > +fn (struct __uintr_frame *frame, unsigned long long uirrv) > +{ > + if (UIRRV != uirrv) > + __builtin_abort (); > + if (RIP != frame->rip) > + __builtin_abort (); > + if (RFLAGS != frame->rflags) > + __builtin_abort (); > + if (RSP != frame->rsp) > + __builtin_abort (); > + > + exit (0); > +} > + > +int > +main () > +{ > + asm ("push $" STRING (RSP) "; \ > + push $" STRING (RFLAGS) "; \ > + push $" STRING (RIP) "; \ > + push $" STRING (UIRRV) "; \ > + jmp " ASMNAME ("fn")); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.dg/torture/pr98219-2.c > b/gcc/testsuite/gcc.dg/torture/pr98219-2.c > new file mode 100644 > index 00000000000..176fb78e42e > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/torture/pr98219-2.c > @@ -0,0 +1,59 @@ > +/* { dg-do run { target { { i?86-*-* x86_64-*-* } && { ! { ia32 } } } } } */ > +/* { dg-skip-if "PR81210 sp not aligned to 16 bytes" { *-*-darwin* } } */ > +/* { dg-options "-muintr -mgeneral-regs-only" } */ > + > +#include <x86gprintrin.h> > + > +extern void exit (int); > +typedef int aligned __attribute__((aligned(64))); > + > +#define UIRRV 0x12345670 > +#define RIP 0x12345671 > +#define RFLAGS 0x12345672 > +#define RSP 0x12345673 > + > +#define STRING(x) XSTRING(x) > +#define XSTRING(x) #x > +#define ASMNAME(cname) ASMNAME2 (__USER_LABEL_PREFIX__, cname) > +#define ASMNAME2(prefix, cname) XSTRING (prefix) cname > + > +int > +check_int (int *i, int align) > +{ > + *i = 20; > + if ((((ptrdiff_t) i) & (align - 1)) != 0) > + __builtin_abort (); > + return *i; > +} > + > +__attribute__((interrupt, used)) > +__attribute__((interrupt, used)) > +void > +fn (struct __uintr_frame *frame, unsigned long long uirrv) > +{ > + aligned i; > + if (check_int (&i, __alignof__(i)) != i) > + __builtin_abort (); > + > + if (UIRRV != uirrv) > + __builtin_abort (); > + if (RIP != frame->rip) > + __builtin_abort (); > + if (RFLAGS != frame->rflags) > + __builtin_abort (); > + if (RSP != frame->rsp) > + __builtin_abort (); > + > + exit (0); > +} > + > +int > +main () > +{ > + asm ("push $" STRING (RSP) "; \ > + push $" STRING (RFLAGS) "; \ > + push $" STRING (RIP) "; \ > + push $" STRING (UIRRV) "; \ > + jmp " ASMNAME ("fn")); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.target/i386/uintr-2.c > b/gcc/testsuite/gcc.target/i386/uintr-2.c > index e705732c1bd..2ff32640827 100644 > --- a/gcc/testsuite/gcc.target/i386/uintr-2.c > +++ b/gcc/testsuite/gcc.target/i386/uintr-2.c > @@ -1,17 +1,18 @@ > /* { dg-do compile { target { ! ia32 } } } */ > /* { dg-options "-O2 -muintr -mgeneral-regs-only" } */ > /* { dg-final { scan-assembler-times "uiret" "2" } } */ > +/* { dg-final { scan-assembler-times "add\[lq]\[ \t]\+\\\$8, %\[er\]sp" "2" > } } */ > > #include <x86gprintrin.h> > > void > __attribute__((interrupt)) > -foo (void *frame) > +foo (void *frame, unsigned long long uirrv) > { > } > > void > __attribute__((interrupt)) > -UINTR_hanlder (struct __uintr_frame *frame) > +UINTR_hanlder (struct __uintr_frame *frame, unsigned long long uirrv) > { > } > diff --git a/gcc/testsuite/gcc.target/i386/uintr-3.c > b/gcc/testsuite/gcc.target/i386/uintr-3.c > index d2843495158..0b925066668 100644 > --- a/gcc/testsuite/gcc.target/i386/uintr-3.c > +++ b/gcc/testsuite/gcc.target/i386/uintr-3.c > @@ -1,9 +1,11 @@ > /* { dg-do compile { target { ! ia32 } } } */ > /* { dg-options "-O2 -muintr" } */ > /* { dg-final { scan-assembler "uiret" } } */ > +/* { dg-final { scan-assembler "add\[lq]\[ \t]\+\\\$8, %\[er\]sp" } } */ > + > #include <x86gprintrin.h> > > void __attribute__ ((target("general-regs-only"), interrupt)) > -UINTR_handler (struct __uintr_frame *p) > +UINTR_handler (struct __uintr_frame *p, unsigned long long uirrv) > { > } > diff --git a/gcc/testsuite/gcc.target/i386/uintr-4.c > b/gcc/testsuite/gcc.target/i386/uintr-4.c > index f3b371b4231..60478b126a7 100644 > --- a/gcc/testsuite/gcc.target/i386/uintr-4.c > +++ b/gcc/testsuite/gcc.target/i386/uintr-4.c > @@ -4,6 +4,6 @@ > #include <x86gprintrin.h> > > void __attribute__ ((interrupt)) > -UINTR_handler (struct __uintr_frame *p) > -{ /* { dg-message "SSE instructions aren't allowed in an interrupt service > routine" } */ > +UINTR_handler (struct __uintr_frame *p, unsigned long long uirrv) > +{ /* { dg-message "SSE instructions aren't allowed in an exception service > routine" } */ > } > diff --git a/gcc/testsuite/gcc.target/i386/uintr-5.c > b/gcc/testsuite/gcc.target/i386/uintr-5.c > index ac44be0a706..d49e54d3134 100644 > --- a/gcc/testsuite/gcc.target/i386/uintr-5.c > +++ b/gcc/testsuite/gcc.target/i386/uintr-5.c > @@ -5,6 +5,6 @@ > #include <x86gprintrin.h> > > void > -UINTR_hanlder (struct __uintr_frame *frame) > +UINTR_hanlder (struct __uintr_frame *frame, unsigned long long uirrv) > { > } > -- > 2.29.2 >