[PATCH] RE: Testsuite / Cilk Plus: Include library path in compile flags in gcc.dg/cilk-plus/cilk-plus.exp

["Iyer, Balaji V" <balaji dot v dot iyer at intel dot com>]

Hello Jakub et al.,
      I went through all your emails and the patch above will fix the following issues:

1. Bug in the fib<>.c test case where it should recursively call fib_serial instead of fib() in the serial case.
2. Remove duplicate or unwanted test case flags (e.g. there were cases were -g and "-O0 -g" were both tested, so I took out one of them)
3. Called builtin_abort() instead of returning a non-zero return value for main in the test cases.
4. Reduce the iteration of fib -- the main reason why I had a larger iteration is that we wanted to force a steal, but I already have a test case that will do that. So, if there is any issue in that logic, that code should fail.

Also, with all these changes, make check-gcc RUNTESTFLAGS='--target_board=unix/-m32 cilk-plus.exp' and make check-gcc RUNTESTFLAGS='--target_board=unix cilk-plus.exp' runs are significantly faster.

Here are the ChangeLog entries: 

+2013-11-05  Balaji V. Iyer  <balaji.v.iyer@intel.com>
+
+       * c-c++-common/cilk-plus/CK/fib.c: Reduced the iteration from
+       40 to 30.  Replaced iteration variable with a #define.  Instead of
+       returning non-zero value for error, called __builtin_abort ().  Fixed
+       a bug of calling fib_serial in serial case instead of fib.
+       * c-c++-common/cilk-plus/CK/fib_init_expr_xy.c: Likewise.
+       * c-c++-common/cilk-plus/CK/fib_no_return.c: Likewise.
+       * c-c++-common/cilk-plus/CK/fib_no_sync.c: Likewise.
+       * gcc.dg/cilk-plus/cilk-plus.exp: Removed duplicate/un-necessary
+       compiler flag testing.
+

Is this Ok to check in?

Thanks,

Balaji V. Iyer.

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Jakub Jelinek
> Sent: Tuesday, November 5, 2013 6:33 AM
> To: Iyer, Balaji V
> Cc: Iain Sandoe; Joseph S. Myers; Tobias Burnus; gcc patches
> Subject: Re: Testsuite / Cilk Plus: Include library path in compile flags in
> gcc.dg/cilk-plus/cilk-plus.exp
> 
> On Tue, Nov 05, 2013 at 12:21:04PM +0100, Jakub Jelinek wrote:
> > Tests that many seconds or more on fast machines, especially if you
> > run them
> > 25 times, are simply not appropriate for gcc testsuite, at least not
> > by default.  We have run_expensive_tests (from
> GCC_RUN_EXPENSIVE_TESTS
> > in environment), which can be used say for:
> > /* { dg-additional-options "-DEXPENSIVE" { target run_expensive_tests
> > } } */ or // { dg-additional-options "-DASAN_AVOID_EXPENSIVE_TESTS=1"
> > { target { ! run_expensive_tests } } } or
> > /* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O2" } } */
> > (just grep for it), but still it shouldn't be prohibitively slow.
> > Remember what is slow on fast machines might turn into days on really
> > slow machines.  Say, if all you are looking for is look for library
> > synchronization issues, I guess all optimization levels still result
> > in similar pattern of library calls, so even for run_expensive_tests
> > you could use higher iteration count for a single optimization level
> > (say -O2) and for all others just use smaller iteration count.
> 
> Oh, another thing, runtime tests should abort () or __builtin_abort () on
> failure, rather then just exit with non-zero status.
> 
> Also, I wonder about the fib_* tests, fib_serial calls fib rather than fib_serial,
> so effectively the only difference between calling fib_serial and fib is just the
> outermost iteration.  Also, are you really sure you have to call fib with all
> values from 0 up to 40?  Isn't it enough to just call fib (40) once and compare
> that with precomputed fib (40) number?
> Because, when a single test takes about 2 minutes to run, it is really too
> much, and very much unnecessarily so.
> 
> 	Jakub

Attachment: diff.txt
Description: diff.txt