| Message ID | 29224af6-f1d5-435e-97dd-ddc463fe534c@BAMAIL02.ba.imgtec.org |
|---|---|
| State | New |
| Headers | show |
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
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
> -----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 >
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
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
> -----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 >
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);