* [PATCH] x86: Revert patches to fix PR target/88794 @ 2019-01-15 14:40 Wei Xiao 2019-01-15 15:14 ` Uros Bizjak 0 siblings, 1 reply; 9+ messages in thread From: Wei Xiao @ 2019-01-15 14:40 UTC (permalink / raw) To: ubizjak Cc: Jakub Jelinek, hongjiu.lu, hjl.tools, kretz, xuepeng.guo, gcc-patches Hi, It turns out that the Intel 64 and IA-32 Architectures Software Developer Manuals (SDM) description about the fixupimm intrinsic is incorrect. So we need to revert 3 patches related to it: r265827, r266026 and r267160. Sorry for the inconvenience. Is it ok? Wei ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Revert patches to fix PR target/88794 2019-01-15 14:40 [PATCH] x86: Revert patches to fix PR target/88794 Wei Xiao @ 2019-01-15 15:14 ` Uros Bizjak 2019-01-15 15:20 ` Jakub Jelinek 0 siblings, 1 reply; 9+ messages in thread From: Uros Bizjak @ 2019-01-15 15:14 UTC (permalink / raw) To: Wei Xiao Cc: Jakub Jelinek, Lu, Hongjiu, H. J. Lu, kretz, Guo, Xuepeng, gcc-patches On Tue, Jan 15, 2019 at 3:40 PM Wei Xiao <wei.william.xiao@gmail.com> wrote: > > Hi, > > It turns out that the Intel 64 and IA-32 Architectures Software Developer > Manuals (SDM) description about the fixupimm intrinsic is incorrect. So we need > to revert 3 patches related to it: r265827, r266026 and r267160. > Sorry for the inconvenience. > > Is it ok? Yes, but please test the compiler after the revert. Please also create a runtime testcase out of the testcase in the PR. Thanks, Uros. > Wei ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Revert patches to fix PR target/88794 2019-01-15 15:14 ` Uros Bizjak @ 2019-01-15 15:20 ` Jakub Jelinek 2019-01-16 11:13 ` Wei Xiao 0 siblings, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2019-01-15 15:20 UTC (permalink / raw) To: Uros Bizjak Cc: Wei Xiao, Lu, Hongjiu, H. J. Lu, kretz, Guo, Xuepeng, gcc-patches On Tue, Jan 15, 2019 at 04:14:06PM +0100, Uros Bizjak wrote: > On Tue, Jan 15, 2019 at 3:40 PM Wei Xiao <wei.william.xiao@gmail.com> wrote: > > > > Hi, > > > > It turns out that the Intel 64 and IA-32 Architectures Software Developer > > Manuals (SDM) description about the fixupimm intrinsic is incorrect. So we need > > to revert 3 patches related to it: r265827, r266026 and r267160. > > Sorry for the inconvenience. > > > > Is it ok? > > Yes, but please test the compiler after the revert. Please also create > a runtime testcase out of the testcase in the PR. For r267160, I'd expect you want to revert just the config/i386/ part and keep the testcases, they should work even with the changes reverted, right? Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Revert patches to fix PR target/88794 2019-01-15 15:20 ` Jakub Jelinek @ 2019-01-16 11:13 ` Wei Xiao 2019-01-16 14:48 ` Wei Xiao 2019-01-17 13:03 ` [PATCH] Read avx512vl-vfixupimms*-2.c testcases (PR target/88489) Jakub Jelinek 0 siblings, 2 replies; 9+ messages in thread From: Wei Xiao @ 2019-01-16 11:13 UTC (permalink / raw) To: Jakub Jelinek Cc: Uros Bizjak, Lu, Hongjiu, H. J. Lu, kretz, Guo, Xuepeng, gcc-patches, wei3.xiao > > Yes, but please test the compiler after the revert. Please also create > > a runtime testcase out of the testcase in the PR. Yes, we have tested it but current runtime testcase can't cover the corner case to expose the incorrectness of SDM. We will add some after the revert. > For r267160, I'd expect you want to revert just the config/i386/ part and > keep the testcases, they should work even with the changes reverted, right? > The testcase part also need to be reverted since we have changed them according to the incorrect intrinsic list in SDM. Jakub Jelinek <jakub@redhat.com> 于2019年1月15日周二 下午11:20写道： > > On Tue, Jan 15, 2019 at 04:14:06PM +0100, Uros Bizjak wrote: > > On Tue, Jan 15, 2019 at 3:40 PM Wei Xiao <wei.william.xiao@gmail.com> wrote: > > > > > > Hi, > > > > > > It turns out that the Intel 64 and IA-32 Architectures Software Developer > > > Manuals (SDM) description about the fixupimm intrinsic is incorrect. So we need > > > to revert 3 patches related to it: r265827, r266026 and r267160. > > > Sorry for the inconvenience. > > > > > > Is it ok? > > > > Yes, but please test the compiler after the revert. Please also create > > a runtime testcase out of the testcase in the PR. > > For r267160, I'd expect you want to revert just the config/i386/ part and > keep the testcases, they should work even with the changes reverted, right? > > Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Revert patches to fix PR target/88794 2019-01-16 11:13 ` Wei Xiao @ 2019-01-16 14:48 ` Wei Xiao 2019-01-16 15:03 ` Jakub Jelinek 2019-01-17 13:03 ` [PATCH] Read avx512vl-vfixupimms*-2.c testcases (PR target/88489) Jakub Jelinek 1 sibling, 1 reply; 9+ messages in thread From: Wei Xiao @ 2019-01-16 14:48 UTC (permalink / raw) To: Jakub Jelinek Cc: Uros Bizjak, Lu, Hongjiu, H. J. Lu, M Kretz, Guo, Xuepeng, gcc-patches, wei3.xiao [-- Attachment #1: Type: text/plain, Size: 469 bytes --] The original runtime testcases are incorrect and I have fixed them as attached. Is it ok to do the revert and fix the testcases for trunk? Wei 2019-01-16 Wei Xiao <wei3.xiao@intel.com> * gcc.target/i386/avx512f-vfixupimmpd-2.c: Fix the test cases for VFIXUPIMM* intrinsics. * gcc.target/i386/avx512f-vfixupimmps-2.c: Ditto. * gcc.target/i386/avx512f-vfixupimmsd-2.c: Ditto. * gcc.target/i386/avx512f-vfixupimmss-2.c: Ditto. [-- Attachment #2: fixupimm_testcases.diff --] [-- Type: application/octet-stream, Size: 5207 bytes --] diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index f032b48..20ed218 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2019-01-16 Wei Xiao <wei3.xiao@intel.com> + + * gcc.target/i386/avx512f-vfixupimmpd-2.c: Fix the test cases for + VFIXUPIMM* intrinsics. + * gcc.target/i386/avx512f-vfixupimmps-2.c: Ditto. + * gcc.target/i386/avx512f-vfixupimmsd-2.c: Ditto. + * gcc.target/i386/avx512f-vfixupimmss-2.c: Ditto. + 2019-01-15 Nikhil Benesch <nikhil.benesch@gmail.com> * gcc.misc-tests/godump-1.c: Add test case for typedef before diff --git a/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmpd-2.c b/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmpd-2.c index 7ce9806..98b5ed1 100644 --- a/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmpd-2.c +++ b/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmpd-2.c @@ -14,12 +14,12 @@ static void -CALC (double *r, double src, long long tbl) +CALC (double *r, double dest, double src, long long tbl) { switch (tbl & 0xf) { case 0: - *r = src; + *r = dest; break; case 1: *r = src; @@ -81,7 +81,7 @@ TEST (void) float vals[2] = { -10, 10 }; - int controls[8] = {0x11111111, 0x77777777, 0x77777777, 0x88888888, + int controls[8] = {0, 0x11111111, 0x77777777, 0x88888888, 0x99999999, 0xaaaaaaaa, 0xbbbbbbbb, 0xcccccccc}; MASK_TYPE mask = MASK_VALUE; @@ -96,7 +96,7 @@ TEST (void) res2.a[j] = DEFAULT_VALUE; res3.a[j] = DEFAULT_VALUE; - CALC (&res_ref[j], s1.a[j], s2.a[j]); + CALC (&res_ref[j], res1.a[j], s1.a[j], s2.a[j]); } res1.x = INTRINSIC (_fixupimm_pd) (res1.x, s1.x, s2.x, 0); diff --git a/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmps-2.c b/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmps-2.c index ffd899e..e5a917f 100644 --- a/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmps-2.c +++ b/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmps-2.c @@ -13,12 +13,12 @@ #include "float.h" static void -CALC (float *r, float src, int tbl) +CALC (float *r, float dest, float src, int tbl) { switch (tbl & 0xf) { case 0: - *r = src; + *r = dest; break; case 1: *r = src; @@ -81,8 +81,8 @@ TEST (void) float vals[2] = { -10, 10 }; - int controls[16] = { 0x11111111, - 0x77777777, 0x88888888, 0x99999999, + int controls[16] = { 0, + 0x11111111, 0x88888888, 0x99999999, 0xaaaaaaaa, 0xbbbbbbbb, 0xcccccccc, 0x77777777, 0x88888888, 0x99999999, 0xaaaaaaaa, 0xbbbbbbbb, 0xcccccccc, @@ -101,7 +101,7 @@ TEST (void) res2.a[j] = DEFAULT_VALUE; res3.a[j] = DEFAULT_VALUE; - CALC (&res_ref[j], s1.a[j], s2.a[j]); + CALC (&res_ref[j], res1.a[j], s1.a[j], s2.a[j]); } res1.x = INTRINSIC (_fixupimm_ps) (res1.x, s1.x, s2.x, 0); diff --git a/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmsd-2.c b/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmsd-2.c index 62c8d4e..d3cd28c 100644 --- a/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmsd-2.c +++ b/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmsd-2.c @@ -10,12 +10,12 @@ #include "avx512f-mask-type.h" void -compute_fixupimmpd (double *r, double src, long long tbl) +compute_fixupimmpd (double *r, double dest, double src, long long tbl) { switch (tbl & 0xf) { case 0: - *r = src; + *r = dest; break; case 1: *r = src; @@ -76,7 +76,7 @@ avx512f_test (void) int i, j; float vals[2] = { -10, 10 }; - int controls[10] = { 0x11111111, + int controls[10] = { 0, 0x77777777, 0x88888888, 0x99999999, 0xaaaaaaaa, 0xbbbbbbbb, 0xcccccccc, 0xdddddddd, 0xeeeeeeee, 0xffffffff @@ -98,7 +98,7 @@ avx512f_test (void) for (j = 0; j < 10; j++) { s2.a[0] = controls[j]; - compute_fixupimmpd (&res_ref[0], s1.a[0], s2.a[0]); + compute_fixupimmpd (&res_ref[0], res1.a[0], s1.a[0], s2.a[0]); res1.x = _mm_fixupimm_sd (res1.x, s1.x, s2.x, 0); res2.x = _mm_mask_fixupimm_sd (res2.x, mask, s1.x, s2.x, 0); diff --git a/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmss-2.c b/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmss-2.c index 26f45a0..7364cc5 100644 --- a/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmss-2.c +++ b/gcc/testsuite/gcc.target/i386/avx512f-vfixupimmss-2.c @@ -10,12 +10,12 @@ #include "avx512f-mask-type.h" void -compute_fixupimmps (float *r, float src, int tbl) +compute_fixupimmps (float *r, float dest, float src, int tbl) { switch (tbl & 0xf) { case 0: - *r = src; + *r = dest; break; case 1: *r = src; @@ -76,7 +76,7 @@ avx512f_test (void) int i, j, k; float vals[2] = { -10, 10 }; - int controls[10] = { 0x11111111, + int controls[10] = { 0, 0x77777777, 0x88888888, 0x99999999, 0xaaaaaaaa, 0xbbbbbbbb, 0xcccccccc, 0xdddddddd, 0xeeeeeeee, 0xffffffff @@ -99,7 +99,7 @@ avx512f_test (void) for (j = 0; j < 10; j++) { s2.a[0] = controls[j]; - compute_fixupimmps (&res_ref[0], s1.a[0], s2.a[0]); + compute_fixupimmps (&res_ref[0], res1.a[0], s1.a[0], s2.a[0]); res1.x = _mm_fixupimm_ss (res1.x, s1.x, s2.x, 0); res2.x = _mm_mask_fixupimm_ss (res2.x, mask, s1.x, s2.x, 0); ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] x86: Revert patches to fix PR target/88794 2019-01-16 14:48 ` Wei Xiao @ 2019-01-16 15:03 ` Jakub Jelinek 0 siblings, 0 replies; 9+ messages in thread From: Jakub Jelinek @ 2019-01-16 15:03 UTC (permalink / raw) To: Wei Xiao Cc: Uros Bizjak, Lu, Hongjiu, H. J. Lu, M Kretz, Guo, Xuepeng, gcc-patches, wei3.xiao On Wed, Jan 16, 2019 at 10:48:28PM +0800, Wei Xiao wrote: > The original runtime testcases are incorrect and I have fixed them as attached. > Is it ok to do the revert and fix the testcases for trunk? LGTM. > 2019-01-16 Wei Xiao <wei3.xiao@intel.com> > > * gcc.target/i386/avx512f-vfixupimmpd-2.c: Fix the test cases for > VFIXUPIMM* intrinsics. > * gcc.target/i386/avx512f-vfixupimmps-2.c: Ditto. > * gcc.target/i386/avx512f-vfixupimmsd-2.c: Ditto. > * gcc.target/i386/avx512f-vfixupimmss-2.c: Ditto. Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] Read avx512vl-vfixupimms*-2.c testcases (PR target/88489) 2019-01-16 11:13 ` Wei Xiao 2019-01-16 14:48 ` Wei Xiao @ 2019-01-17 13:03 ` Jakub Jelinek 2019-01-18 2:42 ` Wei Xiao 1 sibling, 1 reply; 9+ messages in thread From: Jakub Jelinek @ 2019-01-17 13:03 UTC (permalink / raw) To: Uros Bizjak, Wei Xiao Cc: Lu, Hongjiu, H. J. Lu, kretz, Guo, Xuepeng, gcc-patches, wei3.xiao On Wed, Jan 16, 2019 at 07:12:56PM +0800, Wei Xiao wrote: > > > Yes, but please test the compiler after the revert. Please also create > > > a runtime testcase out of the testcase in the PR. > Yes, we have tested it but current runtime testcase can't cover the corner > case to expose the incorrectness of SDM. We will add some after the revert. > > > For r267160, I'd expect you want to revert just the config/i386/ part and > > keep the testcases, they should work even with the changes reverted, right? > > > The testcase part also need to be reverted since we have changed them > according to the incorrect intrinsic list in SDM. I don't really understand this. The testcases succeed just fine for me in the current trunk with all the reversions and test something the current state of the testsuite doesn't check normally, in particular that the testcases run correctly even when -mavx512vl is used. As that misbehaved in the past, we should make sure we don't break that again. Uros, is it ok to reapply this to current trunk? 2019-01-17 Jakub Jelinek <jakub@redhat.com> Reapply: 2018-12-15 Jakub Jelinek <jakub@redhat.com> PR target/88489 * gcc.target/i386/avx512vl-vfixupimmsd-2.c: New test. * gcc.target/i386/avx512vl-vfixupimmss-2.c: New test. --- gcc/testsuite/gcc.target/i386/avx512vl-vfixupimmsd-2.c (nonexistent) +++ gcc/testsuite/gcc.target/i386/avx512vl-vfixupimmsd-2.c (revision 268010) @@ -0,0 +1,20 @@ +/* { dg-do run } */ +/* { dg-options "-mavx512vl -O2 -std=gnu99" } */ +/* { dg-require-effective-target avx512vl } */ +/* { dg-require-effective-target c99_runtime } */ + +#define AVX512VL +#define AVX512F_LEN 512 +#define AVX512F_LEN_HALF 256 +#include "avx512f-vfixupimmsd-2.c" + +static void +test_256 (void) +{ + test_512 (); +} + +static void +test_128 (void) +{ +} --- gcc/testsuite/gcc.target/i386/avx512vl-vfixupimmss-2.c (nonexistent) +++ gcc/testsuite/gcc.target/i386/avx512vl-vfixupimmss-2.c (revision 268010) @@ -0,0 +1,20 @@ +/* { dg-do run } */ +/* { dg-options "-mavx512vl -O2 -std=gnu99" } */ +/* { dg-require-effective-target avx512vl } */ +/* { dg-require-effective-target c99_runtime } */ + +#define AVX512VL +#define AVX512F_LEN 512 +#define AVX512F_LEN_HALF 256 +#include "avx512f-vfixupimmss-2.c" + +static void +test_256 (void) +{ + test_512 (); +} + +static void +test_128 (void) +{ +} Jakub ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Read avx512vl-vfixupimms*-2.c testcases (PR target/88489) 2019-01-17 13:03 ` [PATCH] Read avx512vl-vfixupimms*-2.c testcases (PR target/88489) Jakub Jelinek @ 2019-01-18 2:42 ` Wei Xiao 2019-01-18 7:55 ` Uros Bizjak 0 siblings, 1 reply; 9+ messages in thread From: Wei Xiao @ 2019-01-18 2:42 UTC (permalink / raw) To: Jakub Jelinek Cc: Uros Bizjak, Lu, Hongjiu, H. J. Lu, M Kretz, Guo, Xuepeng, gcc-patches, wei3.xiao > > > For r267160, I'd expect you want to revert just the config/i386/ part and > > > keep the testcases, they should work even with the changes reverted, right? > > > > > The testcase part also need to be reverted since we have changed them > > according to the incorrect intrinsic list in SDM. > > I don't really understand this. > > The testcases succeed just fine for me in the current trunk with all the > reversions and test something the current state of the testsuite doesn't > check normally, in particular that the testcases run correctly even when > -mavx512vl is used. As that misbehaved in the past, we should make sure we > don't break that again. > You're right. The testcases need to be kept to prevent regression. > Uros, is it ok to reapply this to current trunk? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Read avx512vl-vfixupimms*-2.c testcases (PR target/88489) 2019-01-18 2:42 ` Wei Xiao @ 2019-01-18 7:55 ` Uros Bizjak 0 siblings, 0 replies; 9+ messages in thread From: Uros Bizjak @ 2019-01-18 7:55 UTC (permalink / raw) To: Wei Xiao Cc: Jakub Jelinek, Lu, Hongjiu, H. J. Lu, M Kretz, Guo, Xuepeng, gcc-patches, wei3.xiao On Fri, Jan 18, 2019 at 3:42 AM Wei Xiao <wei.william.xiao@gmail.com> wrote: > > > > > For r267160, I'd expect you want to revert just the config/i386/ part and > > > > keep the testcases, they should work even with the changes reverted, right? > > > > > > > The testcase part also need to be reverted since we have changed them > > > according to the incorrect intrinsic list in SDM. > > > > I don't really understand this. > > > > The testcases succeed just fine for me in the current trunk with all the > > reversions and test something the current state of the testsuite doesn't > > check normally, in particular that the testcases run correctly even when > > -mavx512vl is used. As that misbehaved in the past, we should make sure we > > don't break that again. > > > > You're right. The testcases need to be kept to prevent regression. > > > Uros, is it ok to reapply this to current trunk? Yes, please reapply the tests. Thanks, Uros. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-01-18 7:55 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-15 14:40 [PATCH] x86: Revert patches to fix PR target/88794 Wei Xiao 2019-01-15 15:14 ` Uros Bizjak 2019-01-15 15:20 ` Jakub Jelinek 2019-01-16 11:13 ` Wei Xiao 2019-01-16 14:48 ` Wei Xiao 2019-01-16 15:03 ` Jakub Jelinek 2019-01-17 13:03 ` [PATCH] Read avx512vl-vfixupimms*-2.c testcases (PR target/88489) Jakub Jelinek 2019-01-18 2:42 ` Wei Xiao 2019-01-18 7:55 ` Uros Bizjak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).