Message ID | 875y99d5rt.fsf@euler.schwinge.homeip.net |
---|---|
State | New |
Headers | show |
Series | Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS' (was: Update testsuite to run with slim LTO) | expand |
On Wed, 3 May 2023, Thomas Schwinge wrote: > Hi! > > This very likely isn't the only instance of such a kind of problem in the > GCC testsuite ;-) -- but it's one that I've run into, and analyzed: > > On 2011-09-27T19:23:22+0200, Jan Hubicka <hubicka@ucw.cz> wrote: > > this patch updates testsuite to cover both fat and slim LTO when linker plugin > > is used [...] > > This change here: > > > *** lib/lto.exp (revision 179274) > > --- lib/lto.exp (working copy) > > *************** proc lto_init { args } { > > *** 66,79 **** > > # You can put this in the environment before site.exp is written or > > # add it to site.exp directly. > > if ![info exists LTO_OPTIONS] { > > ! set LTO_OPTIONS [list \ > > ! {-O0 -flto -flto-partition=none } \ > > ! {-O2 -flto -flto-partition=none } \ > > ! {-O0 -flto -flto-partition=1to1 } \ > > ! {-O2 -flto -flto-partition=1to1 } \ > > ! {-O0 -flto} \ > > ! {-O2 -flto} \ > > ! ] > > } > > } > > > > --- 66,89 ---- > > # You can put this in the environment before site.exp is written or > > # add it to site.exp directly. > > if ![info exists LTO_OPTIONS] { > > ! if [check_linker_plugin_available] { > > ! set LTO_OPTIONS [list \ > > ! {-O0 -flto -flto-partition=none -fuse-linker-plugin} \ > > ! {-O2 -flto -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects } \ > > ! {-O0 -flto -flto-partition=1to1 -fno-use-linker-plugin } \ > > ! {-O2 -flto -flto-partition=1to1 -fno-use-linker-plugin } \ > > ! {-O0 -flto -fuse-linker-plugin -fno-fat-lto-objects } \ > > ! {-O2 -flto -fuse-linker-plugin} \ > > ! ] > > ! } else { > > ! set LTO_OPTIONS [list \ > > ! {-O0 -flto -flto-partition=none } \ > > ! {-O2 -flto -flto-partition=none } \ > > ! {-O0 -flto -flto-partition=1to1 } \ > > ! {-O2 -flto -flto-partition=1to1 } \ > > ! {-O0 -flto } \ > > ! {-O2 -flto} \ > > ! } > > } > > } > > ... is problematic: initializing the persistent 'LTO_OPTIONS' dependent > on 'check_linker_plugin_available' that is (potentially) variable per > testing variant ('RUNTESTFLAGS=--target_board=unix\{-m64,-m32\}', for > example). > > Similarly: > > > *** lib/c-torture.exp (revision 179274) > > --- lib/c-torture.exp (working copy) > > *************** if [info exists ADDITIONAL_TORTURE_OPTIO > > *** 52,61 **** > > > > set LTO_TORTURE_OPTIONS "" > > if [check_effective_target_lto] { > > ! set LTO_TORTURE_OPTIONS [list \ > > ! { -O2 -flto -flto-partition=none } \ > > ! { -O2 -flto } > > ! ] > > } > > > > global GCC_UNDER_TEST > > --- 52,69 ---- > > > > 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 -fuse-linker-plugin } > > ! } > > } > > ..., and: > > > *** lib/gcc-dg.exp (revision 179274) > > --- lib/gcc-dg.exp (working copy) > > *************** if [info exists ADDITIONAL_TORTURE_OPTIO > > *** 69,78 **** > > > > set LTO_TORTURE_OPTIONS "" > > if [check_effective_target_lto] { > > ! set LTO_TORTURE_OPTIONS [list \ > > ! { -O2 -flto -flto-partition=none } \ > > ! { -O2 -flto } > > ! ] > > } > > > > > > --- 69,86 ---- > > > > 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 -fuse-linker-plugin } > > ! } > > } > > OK to push the attached LGTM, please leave the others a chance to comment in case they spotted anything wrong. Richard. > "Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS'"? > > > Gr??e > Thomas > > > ----------------- > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra?e 201, 80634 M?nchen; Gesellschaft mit beschr?nkter Haftung; Gesch?ftsf?hrer: Thomas Heurung, Frank Th?rauf; Sitz der Gesellschaft: M?nchen; Registergericht M?nchen, HRB 106955 >
Hi! On Wed, 3 May 2023 at 13:47, Richard Biener via Gcc-patches < gcc-patches@gcc.gnu.org> wrote: > On Wed, 3 May 2023, Thomas Schwinge wrote: > > > Hi! > > > > This very likely isn't the only instance of such a kind of problem in the > > GCC testsuite ;-) -- but it's one that I've run into, and analyzed: > > > > On 2011-09-27T19:23:22+0200, Jan Hubicka <hubicka@ucw.cz> wrote: > > > this patch updates testsuite to cover both fat and slim LTO when > linker plugin > > > is used [...] > > > > This change here: > > > > > *** lib/lto.exp (revision 179274) > > > --- lib/lto.exp (working copy) > > > *************** proc lto_init { args } { > > > *** 66,79 **** > > > # You can put this in the environment before site.exp is written > or > > > # add it to site.exp directly. > > > if ![info exists LTO_OPTIONS] { > > > ! set LTO_OPTIONS [list \ > > > ! {-O0 -flto -flto-partition=none } \ > > > ! {-O2 -flto -flto-partition=none } \ > > > ! {-O0 -flto -flto-partition=1to1 } \ > > > ! {-O2 -flto -flto-partition=1to1 } \ > > > ! {-O0 -flto} \ > > > ! {-O2 -flto} \ > > > ! ] > > > } > > > } > > > > > > --- 66,89 ---- > > > # You can put this in the environment before site.exp is written > or > > > # add it to site.exp directly. > > > if ![info exists LTO_OPTIONS] { > > > ! if [check_linker_plugin_available] { > > > ! set LTO_OPTIONS [list \ > > > ! {-O0 -flto -flto-partition=none -fuse-linker-plugin} \ > > > ! {-O2 -flto -flto-partition=none -fuse-linker-plugin > -fno-fat-lto-objects } \ > > > ! {-O0 -flto -flto-partition=1to1 -fno-use-linker-plugin } \ > > > ! {-O2 -flto -flto-partition=1to1 -fno-use-linker-plugin } \ > > > ! {-O0 -flto -fuse-linker-plugin -fno-fat-lto-objects } \ > > > ! {-O2 -flto -fuse-linker-plugin} \ > > > ! ] > > > ! } else { > > > ! set LTO_OPTIONS [list \ > > > ! {-O0 -flto -flto-partition=none } \ > > > ! {-O2 -flto -flto-partition=none } \ > > > ! {-O0 -flto -flto-partition=1to1 } \ > > > ! {-O2 -flto -flto-partition=1to1 } \ > > > ! {-O0 -flto } \ > > > ! {-O2 -flto} \ > > > ! } > > > } > > > } > > > > ... is problematic: initializing the persistent 'LTO_OPTIONS' dependent > > on 'check_linker_plugin_available' that is (potentially) variable per > > testing variant ('RUNTESTFLAGS=--target_board=unix\{-m64,-m32\}', for > > example). > > > > Similarly: > > > > > *** lib/c-torture.exp (revision 179274) > > > --- lib/c-torture.exp (working copy) > > > *************** if [info exists ADDITIONAL_TORTURE_OPTIO > > > *** 52,61 **** > > > > > > set LTO_TORTURE_OPTIONS "" > > > if [check_effective_target_lto] { > > > ! set LTO_TORTURE_OPTIONS [list \ > > > ! { -O2 -flto -flto-partition=none } \ > > > ! { -O2 -flto } > > > ! ] > > > } > > > > > > global GCC_UNDER_TEST > > > --- 52,69 ---- > > > > > > 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 -fuse-linker-plugin } > > > ! } > > > } > > > > ..., and: > > > > > *** lib/gcc-dg.exp (revision 179274) > > > --- lib/gcc-dg.exp (working copy) > > > *************** if [info exists ADDITIONAL_TORTURE_OPTIO > > > *** 69,78 **** > > > > > > set LTO_TORTURE_OPTIONS "" > > > if [check_effective_target_lto] { > > > ! set LTO_TORTURE_OPTIONS [list \ > > > ! { -O2 -flto -flto-partition=none } \ > > > ! { -O2 -flto } > > > ! ] > > > } > > > > > > > > > --- 69,86 ---- > > > > > > 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 -fuse-linker-plugin } > > > ! } > > > } > > > > OK to push the attached > > LGTM, please leave the others a chance to comment in case they > spotted anything wrong. > > Richard. > > > "Let each 'lto_init' determine the default 'LTO_OPTIONS', and > 'torture-init' the 'LTO_TORTURE_OPTIONS'"? > > This is causing issues on arm/aarch64, including: ERROR: can't read "LTO_TORTURE_OPTIONS": no such variable in gcc.target/arm/acle/acle.exp: ERROR: torture-init: LTO_TORTURE_OPTIONS is not empty as expected in gcc.target/aarch64/sls-mitigation/sls-mitigation.exp, gcc.target/aarch64/sve/acle/aarch64-sve-acle-asm.exp, gcc.target/aarch64/sve2/acle/aarch64-sve2-acle-asm.exp, gcc.target/aarch64/torture/aarch64-torture.exp and maybe others Are other targets affected too? > > > > Gr??e > > Thomas > > > > > > ----------------- > > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra?e 201, > 80634 M?nchen; Gesellschaft mit beschr?nkter Haftung; Gesch?ftsf?hrer: > Thomas Heurung, Frank Th?rauf; Sitz der Gesellschaft: M?nchen; > Registergericht M?nchen, HRB 106955 > > > > -- > Richard Biener <rguenther@suse.de> > SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg, > Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman; > HRB 36809 (AG Nuernberg) >
From 75ac220ba923436d2df1b5bbb6356aba2bba1bfe Mon Sep 17 00:00:00 2001 From: Thomas Schwinge <thomas@codesourcery.com> Date: Tue, 2 May 2023 19:57:47 +0200 Subject: [PATCH] Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS' Otherwise, for example for 'RUNTESTFLAGS' of '--target_board=unix\{-m64,-m32\}' vs. '--target_board=unix\{-m32,-m64\}', both variants exercise testing with always the first flag variant's 'LTO_OPTIONS'/'LTO_TORTURE_OPTIONS', which results in unequal test results between the two 'RUNTESTFLAGS' variants if one of the flag variants has 'check_linker_plugin_available' but the other doesn't. Fix-up for r180245 (commit c1a7cdbbcca90ad5260bfc543f8c10f3514e76c1) "Update testsuite to run with slim LTO". gcc/testsuite/ * g++.dg/guality/guality.exp: Move 'torture-init' earlier. * gcc.dg/guality/guality.exp: Likewise. * gfortran.dg/guality/guality.exp: Likewise. * lib/c-torture.exp (LTO_TORTURE_OPTIONS): Don't set. * lib/gcc-dg.exp (LTO_TORTURE_OPTIONS): Don't set. * lib/lto.exp (lto_init, lto_finish): Let each 'lto_init' determine the default 'LTO_OPTIONS'. * lib/torture-options.exp (torture-init, torture-finish): Let each 'torture-init' determine the 'LTO_TORTURE_OPTIONS'. --- gcc/testsuite/g++.dg/guality/guality.exp | 2 +- gcc/testsuite/gcc.dg/guality/guality.exp | 2 +- gcc/testsuite/gfortran.dg/guality/guality.exp | 2 +- gcc/testsuite/lib/c-torture.exp | 17 ----------- gcc/testsuite/lib/gcc-dg.exp | 14 ---------- gcc/testsuite/lib/lto.exp | 13 +++++++++ gcc/testsuite/lib/torture-options.exp | 28 +++++++++++++++++++ 7 files changed, 44 insertions(+), 34 deletions(-) diff --git a/gcc/testsuite/g++.dg/guality/guality.exp b/gcc/testsuite/g++.dg/guality/guality.exp index 2d736d292e93..cd56b06f2f0b 100644 --- a/gcc/testsuite/g++.dg/guality/guality.exp +++ b/gcc/testsuite/g++.dg/guality/guality.exp @@ -37,6 +37,7 @@ proc check_guality {args} { } dg-init +torture-init global GDB if ![info exists ::env(GUALITY_GDB_NAME)] { @@ -54,7 +55,6 @@ report_gdb $::env(GUALITY_GDB_NAME) [info script] global DG_TORTURE_OPTIONS LTO_TORTURE_OPTIONS set guality_dg_torture_options [guality_minimal_options $DG_TORTURE_OPTIONS] -torture-init set-torture-options \ $guality_dg_torture_options \ [list {}] \ diff --git a/gcc/testsuite/gcc.dg/guality/guality.exp b/gcc/testsuite/gcc.dg/guality/guality.exp index 075bebe34e89..a8f2921d8881 100644 --- a/gcc/testsuite/gcc.dg/guality/guality.exp +++ b/gcc/testsuite/gcc.dg/guality/guality.exp @@ -37,6 +37,7 @@ proc check_guality {args} { } dg-init +torture-init global GDB if ![info exists ::env(GUALITY_GDB_NAME)] { @@ -69,7 +70,6 @@ global DG_TORTURE_OPTIONS set guality_dg_torture_options [guality_minimal_options $DG_TORTURE_OPTIONS] set guality_dg_torture_options [guality_transform_options $guality_dg_torture_options] set guality_lto_torture_options [guality_transform_options $LTO_TORTURE_OPTIONS] -torture-init set-torture-options \ $guality_dg_torture_options \ [list {}] \ diff --git a/gcc/testsuite/gfortran.dg/guality/guality.exp b/gcc/testsuite/gfortran.dg/guality/guality.exp index 86a966a9133c..610449523f06 100644 --- a/gcc/testsuite/gfortran.dg/guality/guality.exp +++ b/gcc/testsuite/gfortran.dg/guality/guality.exp @@ -18,6 +18,7 @@ if { [istarget "powerpc-ibm-aix*"] } { } dg-init +torture-init global GDB if ![info exists ::env(GUALITY_GDB_NAME)] { @@ -35,7 +36,6 @@ report_gdb $::env(GUALITY_GDB_NAME) [info script] global DG_TORTURE_OPTIONS set guality_dg_torture_options [guality_minimal_options $DG_TORTURE_OPTIONS] -torture-init set-torture-options \ $guality_dg_torture_options \ diff --git a/gcc/testsuite/lib/c-torture.exp b/gcc/testsuite/lib/c-torture.exp index e54437d5f878..f62c6e9fe6ce 100644 --- a/gcc/testsuite/lib/c-torture.exp +++ b/gcc/testsuite/lib/c-torture.exp @@ -38,7 +38,6 @@ if { $orig_environment_saved == 0 } { # The default option list can be overridden by # TORTURE_OPTIONS="{ list1 } ... { listN }" -set LTO_TORTURE_OPTIONS "" if [info exists TORTURE_OPTIONS] { set C_TORTURE_OPTIONS $TORTURE_OPTIONS } else { @@ -57,22 +56,6 @@ 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] { diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp index 9d79b9402e9e..693cbdd7511a 100644 --- a/gcc/testsuite/lib/gcc-dg.exp +++ b/gcc/testsuite/lib/gcc-dg.exp @@ -73,7 +73,6 @@ if { $orig_environment_saved == 0 } { 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 } else { @@ -93,19 +92,6 @@ if [info exists TORTURE_OPTIONS] { { -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 } - ] - } else { - set LTO_TORTURE_OPTIONS [list \ - { -O2 -flto -flto-partition=none } \ - { -O2 -flto } - ] - } set gcc_force_conventional_output "-ffat-lto-objects" } } diff --git a/gcc/testsuite/lib/lto.exp b/gcc/testsuite/lib/lto.exp index 9e44c443bdbd..c95e7d0a505d 100644 --- a/gcc/testsuite/lib/lto.exp +++ b/gcc/testsuite/lib/lto.exp @@ -215,6 +215,11 @@ proc lto_init { args } { {-O2 -flto} \ ] } + global lto_init_set_LTO_OPTIONS + if [info exists lto_init_set_LTO_OPTIONS] { + error "lto_init_set_LTO_OPTIONS already set" + } + set lto_init_set_LTO_OPTIONS 1 } } @@ -233,6 +238,14 @@ proc lto_finish { } { } elseif [board_info $dest exists mathlib] { unset board_info($dest,mathlib) } + + # Let the next 'lto_init' redetermine the default 'LTO_OPTIONS'. + global lto_init_set_LTO_OPTIONS + if [info exists lto_init_set_LTO_OPTIONS] { + global LTO_OPTIONS + unset LTO_OPTIONS + unset lto_init_set_LTO_OPTIONS + } } # Subsets of tests can be selectively disabled by members of this list: diff --git a/gcc/testsuite/lib/torture-options.exp b/gcc/testsuite/lib/torture-options.exp index 61295604b5bf..394418e9a02f 100644 --- a/gcc/testsuite/lib/torture-options.exp +++ b/gcc/testsuite/lib/torture-options.exp @@ -28,6 +28,27 @@ proc torture-init { args } { if [info exists torture_with_loops] { error "torture-init: torture_with_loops is not empty as expected" } + + global LTO_TORTURE_OPTIONS + if [info exists LTO_TORTURE_OPTIONS] { + error "torture-init: LTO_TORTURE_OPTIONS is not empty as expected" + } + 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 } + ] + } + } } # Return 1 if torture options have already been set, 0 otherwise. @@ -100,6 +121,13 @@ proc torture-finish { args } { } else { error "torture-finish: torture_with_loops is not defined" } + + global LTO_TORTURE_OPTIONS + if [info exists LTO_TORTURE_OPTIONS] { + unset LTO_TORTURE_OPTIONS + } else { + error "torture-finish: LTO_TORTURE_OPTIONS is not defined" + } } # Useful for debugging .exp files. -- 2.39.2