cilking away

Kenny reported that the library name is mixed into the test case name.  This is wrong.  This fixes it.  I also trimmed -O0, as redundant and added support for C++ runtime tests, once you guys want to add one.

The CK tests should either be made to work in C++ land, or moved to gcc.dg/cilk-plus.
The -fcilkplus in *.exp is redundant with that option in *.{c,cc}.  Please remove one instance of them.


        * lib/gcc.exp (gcc_target_compile): Add support for random runtime
        * lib/g++.exp (g++_target_compile): Likewise.
        * gcc.dg/cilk-plus/cilk-plus.exp: Improve support for runtime
        libraries.  Remove debugging.
        * g++.dg/cilk-plus/cilk-plus.exp: Add support to find runtime
        libraries.  Remove -O0, redundant with default.

> -----Original Message-----
> From: Mike Stump [mailto:mikestump@comcast.net]
> Sent: Friday, November 15, 2013 7:41 PM
> To: Iyer, Balaji V
> Cc: gcc-patches@gcc.gnu.org Patches
> Subject: cilking away
> 
> Kenny reported that the library name is mixed into the test case name.  This
> is wrong.  This fixes it.  I also trimmed -O0, as redundant and added support
> for C++ runtime tests, once you guys want to add one.

This is already done in my patch for _Cilk-spawn and _Cilk_sync  support for C++. The patch was submitted ~3-4 weeks ago. It is currently under review (http://gcc.gnu.org/ml/gcc-patches/2013-10/msg01807.html).  If someone could take a look and see if it is Ok for trunk, it would be nice :-).

> 
> The CK tests should either be made to work in C++ land, or moved to
> gcc.dg/cilk-plus.

Yes, they should work in C++ also. Please see my patch above.

> The -fcilkplus in *.exp is redundant with that option in *.{c,cc}.  Please
> remove one instance of them.
> 

It is sort of like a safety measure since there are some error tests which may have dg-options omitted. I don't see a big deal with having it in 2 places, but if it is really bad, I can put it in the tests and remove from the options.

> 
>         * lib/gcc.exp (gcc_target_compile): Add support for random runtime
>         * lib/g++.exp (g++_target_compile): Likewise.
>         * gcc.dg/cilk-plus/cilk-plus.exp: Improve support for runtime
>         libraries.  Remove debugging.
>         * g++.dg/cilk-plus/cilk-plus.exp: Add support to find runtime
>         libraries.  Remove -O0, redundant with default.
> 
> Index: g++.dg/cilk-plus/cilk-plus.exp
> ==========================================================
> =========
> --- g++.dg/cilk-plus/cilk-plus.exp	(revision 204881)
> +++ g++.dg/cilk-plus/cilk-plus.exp	(working copy)
> @@ -29,14 +29,19 @@ g++-dg-runtest [lsort [glob -nocomplain  g++-dg-
> runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C]] ""
>  dg-finish
> 
> +set library_var [get_multilibs]
> +# Pointing the ld_library_path to the Cilk Runtime library binaries.
> +set ld_library_path "${library_var}/libcilkrts/.libs"
> +
> +global TEST_EXTRA_LIBS
> +set TEST_EXTRA_LIBS "-L${library_var}/libcilkrts/.libs"
> +

This part was accidentally omitted from my g++.dg/cilk-plus/cilk-plus.exp change in the above patch. Thanks for catching it :-).

>  dg-init
>  dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/AN/*.c]] " -fcilkplus" " "
> -dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/AN/*.c]] " -O0 -fcilkplus" " "
>  dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/AN/*.c]] " -O1 -fcilkplus" " "
>  dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/AN/*.c]] " -O2 -ftree-vectorize -fcilkplus" " "
>  dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/AN/*.c]] " -O3 -fcilkplus" " "
>  dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/AN/*.c]] " -g -fcilkplus" " "
> -dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/AN/*.c]] " -g -O0 -fcilkplus" " "
>  dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/AN/*.c]] " -g -O1 -fcilkplus" " "
>  dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/AN/*.c]] " -g -O2 -ftree-vectorize -fcilkplus" " "
>  dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/AN/*.c]] " -g -O3 -fcilkplus" " "
> @@ -56,3 +61,5 @@ dg-runtest [lsort [glob -nocomplain $src  dg-runtest
> [lsort [glob -nocomplain $srcdir/g++.dg/cilk-plus/AN/*.cc]] " -g -O3 -fcilkplus"
> " "
>  dg-runtest [lsort [glob -nocomplain $srcdir/g++.dg/cilk-plus/AN/*.cc]] " -O3 -
> ftree-vectorize -fcilkplus -g" " "
>  dg-finish
> +
> +unset TEST_EXTRA_LIBS
> Index: gcc.dg/cilk-plus/cilk-plus.exp
> ==========================================================
> =========
> --- gcc.dg/cilk-plus/cilk-plus.exp	(revision 204881)
> +++ gcc.dg/cilk-plus/cilk-plus.exp	(working copy)
> @@ -24,13 +24,12 @@ if { ![check_effective_target_cilkplus]
>      return;
>  }
> 
> -verbose "$tool $libdir" 1
>  set library_var [get_multilibs]
>  # Pointing the ld_library_path to the Cilk Runtime library binaries.
>  set ld_library_path "${library_var}/libcilkrts/.libs"
> 
> -set ALWAYS_CFLAGS ""
> -lappend ALWAYS_CFLAGS "-L${library_var}/libcilkrts/.libs"
> +global TEST_EXTRA_LIBS
> +set TEST_EXTRA_LIBS "-L${library_var}/libcilkrts/.libs"
> 
>  dg-init
> 
> @@ -51,14 +50,15 @@ dg-runtest [lsort [glob -nocomplain $src  dg-runtest
> [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/AN/*.c]] " -fcilkplus
> -O3 -std=c99" " "
>  dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/AN/*.c]] " -fcilkplus -g -O0 -std=c99" " "
> 
> -dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -g -fcilkplus $ALWAYS_CFLAGS " " "
> -dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O1 -fcilkplus $ALWAYS_CFLAGS" " "
> -dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O2 -std=c99 -fcilkplus $ALWAYS_CFLAGS" " "
> -dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O2 -ftree-vectorize -fcilkplus $ALWAYS_CFLAGS" " "
> -dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]]
> " -O3 -g -fcilkplus $ALWAYS_CFLAGS" " "
> +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/CK/*.c]] " -g -fcilkplus" " "
> +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/CK/*.c]] " -O1 -fcilkplus" " "
> +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/CK/*.c]] " -O2 -std=c99 -fcilkplus" " "
> +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/CK/*.c]] " -O2 -ftree-vectorize -fcilkplus" " "
> +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/CK/*.c]] " -O3 -g -fcilkplus" " "
>  if { [check_effective_target_lto] } {
> -    dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/CK/*.c]] " -O3 -flto -g -fcilkplus $ALWAYS_CFLAGS" " "
> +    dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/CK/*.c]] " -O3 -flto -g -fcilkplus" " "
>  }
> 
> -
>  dg-finish
> +
> +unset TEST_EXTRA_LIBS
> Index: lib/g++.exp
> ==========================================================
> =========
> --- lib/g++.exp	(revision 204881)
> +++ lib/g++.exp	(working copy)
> @@ -294,6 +294,11 @@ proc g++_target_compile { source dest ty
>  	lappend options "ldflags=${wrap_flags}"
>      }
> 
> +    global TEST_EXTRA_LIBS
> +    if [info exists TEST_EXTRA_LIBS] {
> +	lappend options "ldflags=$TEST_EXTRA_LIBS"
> +    }
> +
>      lappend options "additional_flags=[libio_include_flags]"
>      lappend options "compiler=$GXX_UNDER_TEST"
>      lappend options "timeout=[timeout_value]"
> Index: lib/gcc.exp
> ==========================================================
> =========
> --- lib/gcc.exp	(revision 204881)
> +++ lib/gcc.exp	(working copy)
> @@ -134,6 +134,11 @@ proc gcc_target_compile { source dest ty
>  	lappend options "ldflags=$wrap_flags"
>      }
> 
> +    global TEST_EXTRA_LIBS
> +    if [info exists TEST_EXTRA_LIBS] {
> +	lappend options "ldflags=$TEST_EXTRA_LIBS"
> +    }
> +
>      if [target_info exists gcc,stack_size] {
>  	lappend options "additional_flags=-DSTACK_SIZE=[target_info
> gcc,stack_size]"
>      }

All this work looks very well organized and seem correct. But, I have not had a chance to test them. I will look into this Monday/Tuesday.

Thanks for helping me with this!

Sincerely,

Balaji V. Iyer.

On Nov 15, 2013, at 8:23 PM, "Iyer, Balaji V" <balaji.v.iyer@intel.com> wrote:
> This is already done in my patch for _Cilk-spawn and _Cilk_sync  support for C++. The patch was submitted ~3-4 weeks ago.

Ping once a day until reviewed.  :-)  You should form a new patch and post it and be sure to cc jason on that email.  It is possible he just hasn't seen it yet.  Be sure to post and ping the work.

> It is currently under review (http://gcc.gnu.org/ml/gcc-patches/2013-10/msg01807.html).

I saw no evidence it is currently under review.  I think the state is, jason hasn't been cced on the C++ patch and don't know of its existence.

>> The -fcilkplus in *.exp is redundant with that option in *.{c,cc}.  Please
>> remove one instance of them.
> 
> It is sort of like a safety measure

We don't want a sort of safety measure.

> since there are some error tests which may have dg-options omitted.

Once you decide if those test are wrong for not having dg-options, or not, the rest will just fall out.

> if it is really bad, I can put it in the tests and remove from the options.

Yes, it is really bad, thanks.