| Message ID | CAMe9rOqkihrjnk2sO16HWds3Jfoq7ao_Qi-VEUqk8Zeo_oeimA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86-64: Pass aggregates with only float/double in GPRs for MS_ABI | expand |
On Wed, Feb 5, 2020 at 6:59 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > MS_ABI requires passing aggregates with only float/double in integer > registers. Checked gcc outputs against Clang and fixed: > > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=54 > -Wno-unused-variable -Wno-unused-parameter > -Wno-unused-but-set-variable -Wno-uninitialized -O0 > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=54 > -Wno-unused-variable -Wno-unused-parameter > -Wno-unused-but-set-variable -Wno-uninitialized -O2 > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=55 > -Wno-unused-variable -Wno-unused-parameter > -Wno-unused-but-set-variable -Wno-uninitialized -O0 > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=55 > -Wno-unused-variable -Wno-unused-parameter > -Wno-unused-but-set-variable -Wno-uninitialized -O2 > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=56 > -Wno-unused-variable -Wno-unused-parameter > -Wno-unused-but-set-variable -Wno-uninitialized -O0 > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=56 > -Wno-unused-variable -Wno-unused-parameter > -Wno-unused-but-set-variable -Wno-uninitialized -O2 > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > > in libffi testsuite. > > OK for master and backports to GCC 8/9 branches? > > gcc/ > > PR target/85667 > * config/i386/i386.c (function_arg_ms_64): Add a type argument. > Don't return aggregates with only SFmode and DFmode in SSE > register. > (ix86_function_arg): Pass arg.type to function_arg_ms_64. > > gcc/testsuite/ > > PR target/85667 > * gcc.target/i386/pr85667-10.c: New test. > * gcc.target/i386/pr85667-7.c: Likewise. > * gcc.target/i386/pr85667-8.c: Likewise. > * gcc.target/i386/pr85667-9.c: Likewise. LGTM, but should really be reviewed by cygwin, mingw-w64 maintainer (CC'd). Thanks, Uros. > > -- > H.J.
On Wed, Feb 05, 2020 at 09:51:14PM +0100, Uros Bizjak wrote: > On Wed, Feb 5, 2020 at 6:59 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > MS_ABI requires passing aggregates with only float/double in integer > > registers. Checked gcc outputs against Clang and fixed: > > > > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=54 > > -Wno-unused-variable -Wno-unused-parameter > > -Wno-unused-but-set-variable -Wno-uninitialized -O0 > > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=54 > > -Wno-unused-variable -Wno-unused-parameter > > -Wno-unused-but-set-variable -Wno-uninitialized -O2 > > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=55 > > -Wno-unused-variable -Wno-unused-parameter > > -Wno-unused-but-set-variable -Wno-uninitialized -O0 > > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=55 > > -Wno-unused-variable -Wno-unused-parameter > > -Wno-unused-but-set-variable -Wno-uninitialized -O2 > > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=56 > > -Wno-unused-variable -Wno-unused-parameter > > -Wno-unused-but-set-variable -Wno-uninitialized -O0 > > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > > FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=56 > > -Wno-unused-variable -Wno-unused-parameter > > -Wno-unused-but-set-variable -Wno-uninitialized -O2 > > -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > > > > in libffi testsuite. > > > > OK for master and backports to GCC 8/9 branches? > > > > gcc/ > > > > PR target/85667 > > * config/i386/i386.c (function_arg_ms_64): Add a type argument. > > Don't return aggregates with only SFmode and DFmode in SSE > > register. > > (ix86_function_arg): Pass arg.type to function_arg_ms_64. > > > > gcc/testsuite/ > > > > PR target/85667 > > * gcc.target/i386/pr85667-10.c: New test. > > * gcc.target/i386/pr85667-7.c: Likewise. > > * gcc.target/i386/pr85667-8.c: Likewise. > > * gcc.target/i386/pr85667-9.c: Likewise. > > LGTM, but should really be reviewed by cygwin, mingw-w64 maintainer (CC'd). > I checked the result against MSVC v19.10 at https://godbolt.org/z/2NPygd My patch matches MSVC v19.10. I am checking it in tomorrow unless mingw-w64 maintainer objects. Thanks. H.J.
On 2/7/20 3:23 AM, H.J. Lu wrote: > On Wed, Feb 05, 2020 at 09:51:14PM +0100, Uros Bizjak wrote: >> On Wed, Feb 5, 2020 at 6:59 PM H.J. Lu <hjl.tools@gmail.com> wrote: >>> >>> MS_ABI requires passing aggregates with only float/double in integer >>> registers. Checked gcc outputs against Clang and fixed: >>> >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=54 >>> -Wno-unused-variable -Wno-unused-parameter >>> -Wno-unused-but-set-variable -Wno-uninitialized -O0 >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=54 >>> -Wno-unused-variable -Wno-unused-parameter >>> -Wno-unused-but-set-variable -Wno-uninitialized -O2 >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=55 >>> -Wno-unused-variable -Wno-unused-parameter >>> -Wno-unused-but-set-variable -Wno-uninitialized -O0 >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=55 >>> -Wno-unused-variable -Wno-unused-parameter >>> -Wno-unused-but-set-variable -Wno-uninitialized -O2 >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=56 >>> -Wno-unused-variable -Wno-unused-parameter >>> -Wno-unused-but-set-variable -Wno-uninitialized -O0 >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=56 >>> -Wno-unused-variable -Wno-unused-parameter >>> -Wno-unused-but-set-variable -Wno-uninitialized -O2 >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test >>> >>> in libffi testsuite. >>> >>> OK for master and backports to GCC 8/9 branches? >>> >>> gcc/ >>> >>> PR target/85667 >>> * config/i386/i386.c (function_arg_ms_64): Add a type argument. >>> Don't return aggregates with only SFmode and DFmode in SSE >>> register. >>> (ix86_function_arg): Pass arg.type to function_arg_ms_64. >>> >>> gcc/testsuite/ >>> >>> PR target/85667 >>> * gcc.target/i386/pr85667-10.c: New test. >>> * gcc.target/i386/pr85667-7.c: Likewise. >>> * gcc.target/i386/pr85667-8.c: Likewise. >>> * gcc.target/i386/pr85667-9.c: Likewise. >> >> LGTM, but should really be reviewed by cygwin, mingw-w64 maintainer (CC'd). >> > > I checked the result against MSVC v19.10 at > > https://godbolt.org/z/2NPygd > > My patch matches MSVC v19.10. I am checking it in tomorrow unless > mingw-w64 maintainer objects. > Please go ahead and thanks.
On Fri, Feb 7, 2020 at 2:14 AM JonY <10walls@gmail.com> wrote: > > On 2/7/20 3:23 AM, H.J. Lu wrote: > > On Wed, Feb 05, 2020 at 09:51:14PM +0100, Uros Bizjak wrote: > >> On Wed, Feb 5, 2020 at 6:59 PM H.J. Lu <hjl.tools@gmail.com> wrote: > >>> > >>> MS_ABI requires passing aggregates with only float/double in integer > >>> registers. Checked gcc outputs against Clang and fixed: > >>> > >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=54 > >>> -Wno-unused-variable -Wno-unused-parameter > >>> -Wno-unused-but-set-variable -Wno-uninitialized -O0 > >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=54 > >>> -Wno-unused-variable -Wno-unused-parameter > >>> -Wno-unused-but-set-variable -Wno-uninitialized -O2 > >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=55 > >>> -Wno-unused-variable -Wno-unused-parameter > >>> -Wno-unused-but-set-variable -Wno-uninitialized -O0 > >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=55 > >>> -Wno-unused-variable -Wno-unused-parameter > >>> -Wno-unused-but-set-variable -Wno-uninitialized -O2 > >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=56 > >>> -Wno-unused-variable -Wno-unused-parameter > >>> -Wno-unused-but-set-variable -Wno-uninitialized -O0 > >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > >>> FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=56 > >>> -Wno-unused-variable -Wno-unused-parameter > >>> -Wno-unused-but-set-variable -Wno-uninitialized -O2 > >>> -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test > >>> > >>> in libffi testsuite. > >>> > >>> OK for master and backports to GCC 8/9 branches? > >>> > >>> gcc/ > >>> > >>> PR target/85667 > >>> * config/i386/i386.c (function_arg_ms_64): Add a type argument. > >>> Don't return aggregates with only SFmode and DFmode in SSE > >>> register. > >>> (ix86_function_arg): Pass arg.type to function_arg_ms_64. > >>> > >>> gcc/testsuite/ > >>> > >>> PR target/85667 > >>> * gcc.target/i386/pr85667-10.c: New test. > >>> * gcc.target/i386/pr85667-7.c: Likewise. > >>> * gcc.target/i386/pr85667-8.c: Likewise. > >>> * gcc.target/i386/pr85667-9.c: Likewise. > >> > >> LGTM, but should really be reviewed by cygwin, mingw-w64 maintainer (CC'd). > >> > > > > I checked the result against MSVC v19.10 at > > > > https://godbolt.org/z/2NPygd > > > > My patch matches MSVC v19.10. I am checking it in tomorrow unless > > mingw-w64 maintainer objects. > > > > Please go ahead and thanks. > I checked it into master and backported it to releases/gcc-9 branch. No plan to fix releases/gcc-8 branch. Thanks.
From e561fd8fcb46b8d8e40942c077e26ce120832747 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Wed, 5 Feb 2020 09:49:56 -0800 Subject: [PATCH] x86-64: Pass aggregates with only float/double in GPRs for MS_ABI MS_ABI requires passing aggregates with only float/double in integer registers. Checked gcc outputs against Clang and fixed: FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=54 -Wno-unused-variable -Wno-unused-parameter -Wno-unused-but-set-variable -Wno-uninitialized -O0 -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=54 -Wno-unused-variable -Wno-unused-parameter -Wno-unused-but-set-variable -Wno-uninitialized -O2 -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=55 -Wno-unused-variable -Wno-unused-parameter -Wno-unused-but-set-variable -Wno-uninitialized -O0 -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=55 -Wno-unused-variable -Wno-unused-parameter -Wno-unused-but-set-variable -Wno-uninitialized -O2 -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=56 -Wno-unused-variable -Wno-unused-parameter -Wno-unused-but-set-variable -Wno-uninitialized -O0 -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test FAIL: libffi.bhaible/test-callback.c -W -Wall -Wno-psabi -DDGTEST=56 -Wno-unused-variable -Wno-unused-parameter -Wno-unused-but-set-variable -Wno-uninitialized -O2 -DABI_NUM=FFI_GNUW64 -DABI_ATTR=MSABI execution test in libffi testsuite. gcc/ PR target/85667 * config/i386/i386.c (function_arg_ms_64): Add a type argument. Don't return aggregates with only SFmode and DFmode in SSE register. (ix86_function_arg): Pass arg.type to function_arg_ms_64. gcc/testsuite/ PR target/85667 * gcc.target/i386/pr85667-10.c: New test. * gcc.target/i386/pr85667-7.c: Likewise. * gcc.target/i386/pr85667-8.c: Likewise. * gcc.target/i386/pr85667-9.c: Likewise. --- gcc/config/i386/i386.c | 10 ++++-- gcc/testsuite/gcc.target/i386/pr85667-10.c | 21 +++++++++++++ gcc/testsuite/gcc.target/i386/pr85667-7.c | 36 ++++++++++++++++++++++ gcc/testsuite/gcc.target/i386/pr85667-8.c | 21 +++++++++++++ gcc/testsuite/gcc.target/i386/pr85667-9.c | 36 ++++++++++++++++++++++ 5 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr85667-10.c create mode 100644 gcc/testsuite/gcc.target/i386/pr85667-7.c create mode 100644 gcc/testsuite/gcc.target/i386/pr85667-8.c create mode 100644 gcc/testsuite/gcc.target/i386/pr85667-9.c diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index ffda3e8fd21..f769cb8f75e 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -3153,7 +3153,7 @@ function_arg_64 (const CUMULATIVE_ARGS *cum, machine_mode mode, static rtx function_arg_ms_64 (const CUMULATIVE_ARGS *cum, machine_mode mode, - machine_mode orig_mode, bool named, + machine_mode orig_mode, bool named, const_tree type, HOST_WIDE_INT bytes) { unsigned int regno; @@ -3173,7 +3173,10 @@ function_arg_ms_64 (const CUMULATIVE_ARGS *cum, machine_mode mode, if (TARGET_SSE && (mode == SFmode || mode == DFmode)) { if (named) - regno = cum->regno + FIRST_SSE_REG; + { + if (type == NULL_TREE || !AGGREGATE_TYPE_P (type)) + regno = cum->regno + FIRST_SSE_REG; + } else { rtx t1, t2; @@ -3253,7 +3256,8 @@ ix86_function_arg (cumulative_args_t cum_v, const function_arg_info &arg) enum calling_abi call_abi = cum ? cum->call_abi : ix86_abi; if (call_abi == MS_ABI) - reg = function_arg_ms_64 (cum, mode, arg.mode, arg.named, bytes); + reg = function_arg_ms_64 (cum, mode, arg.mode, arg.named, + arg.type, bytes); else reg = function_arg_64 (cum, mode, arg.mode, arg.type, arg.named); } diff --git a/gcc/testsuite/gcc.target/i386/pr85667-10.c b/gcc/testsuite/gcc.target/i386/pr85667-10.c new file mode 100644 index 00000000000..e8f3026227d --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr85667-10.c @@ -0,0 +1,21 @@ +/* { dg-do compile { target lp64 } } */ +/* { dg-options "-O2 -masm=att" } */ +/* { dg-final { scan-assembler-times "movq\[ \t\]*%rcx, .*" 1 } } */ +/* { dg-final { scan-assembler-times "movq\[ \t\]*%rdx, .*" 1 } } */ +/* { dg-final { scan-assembler-times "movq\[ \t\]*%r8, .*" 1 } } */ +/* { dg-final { scan-assembler-times "movq\[ \t\]*%r9, .*" 1 } } */ +/* { dg-final { scan-assembler-times "addsd\[ \t]*40\\\(%rsp\\\), .*" 1 } } */ +/* { dg-final { scan-assembler-times "movq\[^\n\r\]*, %rax" 1 } } */ + +typedef struct +{ + double x; +} Double; + +Double __attribute__((ms_abi)) +fn1 (Double x1, Double x2, Double x3, Double x4, Double x5) +{ + Double v; + v.x = x1.x + x2.x + x3.x + x4.x + x5.x; + return v; +} diff --git a/gcc/testsuite/gcc.target/i386/pr85667-7.c b/gcc/testsuite/gcc.target/i386/pr85667-7.c new file mode 100644 index 00000000000..6bd860975a7 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr85667-7.c @@ -0,0 +1,36 @@ +/* { dg-do run { target lp64 } } */ +/* { dg-options "-O2" } */ + +void abort (void); + +typedef struct +{ + float x; +} Float; + +Float __attribute__((ms_abi, noinline, noclone)) +fn1 (Float x1, Float x2, Float x3, Float x4, Float x5) +{ + Float v; + v.x = x1.x + x2.x + x3.x + x4.x + x5.x; + return v; +} +int main () +{ + Float a, a1, a2, a3, a4, a5; + float x1 = 1.1; + float x2 = 3.1; + float x3 = 4.2; + float x4 = 14.2; + float x5 = -7.2; + float x = x1 + x2 + x3 + x4 + x5; + a1.x = x1; + a2.x = x2; + a3.x = x3; + a4.x = x4; + a5.x = x5; + a = fn1 (a1, a2, a3, a4, a5); + if (a.x == x); + return 0; + abort (); +} diff --git a/gcc/testsuite/gcc.target/i386/pr85667-8.c b/gcc/testsuite/gcc.target/i386/pr85667-8.c new file mode 100644 index 00000000000..09a7593fdb3 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr85667-8.c @@ -0,0 +1,21 @@ +/* { dg-do compile { target lp64 } } */ +/* { dg-options "-O2 -masm=att" } */ +/* { dg-final { scan-assembler-times "movd\[ \t\]*%ecx, .*" 1 } } */ +/* { dg-final { scan-assembler-times "movd\[ \t\]*%edx, .*" 1 } } */ +/* { dg-final { scan-assembler-times "movd\[ \t\]*%r8d, .*" 1 } } */ +/* { dg-final { scan-assembler-times "movd\[ \t\]*%r9d, .*" 1 } } */ +/* { dg-final { scan-assembler-times "addss\[ \t]*40\\\(%rsp\\\), .*" 1 } } */ +/* { dg-final { scan-assembler-times "movd\[^\n\r\]*, %eax" 1 } } */ + +typedef struct +{ + float x; +} Float; + +Float __attribute__((ms_abi)) +fn1 (Float x1, Float x2, Float x3, Float x4, Float x5) +{ + Float v; + v.x = x1.x + x2.x + x3.x + x4.x + x5.x; + return v; +} diff --git a/gcc/testsuite/gcc.target/i386/pr85667-9.c b/gcc/testsuite/gcc.target/i386/pr85667-9.c new file mode 100644 index 00000000000..8c9279a18bf --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr85667-9.c @@ -0,0 +1,36 @@ +/* { dg-do run { target lp64 } } */ +/* { dg-options "-O2" } */ + +void abort (void); + +typedef struct +{ + double x; +} Double; + +Double __attribute__((ms_abi, noinline, noclone)) +fn1 (Double x1, Double x2, Double x3, Double x4, Double x5) +{ + Double v; + v.x = x1.x + x2.x + x3.x + x4.x + x5.x; + return v; +} +int main () +{ + Double a, a1, a2, a3, a4, a5; + double x1 = 1.1; + double x2 = 3.1; + double x3 = 4.2; + double x4 = 14.2; + double x5 = -7.2; + double x = x1 + x2 + x3 + x4 + x5; + a1.x = x1; + a2.x = x2; + a3.x = x3; + a4.x = x4; + a5.x = x5; + a = fn1 (a1, a2, a3, a4, a5); + if (a.x == x); + return 0; + abort (); +} -- 2.24.1