Patchwork [testsuite,cilk] Fix cilk tests for simulators

login
register
mail settings
Submitter Steve Ellcey
Date June 3, 2013, 5:49 p.m.
Message ID <29224af6-f1d5-435e-97dd-ddc463fe534c@BAMAIL02.ba.imgtec.org>
Download mbox | patch
Permalink /patch/248370/
State New
Headers show

Comments

Steve Ellcey - June 3, 2013, 5:49 p.m.
A number of the new cilk tests in gcc/testsuite/c-c++-common/cilk-plus/AN
fail for me when run via the gnu simulator on mips.  The problem is that
the gnu simulator does not set up argc and argv.  After asking in the gdb
mailing list I believe this is an issue for multiple simulators on multiple
platforms.  Looking through the GCC testsuite I did not see any other tests
that looked at argc/argv so I would like to change these tests to not use
argc/argv either.  In some tests I added a define (IN_GCC_TESTSUITE) that
I set to 1 and then don't check argc if this is set, in others I just used
the constant value 1 instead of using argc and in one (where argc was being
changed) I replaced the use of argc with a local variable.

Tested on mips-mti-elf with the GNU simulator.

OK to checkin?


2013-06-03  Steve Ellcey  <sellcey@mips.com>

	* c-c++-common/cilk-plus/AN/array_test2.c: Do not check argc value.
	* c-c++-common/cilk-plus/AN/array_test_ND.c: Ditto.
	* c-c++-common/cilk-plus/AN/comma_exp.c: Ditto.
	* c-c++-common/cilk-plus/AN/if_test.c: Ditto.
	* c-c++-common/cilk-plus/AN/conditional.c:  Replace argc with const 1.
	* c-c++-common/cilk-plus/AN/sec_reduce_return.c: Ditto.
	* c-c++-common/cilk-plus/AN/test_builtin_return.c: Ditto.
	* c-c++-common/cilk-plus/AN/exec-once2.c: Replace argc with local var.
Jeff Law - June 3, 2013, 6:27 p.m.
On 06/03/2013 11:49 AM, Steve Ellcey wrote:
>
> A number of the new cilk tests in gcc/testsuite/c-c++-common/cilk-plus/AN
> fail for me when run via the gnu simulator on mips.  The problem is that
> the gnu simulator does not set up argc and argv.  After asking in the gdb
> mailing list I believe this is an issue for multiple simulators on multiple
> platforms.  Looking through the GCC testsuite I did not see any other tests
> that looked at argc/argv so I would like to change these tests to not use
> argc/argv either.  In some tests I added a define (IN_GCC_TESTSUITE) that
> I set to 1 and then don't check argc if this is set, in others I just used
> the constant value 1 instead of using argc and in one (where argc was being
> changed) I replaced the use of argc with a local variable.
>
> Tested on mips-mti-elf with the GNU simulator.
Yea, this should have been caught earlier.  argc/argv aren't set 
properly in many simulator environments.

>>   {
>     int x = 0;
> -  if (argc == 1)
> +  if (argc == 1 || IN_GCC_TESTSUITE)
So why not just eliminate the conditional completely and simplify the 
test appropriately?  The only reason I can think of to keep it as-is 
would be if the test were from another suite and we wanted to minimize 
the amount of divergence from that other testsuite.

Balaji, is there a good reason to keep the argc/argv usage in these tests?

jeff
Jakub Jelinek - June 3, 2013, 6:30 p.m.
On Mon, Jun 03, 2013 at 12:27:15PM -0600, Jeff Law wrote:
> On 06/03/2013 11:49 AM, Steve Ellcey wrote:
> >
> >A number of the new cilk tests in gcc/testsuite/c-c++-common/cilk-plus/AN
> >fail for me when run via the gnu simulator on mips.  The problem is that
> >the gnu simulator does not set up argc and argv.  After asking in the gdb
> >mailing list I believe this is an issue for multiple simulators on multiple
> >platforms.  Looking through the GCC testsuite I did not see any other tests
> >that looked at argc/argv so I would like to change these tests to not use
> >argc/argv either.  In some tests I added a define (IN_GCC_TESTSUITE) that
> >I set to 1 and then don't check argc if this is set, in others I just used
> >the constant value 1 instead of using argc and in one (where argc was being
> >changed) I replaced the use of argc with a local variable.
> >
> >Tested on mips-mti-elf with the GNU simulator.
> Yea, this should have been caught earlier.  argc/argv aren't set
> properly in many simulator environments.
> 
> >>  {
> >    int x = 0;
> >-  if (argc == 1)
> >+  if (argc == 1 || IN_GCC_TESTSUITE)
> So why not just eliminate the conditional completely and simplify
> the test appropriately?  The only reason I can think of to keep it
> as-is would be if the test were from another suite and we wanted to
> minimize the amount of divergence from that other testsuite.

Another reason (at least in a couple of tests I think that is the case)
is that argc is used as a variable without known compile time value, but
with expected known runtime value.  That can be replaced with something
like:
  int var = 1;
  __asm volatile ("" : "+r" (var));
or so.

	Jakub
Iyer, Balaji V - June 3, 2013, 7:27 p.m.
> -----Original Message-----
> From: Jeff Law [mailto:law@redhat.com]
> Sent: Monday, June 03, 2013 2:27 PM
> To: Steve Ellcey
> Cc: Iyer, Balaji V; gcc-patches@gcc.gnu.org
> Subject: Re: [patch, testsuite, cilk] Fix cilk tests for simulators
> 
> On 06/03/2013 11:49 AM, Steve Ellcey wrote:
> >
> > A number of the new cilk tests in
> > gcc/testsuite/c-c++-common/cilk-plus/AN
> > fail for me when run via the gnu simulator on mips.  The problem is
> > that the gnu simulator does not set up argc and argv.  After asking in
> > the gdb mailing list I believe this is an issue for multiple
> > simulators on multiple platforms.  Looking through the GCC testsuite I
> > did not see any other tests that looked at argc/argv so I would like
> > to change these tests to not use argc/argv either.  In some tests I
> > added a define (IN_GCC_TESTSUITE) that I set to 1 and then don't check
> > argc if this is set, in others I just used the constant value 1
> > instead of using argc and in one (where argc was being
> > changed) I replaced the use of argc with a local variable.
> >
> > Tested on mips-mti-elf with the GNU simulator.
> Yea, this should have been caught earlier.  argc/argv aren't set properly in many
> simulator environments.
> 
> >>   {
> >     int x = 0;
> > -  if (argc == 1)
> > +  if (argc == 1 || IN_GCC_TESTSUITE)
> So why not just eliminate the conditional completely and simplify the test
> appropriately?  The only reason I can think of to keep it as-is would be if the test
> were from another suite and we wanted to minimize the amount of divergence
> from that other testsuite.
> 
> Balaji, is there a good reason to keep the argc/argv usage in these tests?

I am OK with Steve's changes in most cases. In a few cases, I am using it as a parameter to pass into tests. On a top-level, the main reason why I used argc, and argv is that, I want to make sure the compiler will never do things like constant propagation, and it will pass it as a variable. 

Thanks,

Balaji V. Iyer.

> 
> jeff
>
Jeff Law - June 3, 2013, 7:47 p.m.
On 06/03/2013 01:27 PM, Iyer, Balaji V wrote:
>
> I am OK with Steve's changes in most cases. In a few cases, I am
> using it as a parameter to pass into tests. On a top-level, the main
> reason why I used argc, and argv is that, I want to make sure the
> compiler will never do things like constant propagation, and it will
> pass it as a variable.
So use Jakub's trick, or define non-inlinable functions which return 
suitable tables.

We simply can't use argc/argv in the manner in which those tests do and 
I'd rather clean up the test to avoid argc/argv than support two paths 
through the test, one with argc/argv, one without.

jeff
Steve Ellcey - June 3, 2013, 10:31 p.m.
On Mon, 2013-06-03 at 13:47 -0600, Jeff Law wrote:
> On 06/03/2013 01:27 PM, Iyer, Balaji V wrote:
> >
> > I am OK with Steve's changes in most cases. In a few cases, I am
> > using it as a parameter to pass into tests. On a top-level, the main
> > reason why I used argc, and argv is that, I want to make sure the
> > compiler will never do things like constant propagation, and it will
> > pass it as a variable.
> So use Jakub's trick, or define non-inlinable functions which return 
> suitable tables.
> 
> We simply can't use argc/argv in the manner in which those tests do and 
> I'd rather clean up the test to avoid argc/argv than support two paths 
> through the test, one with argc/argv, one without.
> 
> jeff

I'll leave fixing the tests to Balaji then instead of doing it myself
since that way he can be sure that they are testing what he wants to
test.

Steve Ellcey
sellcey@mips.com
Iyer, Balaji V - June 3, 2013, 10:32 p.m.
> -----Original Message-----
> From: Steve Ellcey [mailto:sellcey@mips.com]
> Sent: Monday, June 03, 2013 6:31 PM
> To: Jeff Law
> Cc: Iyer, Balaji V; gcc-patches@gcc.gnu.org
> Subject: Re: [patch, testsuite, cilk] Fix cilk tests for simulators
> 
> On Mon, 2013-06-03 at 13:47 -0600, Jeff Law wrote:
> > On 06/03/2013 01:27 PM, Iyer, Balaji V wrote:
> > >
> > > I am OK with Steve's changes in most cases. In a few cases, I am
> > > using it as a parameter to pass into tests. On a top-level, the main
> > > reason why I used argc, and argv is that, I want to make sure the
> > > compiler will never do things like constant propagation, and it will
> > > pass it as a variable.
> > So use Jakub's trick, or define non-inlinable functions which return
> > suitable tables.
> >
> > We simply can't use argc/argv in the manner in which those tests do
> > and I'd rather clean up the test to avoid argc/argv than support two
> > paths through the test, one with argc/argv, one without.
> >
> > jeff
> 
> I'll leave fixing the tests to Balaji then instead of doing it myself since that way
> he can be sure that they are testing what he wants to test.

OK, I will look into this. 

-Balaji V. Iyer.

> 
> Steve Ellcey
> sellcey@mips.com
>

Patch

diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/array_test2.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/array_test2.c
index bd7a4ad..178bba6 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/array_test2.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/array_test2.c
@@ -1,12 +1,12 @@ 
 /* { dg-do run } */
-/* { dg-options "-fcilkplus" } */
+/* { dg-options "-fcilkplus -DIN_GCC_TESTSUITE=1" } */
 
 #include <stdlib.h>
 int main2(int argc, char **argv);
 int main(int argc, char **argv)
 {
   int x = 0;
-  if (argc == 1)
+  if (argc == 1 || IN_GCC_TESTSUITE)
     {
       const char *array[] = {"a.out", "5"};	     
       x = main2 (2, (char **)array);
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/array_test_ND.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/array_test_ND.c
index 1431c22..441342b 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/array_test_ND.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/array_test_ND.c
@@ -1,12 +1,12 @@ 
 /* { dg-do run } */
-/* { dg-options "-fcilkplus" } */
+/* { dg-options "-fcilkplus -DIN_GCC_TESTSUITE=1" } */
 
 #include <stdlib.h>
 int main2(int argc, char **argv);
 int main(int argc, char **argv)
 {
   int x = 0;
-  if (argc == 1)
+  if (argc == 1 || IN_GCC_TESTSUITE)
     {
       const char *array[] = {"a.out", "10", "15"};	     
       x = main2 (3, (char **)array);
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/comma_exp.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/comma_exp.c
index bcb3e1b..43138bb 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/comma_exp.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/comma_exp.c
@@ -1,5 +1,5 @@ 
 /* { dg-do run } */
-/* { dg-options "-fcilkplus" } */
+/* { dg-options "-fcilkplus -DIN_GCC_TESTSUITE" } */
 
 #include <stdlib.h>
 
@@ -7,7 +7,7 @@  int main2 (int argc, char **argv);
 int main(int argc, char **argv)
 {
   int x = 0;
-  if (argc == 1)
+  if (argc == 1 || IN_GCC_TESTSUITE)
     {
       const char *array[] = {"a.out", "5"};	     
       x = main2 (2, (char **)array);
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/conditional.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/conditional.c
index 0be99b2..4345a26 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/conditional.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/conditional.c
@@ -31,7 +31,7 @@  int main(int argc, char **argv)
     array[ii] = 3; 
   }
   array3 = (short *) malloc (sizeof (short) * 1000);
-  array3[0:1000:argc] = cond[:] ? array[0:(argc * 1000)] : array2[argc-1:1000];
+  array3[0:1000:1] = cond[:] ? array[0:(1 * 1000)] : array2[0:1000];
   
   for (ii = 0; ii < 1000; ii++) {
     if ((cond[ii] == 0 && array3[ii] != 5) 
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/exec-once2.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/exec-once2.c
index 8d208b9..6cbfb66 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/exec-once2.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/exec-once2.c
@@ -34,23 +34,23 @@  int func4(int x)
 }
 
 
-/* This program makes an assumption that argc == 1.  */
 int main (int argc, char **argv)
 {
 
   int array[2500];
+  int i = 1;
 
   /* This should set array[0->999] to 5.  */
-  array[argc-1:func2(++argc):1] = 5;
+  array[i-1:func2(++i):1] = 5;
   array[1000:500:1] = 10; /* set all variables in array[1000-->1499] to 10.  */
   array[1500:500:1] = 15; /* set all variables in array[1500-->1999] to 15.  */
   array[2000:500:1] = 20; /* set all variables in array[2000-->2499] to 20.  */
   array[2000:500:1] = 25; /* set all variables in array[2500-->2999] to 25.  */
   array[2000:500:1] = 30; /* set all variables in array[3000-->3499] to 30.  */
   
-  argc = func3 (argc); /* This will set argc back to 1.  */
+  i = func3 (i); /* This will set i back to 1.  */
 #if HAVE_IO
-  printf("argc = %d\n", argc);
+  printf("i = %d\n", i);
 #endif
   /* If the parameters inside the function get evaluated only once, then this
      if statement must work fine, i.e. the triplet values will be 0, 1000, 1.
@@ -58,7 +58,7 @@  int main (int argc, char **argv)
      Otherwise, the program should crash or give some uneasy value.  */
 
   /* If done correctly, it should boil down to: array[0:1000:1].  */
-  if (array[func3(argc):func2(++argc)] != 5) {
+  if (array[func3(i):func2(++i)] != 5) {
 #ifdef HAVE_IO
     printf ("Should not be there(1).\n");
 #endif
@@ -66,7 +66,7 @@  int main (int argc, char **argv)
   }
   
   /* If done correctly, it should boil down to: array[999:500:-1].  */
-  if (func4(array[func2(argc)-1:func2(argc--):func1(argc)]) != 5) {
+  if (func4(array[func2(i)-1:func2(i--):func1(i)]) != 5) {
 #ifdef HAVE_IO
     printf ("Should not be there(2).\n");
 #endif
@@ -74,7 +74,7 @@  int main (int argc, char **argv)
   }
 
   /* If done correctly, it should boil down to: array[1000:500:1].  */
-  if (func4 (func4(array[func2(argc++):500: func1(argc--)])) != 5) {
+  if (func4 (func4(array[func2(i++):500: func1(i--)])) != 5) {
 #ifdef HAVE_IO
     printf ("Should not be there(3).\n");
 #endif
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c
index 53ceeec..200e8b3 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/if_test.c
@@ -1,5 +1,5 @@ 
 /* { dg-do run } */
-/* { dg-options "-fcilkplus" } */
+/* { dg-options "-fcilkplus -DIN_GCC_TESTSUITE=1" } */
 
 #if HAVE_IO
 #include <stdio.h>
@@ -9,7 +9,7 @@  int main2 (int argc, char **argv);
 int main(int argc, char **argv)
 {
   int x = 0;
-  if (argc == 1)
+  if (argc == 1 || IN_GCC_TESTSUITE)
     {
       const char *array[] = {"a.out", "10", "15"};	     
       x = main2 (3, (char **) array);
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/sec_reduce_return.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/sec_reduce_return.c
index a72cfaf..107bbe4 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/sec_reduce_return.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/sec_reduce_return.c
@@ -15,7 +15,7 @@  int main (int argc, char **argv)
 {
   int array[10000];
 
-  array[:] = argc; /* All elements should be one.  */
+  array[:] = 1; /* All elements should be one.  */
 
   if (add_all (array, 10000) != 10000)
     return 1;
diff --git a/gcc/testsuite/c-c++-common/cilk-plus/AN/test_builtin_return.c b/gcc/testsuite/c-c++-common/cilk-plus/AN/test_builtin_return.c
index 0df324a..78e2dc2 100644
--- a/gcc/testsuite/c-c++-common/cilk-plus/AN/test_builtin_return.c
+++ b/gcc/testsuite/c-c++-common/cilk-plus/AN/test_builtin_return.c
@@ -35,8 +35,8 @@  int main(int argc, char **argv)
   int ii = 0;
   for (ii = 0; ii < NUMBER; ii++)
     {
-      array[ii] = argc; /* This should calculate to 1.  */
-      array2[ii]  = argc * argc + argc;  /* This should calculate to 2.  */
+      array[ii] = 1;
+      array2[ii]  = 2;
     }
 
   return_value = func1 (array, array2);