[PATCH][testsuite] Fix TORTURE_OPTIONS overriding

[PATCH][testsuite] Fix TORTURE_OPTIONS overriding

[Richard Biener]

Currently when doing

make check-gcc RUNTESTFLAGS="TORTURE_OPTIONS=\\\"{ -O3 } { -O2 }\\\" 
dg-torture.exp"

you get -O3 and -O2 but also the two LTO torture option combinations.
That's undesired (those are the most expensive anyway).  The following
patch avoids this by setting LTO_TORTURE_OPTIONS only when
TORTURE_OPTIONS isn't specified.

Tested with and without TORTURE_OPTIONS for C and fortran tortures.

Seems the instruction in c-torture.exp how to override TORTURE_OPTIONS
is off, RUNTESTFLAGS="TORTURE_OPTIONS=\\\"{ { -O3 } { -O2 } }\\\"
certainly doesn't do what it should.

Ok for trunk?

Thanks,
Richard.

2015-06-18  Richard Biener  <rguenther@suse.de>

	* lib/c-torture.exp: Set LTO_TORTURE_OPTIONS conditional on
	not existing TORTURE_OPTIONS only.
	* lib/gcc-dg.exp: Likewise.

Index: gcc/testsuite/lib/c-torture.exp
===================================================================
--- gcc/testsuite/lib/c-torture.exp	(revision 224545)
+++ gcc/testsuite/lib/c-torture.exp	(working copy)
@@ -22,8 +22,9 @@ load_lib target-libpath.exp
 load_lib target-utils.exp
 
 # The default option list can be overridden by
-# TORTURE_OPTIONS="{ { list1 } ... { listN } }"
+# TORTURE_OPTIONS="{ list1 } ... { listN }"
 
+set LTO_TORTURE_OPTIONS ""
 if [info exists TORTURE_OPTIONS] {
     set C_TORTURE_OPTIONS $TORTURE_OPTIONS
 } else {
@@ -44,6 +45,22 @@ if [info exists TORTURE_OPTIONS] {
 	{ -O3 -g } \
 	{ -Os } \
 	{ -Og -g } ]
+
+    if [check_effective_target_lto] {
+        # When having plugin test both slim and fat LTO and plugin/nonplugin
+        # path.
+        if [check_linker_plugin_available] {
+          set LTO_TORTURE_OPTIONS [list \
+    	      { -O2 -flto -fno-use-linker-plugin -flto-partition=none } \
+	      { -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects }
+          ]
+        } else {
+          set LTO_TORTURE_OPTIONS [list \
+	      { -O2 -flto -flto-partition=none } \
+	      { -O2 -flto }
+          ]
+        }
+    }
 }
 
 if [info exists ADDITIONAL_TORTURE_OPTIONS] {
@@ -65,23 +82,6 @@ if { $orig_environment_saved == 0 } {
     set_ld_library_path_env_vars
 }
 
-set LTO_TORTURE_OPTIONS ""
-if [check_effective_target_lto] {
-    # When having plugin test both slim and fat LTO and plugin/nonplugin
-    # path.
-    if [check_linker_plugin_available] {
-      set LTO_TORTURE_OPTIONS [list \
-	  { -O2 -flto -fno-use-linker-plugin -flto-partition=none } \
-	  { -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects }
-      ]
-    } else {
-      set LTO_TORTURE_OPTIONS [list \
-	  { -O2 -flto -flto-partition=none } \
-	  { -O2 -flto }
-      ]
-    }
-}
-
 #
 # c-torture-compile -- runs the Tege C-torture test
 #
Index: gcc/testsuite/lib/gcc-dg.exp
===================================================================
--- gcc/testsuite/lib/gcc-dg.exp	(revision 224545)
+++ gcc/testsuite/lib/gcc-dg.exp	(working copy)
@@ -46,6 +46,7 @@ if ![info exists GCC_UNDER_TEST] {
     set GCC_UNDER_TEST "[find_gcc]"
 }
 
+set LTO_TORTURE_OPTIONS ""
 if [info exists TORTURE_OPTIONS] {
     set DG_TORTURE_OPTIONS $TORTURE_OPTIONS
 } else {
@@ -65,6 +66,23 @@ if [info exists TORTURE_OPTIONS] {
 	{ -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions } \
 	{ -O3 -g } \
 	{ -Os } ]
+
+    if [check_effective_target_lto] {
+        # When having plugin test both slim and fat LTO and plugin/nonplugin
+        # path.
+	if [check_linker_plugin_available] {
+           set LTO_TORTURE_OPTIONS [list \
+	      { -O2 -flto -fno-use-linker-plugin -flto-partition=none } \
+	      { -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects }
+           ]
+           set gcc_force_conventional_output "-ffat-lto-objects"
+        } else {
+           set LTO_TORTURE_OPTIONS [list \
+	      { -O2 -flto -flto-partition=none } \
+	      { -O2 -flto }
+           ]
+        }
+    }
 }
 
 if [info exists ADDITIONAL_TORTURE_OPTIONS] {
@@ -87,24 +105,6 @@ if { $orig_environment_saved == 0 } {
 global gcc_force_conventional_output
 set gcc_force_conventional_output ""
 
-set LTO_TORTURE_OPTIONS ""
-if [check_effective_target_lto] {
-    # When having plugin test both slim and fat LTO and plugin/nonplugin
-    # path.
-    if [check_linker_plugin_available] {
-      set LTO_TORTURE_OPTIONS [list \
-	  { -O2 -flto -fno-use-linker-plugin -flto-partition=none } \
-	  { -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects }
-      ]
-      set gcc_force_conventional_output "-ffat-lto-objects"
-    } else {
-      set LTO_TORTURE_OPTIONS [list \
-	  { -O2 -flto -flto-partition=none } \
-	  { -O2 -flto }
-      ]
-    }
-}
-
 # Deduce generated files from tool flags, return finalcode string
 proc schedule-cleanups { opts } {
     global additional_sources

Re: [PATCH][testsuite] Fix TORTURE_OPTIONS overriding

[Mike Stump]

On Jun 18, 2015, at 3:10 AM, Richard Biener <rguenther@suse.de> wrote:
> Ok for trunk?

Ok.

Re: [PATCH][testsuite] Fix TORTURE_OPTIONS overriding

[James Greenhalgh]

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]


On Thu, Jun 18, 2015 at 11:10:01AM +0100, Richard Biener wrote:
>
> Currently when doing
>
> make check-gcc RUNTESTFLAGS="TORTURE_OPTIONS=\\\"{ -O3 } { -O2 }\\\"
> dg-torture.exp"
>
> you get -O3 and -O2 but also the two LTO torture option combinations.
> That's undesired (those are the most expensive anyway).  The following
> patch avoids this by setting LTO_TORTURE_OPTIONS only when
> TORTURE_OPTIONS isn't specified.
>
> Tested with and without TORTURE_OPTIONS for C and fortran tortures.
>
> Seems the instruction in c-torture.exp how to override TORTURE_OPTIONS
> is off, RUNTESTFLAGS="TORTURE_OPTIONS=\\\"{ { -O3 } { -O2 } }\\\"
> certainly doesn't do what it should.

This patch causes issues for ARM and AArch64 cross multilib
testing. There are two issues, one is that we now clobber
gcc_force_conventional_output after setting it in the conditional this patch
moved (hits all targets, see the new x86-64 failures like pr61848.c).

The other is that we no longer protect environment settings before calling
check_effective_target_lto, which results in our cross --specs files no
longer being on the path.

I've fixed these issues by rearranging the file again, but I'm not
sure if what I've done is sensible and does not cause other issues. This
seems to bring back the tests I'd lost overnight, and doesn't cause
issues elsewhere.

I've run some cross-tests to ensure this brings back the missing tests,
and a full x86-64 testrun to make sure I haven't dropped any from there.

OK for trunk?

Thanks,
James

---
2015-06-23  James Greenhalgh  <james.greenhalgh@arm.com>

        * lib/c-torture.exp: Don't call check_effective_target_lto
	before setting up environment correctly.
        * lib/gcc-dg.exp: Likewise, and protect
	gcc_force_conventional_output.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Re-PATCH-testsuite-Fix-TORTURE_OPTIONS-overriding.patch --]
[-- Type: text/x-patch;  name=0001-Re-PATCH-testsuite-Fix-TORTURE_OPTIONS-overriding.patch, Size: 3075 bytes --]

diff --git a/gcc/testsuite/lib/c-torture.exp b/gcc/testsuite/lib/c-torture.exp
index 607e7d0..c88c439 100644
--- a/gcc/testsuite/lib/c-torture.exp
+++ b/gcc/testsuite/lib/c-torture.exp
@@ -21,6 +21,20 @@ load_lib file-format.exp
 load_lib target-libpath.exp
 load_lib target-utils.exp
 
+global GCC_UNDER_TEST
+if ![info exists GCC_UNDER_TEST] {
+    set GCC_UNDER_TEST "[find_gcc]"
+}
+
+global orig_environment_saved
+
+# This file may be sourced, so don't override environment settings
+# that have been previously setup.
+if { $orig_environment_saved == 0 } {
+    append ld_library_path [gcc-set-multilib-library-path $GCC_UNDER_TEST]
+    set_ld_library_path_env_vars
+}
+
 # The default option list can be overridden by
 # TORTURE_OPTIONS="{ list1 } ... { listN }"
 
@@ -68,20 +82,6 @@ if [info exists ADDITIONAL_TORTURE_OPTIONS] {
 	[concat $C_TORTURE_OPTIONS $ADDITIONAL_TORTURE_OPTIONS]
 }
 
-global GCC_UNDER_TEST
-if ![info exists GCC_UNDER_TEST] {
-    set GCC_UNDER_TEST "[find_gcc]"
-}
-
-global orig_environment_saved
-
-# This file may be sourced, so don't override environment settings
-# that have been previously setup.
-if { $orig_environment_saved == 0 } {
-    append ld_library_path [gcc-set-multilib-library-path $GCC_UNDER_TEST]
-    set_ld_library_path_env_vars
-}
-
 #
 # c-torture-compile -- runs the Tege C-torture test
 #
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index 00ca0c5..d463f81 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -46,6 +46,19 @@ if ![info exists GCC_UNDER_TEST] {
     set GCC_UNDER_TEST "[find_gcc]"
 }
 
+# This file may be sourced, so don't override environment settings
+# that have been previously setup.
+if { $orig_environment_saved == 0 } {
+    append ld_library_path [gcc-set-multilib-library-path $GCC_UNDER_TEST]
+    set_ld_library_path_env_vars
+}
+
+# Some torture-options cause intermediate code output, unusable for
+# testing using e.g. scan-assembler.  In this variable are the options
+# how to force it, when needed.
+global gcc_force_conventional_output
+set gcc_force_conventional_output ""
+
 set LTO_TORTURE_OPTIONS ""
 if [info exists TORTURE_OPTIONS] {
     set DG_TORTURE_OPTIONS $TORTURE_OPTIONS
@@ -92,19 +105,6 @@ if [info exists ADDITIONAL_TORTURE_OPTIONS] {
 
 global orig_environment_saved
 
-# This file may be sourced, so don't override environment settings
-# that have been previously setup.
-if { $orig_environment_saved == 0 } {
-    append ld_library_path [gcc-set-multilib-library-path $GCC_UNDER_TEST]
-    set_ld_library_path_env_vars
-}
-
-# Some torture-options cause intermediate code output, unusable for
-# testing using e.g. scan-assembler.  In this variable are the options
-# how to force it, when needed.
-global gcc_force_conventional_output
-set gcc_force_conventional_output ""
-
 # Deduce generated files from tool flags, return finalcode string
 proc schedule-cleanups { opts } {
     global additional_sources

Re: [PATCH][testsuite] Fix TORTURE_OPTIONS overriding

[Richard Biener]

On Tue, 23 Jun 2015, James Greenhalgh wrote:

> 
> On Thu, Jun 18, 2015 at 11:10:01AM +0100, Richard Biener wrote:
> >
> > Currently when doing
> >
> > make check-gcc RUNTESTFLAGS="TORTURE_OPTIONS=\\\"{ -O3 } { -O2 }\\\"
> > dg-torture.exp"
> >
> > you get -O3 and -O2 but also the two LTO torture option combinations.
> > That's undesired (those are the most expensive anyway).  The following
> > patch avoids this by setting LTO_TORTURE_OPTIONS only when
> > TORTURE_OPTIONS isn't specified.
> >
> > Tested with and without TORTURE_OPTIONS for C and fortran tortures.
> >
> > Seems the instruction in c-torture.exp how to override TORTURE_OPTIONS
> > is off, RUNTESTFLAGS="TORTURE_OPTIONS=\\\"{ { -O3 } { -O2 } }\\\"
> > certainly doesn't do what it should.
> 
> This patch causes issues for ARM and AArch64 cross multilib
> testing. There are two issues, one is that we now clobber
> gcc_force_conventional_output after setting it in the conditional this patch
> moved (hits all targets, see the new x86-64 failures like pr61848.c).
> 
> The other is that we no longer protect environment settings before calling
> check_effective_target_lto, which results in our cross --specs files no
> longer being on the path.
> 
> I've fixed these issues by rearranging the file again, but I'm not
> sure if what I've done is sensible and does not cause other issues. This
> seems to bring back the tests I'd lost overnight, and doesn't cause
> issues elsewhere.
> 
> I've run some cross-tests to ensure this brings back the missing tests,
> and a full x86-64 testrun to make sure I haven't dropped any from there.
> 
> OK for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> James
> 
> ---
> 2015-06-23  James Greenhalgh  <james.greenhalgh@arm.com>
> 
>         * lib/c-torture.exp: Don't call check_effective_target_lto
> 	before setting up environment correctly.
>         * lib/gcc-dg.exp: Likewise, and protect
> 	gcc_force_conventional_output.
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)