diff mbox

[AArch64,testsuite] PR target/70004: Remove check using undefined behaviour

Message ID 56D96640.9060704@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov March 4, 2016, 10:41 a.m. UTC 
On 02/03/16 16:17, Jakub Jelinek wrote:
> On Tue, Mar 01, 2016 at 02:29:19PM +0000, Kyrill Tkachov wrote:
>> This test was reported as starting to fail a scan-assembler check with -mtune=thunderx.
>> The compiler decided to move the result back into the integer registers and perform the shift there
>> rather than do it on the SIMD side.
>> However, the C code is undefined because the shift is wider than the type. GCC even warns for it, but the
>> test catches it with dg-warning.
>> I don't think it's correct for the test to rely on undefined code, so the simplest thing to do is to delete
>> the undefined code.
>>
>> Ok for trunk?
>>
>> Thanks,
>> Kyrill
>>
>> 2016-03-01  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>      PR target/70004
>>      * gcc.target/aarch64/scalar_shift_1.c: (test_corners_sisd_di):
>>      Delete.
>>      (test_corners_sisd_si): Likewise.
>>      (main): Remove checks of the above.
> I think you should extend this patch, by adding the removed functions
> into another, dg-do compile only test without any scan-assembler stuff in
> there, just to make sure you don't ICE on it and emit the warnings that
> should be emitted, and possibly by keeping the well defined corner case
> stuff in there instead of removing the functions altogether.  b >> 63 and
> b >> 0 are both well defined, similarly b >> 31 and b >> 0 in the other
> function.

Can do. I've moved the dodgy functions into their own separate compile test.
The test passes.
Ok?

Thanks,
Kyrill

2016-03-04  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/70004
     * gcc.target/aarch64/scalar_shift_1.c: (test_corners_sisd_di):
     Delete.
     (test_corners_sisd_si): Likewise.
     (main): Remove checks of the above.
     * gcc.target/aarch64/shift_wide_invalid_1.c: New test.

>> diff --git a/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c b/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
>> index 8465c896705dbfd4c76b0815511ea7b4b034e095..7be1b12a75bf9f201644aef471c5edb99979c0c5 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
>> @@ -181,34 +181,6 @@ test_ashift_right_int_si (Int32x1 b, Int32x1 c)
>>   /* { dg-final { scan-assembler "asr\tw\[0-9\]+,\ w\[0-9\]+,\ 4" } } */
>>   /* { dg-final { scan-assembler "asr\tw\[0-9\]+,\ w\[0-9\]+,\ w\[0-9\]+" } } */
>>   
>> -Int64x1
>> -test_corners_sisd_di (Int64x1 b)
>> -{
>> -  force_simd_di (b);
>> -  b = b >> 63;
>> -  force_simd_di (b);
>> -  b = b >> 0;
>> -  b += b >> 65; /* { dg-warning "right shift count >= width of type" } */
>> -
>> -  return b;
>> -}
>> -/* { dg-final { scan-assembler "sshr\td\[0-9\]+,\ d\[0-9\]+,\ 63" } } */
>> -
>> -Int32x1
>> -test_corners_sisd_si (Int32x1 b)
>> -{
>> -  force_simd_si (b);
>> -  b = b >> 31;
>> -  force_simd_si (b);
>> -  b = b >> 0;
>> -  b += b >> 33; /* { dg-warning "right shift count >= width of type" } */
>> -
>> -  return b;
>> -}
>> -/* { dg-final { scan-assembler "sshr\tv\[0-9\]+\.2s,\ v\[0-9\]+\.2s,\ 31" } } */
>> -
>> -
>> -
>>   #define CHECK(var,val) \
>>   do                     \
>>     {                    \
>> @@ -236,8 +208,6 @@ main ()
>>     CHECK (x, 0xffffffff21524110ull);
>>     x = test_ashift_right_sisd_di (x, 8);
>>     CHECK (x, 0xffffffffffff2152ull);
>> -  x = test_corners_sisd_di (x);
>> -  CHECK (x, 0xfffffffffffffffeull);
>>   
>>     y = test_lshift_left_sisd_si (y, 4);
>>     CHECK (y, 0xadbeef00);
>> @@ -252,8 +222,6 @@ main ()
>>     CHECK (y, 0xffff5241);
>>     y = test_ashift_right_sisd_si (y, 4);
>>     CHECK (y, 0xffffff52);
>> -  y = test_corners_sisd_si (y);
>> -  CHECK (y, 0xfffffffe);
>>   
>>     return 0;
>>   }
>
> 	Jakub

Comments

Jakub Jelinek March 4, 2016, 11:02 a.m. UTC | #1 
On Fri, Mar 04, 2016 at 10:41:04AM +0000, Kyrill Tkachov wrote:
> Can do. I've moved the dodgy functions into their own separate compile test.
> The test passes.
> Ok?

LGTM.

	Jakub
James Greenhalgh March 4, 2016, 11:06 a.m. UTC | #2 
On Fri, Mar 04, 2016 at 12:02:59PM +0100, Jakub Jelinek wrote:
> On Fri, Mar 04, 2016 at 10:41:04AM +0000, Kyrill Tkachov wrote:
> > Can do. I've moved the dodgy functions into their own separate compile test.
> > The test passes.
> > Ok?
> 
> LGTM.

OK from me too.

James
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c b/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
index 8465c896705dbfd4c76b0815511ea7b4b034e095..7be1b12a75bf9f201644aef471c5edb99979c0c5 100644
--- a/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/scalar_shift_1.c
@@ -181,34 +181,6 @@  test_ashift_right_int_si (Int32x1 b, Int32x1 c)
 /* { dg-final { scan-assembler "asr\tw\[0-9\]+,\ w\[0-9\]+,\ 4" } } */
 /* { dg-final { scan-assembler "asr\tw\[0-9\]+,\ w\[0-9\]+,\ w\[0-9\]+" } } */
 
-Int64x1
-test_corners_sisd_di (Int64x1 b)
-{
-  force_simd_di (b);
-  b = b >> 63;
-  force_simd_di (b);
-  b = b >> 0;
-  b += b >> 65; /* { dg-warning "right shift count >= width of type" } */
-
-  return b;
-}
-/* { dg-final { scan-assembler "sshr\td\[0-9\]+,\ d\[0-9\]+,\ 63" } } */
-
-Int32x1
-test_corners_sisd_si (Int32x1 b)
-{
-  force_simd_si (b);
-  b = b >> 31;
-  force_simd_si (b);
-  b = b >> 0;
-  b += b >> 33; /* { dg-warning "right shift count >= width of type" } */
-
-  return b;
-}
-/* { dg-final { scan-assembler "sshr\tv\[0-9\]+\.2s,\ v\[0-9\]+\.2s,\ 31" } } */
-
-
-
 #define CHECK(var,val) \
 do                     \
   {                    \
@@ -236,8 +208,6 @@  main ()
   CHECK (x, 0xffffffff21524110ull);
   x = test_ashift_right_sisd_di (x, 8);
   CHECK (x, 0xffffffffffff2152ull);
-  x = test_corners_sisd_di (x);
-  CHECK (x, 0xfffffffffffffffeull);
 
   y = test_lshift_left_sisd_si (y, 4);
   CHECK (y, 0xadbeef00);
@@ -252,8 +222,6 @@  main ()
   CHECK (y, 0xffff5241);
   y = test_ashift_right_sisd_si (y, 4);
   CHECK (y, 0xffffff52);
-  y = test_corners_sisd_si (y);
-  CHECK (y, 0xfffffffe);
 
   return 0;
 }
diff --git a/gcc/testsuite/gcc.target/aarch64/shift_wide_invalid_1.c b/gcc/testsuite/gcc.target/aarch64/shift_wide_invalid_1.c
new file mode 100644
index 0000000000000000000000000000000000000000..6b71cb580924be35d2b9d2283b6262f5d2ce9729
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/shift_wide_invalid_1.c
@@ -0,0 +1,36 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+/* These contain undefined behavior but may trigger edge cases in the
+   vector shift patterns.  We don't check for their generation, we only
+   care about not ICEing.  */
+
+typedef long long int Int64x1;
+typedef int Int32x1;
+
+#define force_simd_di(v) asm volatile ("mov %d0, %1.d[0]" : "=w"(v) : "w"(v) :)
+#define force_simd_si(v) asm volatile ("mov %s0, %1.s[0]" : "=w"(v) : "w"(v) :)
+
+Int64x1
+foo_di (Int64x1 b)
+{
+  force_simd_di (b);
+  b = b >> 63;
+  force_simd_di (b);
+  b = b >> 0;
+  b += b >> 65; /* { dg-warning "right shift count >= width of type" } */
+
+  return b;
+}
+
+Int32x1
+foo_si (Int32x1 b)
+{
+  force_simd_si (b);
+  b = b >> 31;
+  force_simd_si (b);
+  b = b >> 0;
+  b += b >> 33; /* { dg-warning "right shift count >= width of type" } */
+
+  return b;
+}