Update testsuite to run with slim LTO

Update testsuite to run with slim LTO

[Jan Hubicka]

Hi,
this patch updates testsuite to cover both fat and slim LTO when linker plugin
is used and also both linker plugin and collect2 paths.  I didn't wanted to
slow down testing too much so I just distributes the flags across existing runs
with aim to maximize the coverage of testing matrix that is bit large now.
I believe it is sufficient and testsuite now runs a bit faster than previously
since slim LTO saves some effort.

sync and pr34850 tests doesn't pass with slim LTO. The reason is that they
excpects diagnostics that is output too late in compilation (usually at
expansion time).  These should be probably fixed as QOI issue but they are not
real bug - the diagnostics will be output at linktime.  I will open PR tracking
this.  We probably should output pretty much everything till end of early opts
except for stuff that really looks for optimization results.  Especially now
when we handle always inline in early inlining.

Honza

	* lib/lto.exp: When linker plugin is available test both
	plugin/non-plugin LTO paths as well as fat and slim LTO.
	lib/c-torture.exp: Likewise.
	lib/gcc-dg.exp: Likweise
	* gcc.c-torture/compile/sync-1.c: Do not run with slim LTO.
	* gcc.c-torture/compile/sync-2.c: Do not run with slim LTO.
	* gcc.c-torture/compile/sync-3.c: Do not run with slim LTO.
	* gcc.dg/noncompile/920507-1.c: Do not run with slim LTO.
	* g++.dg/torture/pr34850.C: Do not run with slim LTO
Index: lib/lto.exp
===================================================================
*** 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_effective_target_lto] {
! 	  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}		\
! 	}
      }
  }
  
Index: lib/c-torture.exp
===================================================================
*** 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 }
!     }
  }
  
  global GCC_UNDER_TEST
Index: lib/gcc-dg.exp
===================================================================
*** 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 }
!     }
  }
  
  
Index: gcc.c-torture/execute/builtins/strstr-asm.c
===================================================================
*** gcc.c-torture/execute/builtins/strstr-asm.c	(revision 179274)
--- gcc.c-torture/execute/builtins/strstr-asm.c	(working copy)
***************
*** 2,7 ****
--- 2,8 ----
  
     Ensure all expected transformations of builtin strstr occur and
     perform correctly in presence of redirect.  */
+ /* { dg-options "-ffat-lto-objects" } */
  
  #define ASMNAME(cname)  ASMNAME2 (__USER_LABEL_PREFIX__, cname)
  #define ASMNAME2(prefix, cname) STRING (prefix) cname
Index: gcc.c-torture/compile/sync-1.c
===================================================================
*** gcc.c-torture/compile/sync-1.c	(revision 179274)
--- gcc.c-torture/compile/sync-1.c	(working copy)
***************
*** 1,5 ****
--- 1,6 ----
  /* { dg-message "note: '__sync_fetch_and_nand' changed semantics in GCC 4.4" "" { target *-*-* } 0 } */
  /* { dg-message "note: '__sync_nand_and_fetch' changed semantics in GCC 4.4" "" { target *-*-* } 0 } */
+ /* { dg-options "-ffat-lto-objects" } */
  
  /* Validate that each of the __sync builtins compiles.  This won't 
     necessarily link, since the target might not support the builtin,
Index: gcc.c-torture/compile/sync-3.c
===================================================================
*** gcc.c-torture/compile/sync-3.c	(revision 179274)
--- gcc.c-torture/compile/sync-3.c	(working copy)
***************
*** 1,4 ****
--- 1,5 ----
  /* { dg-message "note: '__sync_fetch_and_nand' changed semantics in GCC 4.4" "" { target *-*-* } 0 } */
+ /* { dg-options "-ffat-lto-objects" } */
  
  /* Validate that each of the __sync builtins compiles.  This won't 
     necessarily link, since the target might not support the builtin,
Index: gcc.c-torture/compile/sync-2.c
===================================================================
*** gcc.c-torture/compile/sync-2.c	(revision 179274)
--- gcc.c-torture/compile/sync-2.c	(working copy)
***************
*** 1,4 ****
--- 1,5 ----
  /* { dg-message "note: '__sync_fetch_and_nand' changed semantics in GCC 4.4" "" { target *-*-* } 0 } */
+ /* { dg-options "-ffat-lto-objects" } */
  
  /* Validate that each of the __sync builtins compiles.  This won't 
     necessarily link, since the target might not support the builtin,
Index: gcc.dg/noncompile/invalid_asm.c
===================================================================
*** gcc.dg/noncompile/invalid_asm.c	(revision 179274)
--- gcc.dg/noncompile/invalid_asm.c	(working copy)
***************
*** 1,3 ****
--- 1,4 ----
+ /* { dg-options "-ffat-lto-objects" } */
  asm_invalid_register_name()
  {
    asm("":::"this_is_an_invalid_register_name");	/* { dg-error "unknown register" } */
Index: gcc.dg/noncompile/920507-1.c
===================================================================
*** gcc.dg/noncompile/920507-1.c	(revision 179274)
--- gcc.dg/noncompile/920507-1.c	(working copy)
***************
*** 1,3 ****
--- 1,4 ----
+ /* { dg-options "-ffat-lto-objects" } */
  int *
  x(void)
  {
Index: gcc.dg/torture/pr36400.c
===================================================================
*** gcc.dg/torture/pr36400.c	(revision 179274)
--- gcc.dg/torture/pr36400.c	(working copy)
***************
*** 1,4 ****
--- 1,5 ----
  /* { dg-do compile } */
+ /* { dg-options "-ffat-lto-objects" } */
  
  struct barstruct { char const* some_string; };
  
Index: g++.dg/torture/pr34850.C
===================================================================
*** g++.dg/torture/pr34850.C	(revision 179283)
--- g++.dg/torture/pr34850.C	(working copy)
***************
*** 1,5 ****
--- 1,6 ----
  /* { dg-do compile } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
+ /* { dg-options "-ffat-lto-objects" } */
  
  typedef unsigned char uint8_t;
  typedef unsigned int uint32_t;

Re: Update testsuite to run with slim LTO

[Diego Novillo]

On 11-09-27 13:23 , Jan Hubicka wrote:

> sync and pr34850 tests doesn't pass with slim LTO. The reason is that they
> excpects diagnostics that is output too late in compilation (usually at
> expansion time).  These should be probably fixed as QOI issue but they are not
> real bug - the diagnostics will be output at linktime.  I will open PR tracking
> this.  We probably should output pretty much everything till end of early opts
> except for stuff that really looks for optimization results.  Especially now
> when we handle always inline in early inlining.

Could you add a link to the PR in these tests?

> 	* lib/lto.exp: When linker plugin is available test both
> 	plugin/non-plugin LTO paths as well as fat and slim LTO.
> 	lib/c-torture.exp: Likewise.
> 	lib/gcc-dg.exp: Likweise
> 	* gcc.c-torture/compile/sync-1.c: Do not run with slim LTO.
> 	* gcc.c-torture/compile/sync-2.c: Do not run with slim LTO.
> 	* gcc.c-torture/compile/sync-3.c: Do not run with slim LTO.
> 	* gcc.dg/noncompile/920507-1.c: Do not run with slim LTO.
> 	* g++.dg/torture/pr34850.C: Do not run with slim LTO

OK with the change above.


Diego.

Breakage with "Update testsuite to run with slim LTO"

[Hans-Peter Nilsson]

> Date: Tue, 27 Sep 2011 19:23:22 +0200
> From: Jan Hubicka <hubicka@ucw.cz>

> this patch updates testsuite to cover both fat and slim LTO when linker plugin
> is used and also both linker plugin and collect2 paths.  I didn't wanted to
> slow down testing too much so I just distributes the flags across existing runs
> with aim to maximize the coverage of testing matrix that is bit large now.
> I believe it is sufficient and testsuite now runs a bit faster than previously
> since slim LTO saves some effort.
> 
> sync and pr34850 tests doesn't pass with slim LTO. The reason is that they
> excpects diagnostics that is output too late in compilation (usually at
> expansion time).  These should be probably fixed as QOI issue but they are not
> real bug - the diagnostics will be output at linktime.  I will open PR tracking
> this.  We probably should output pretty much everything till end of early opts
> except for stuff that really looks for optimization results.  Especially now
> when we handle always inline in early inlining.
> 
> Honza
> 
> 	* lib/lto.exp: When linker plugin is available test both
> 	plugin/non-plugin LTO paths as well as fat and slim LTO.
> 	lib/c-torture.exp: Likewise.
> 	lib/gcc-dg.exp: Likweise

Looks like this patch broke, for cris-elf with TOT binutils:

Running /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.dg/torture/dg-torture.exp ...
FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler in-asm: .mof
FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler out-asm: .mof
FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler in2-asm: .mof
FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler out2-asm: .mof

which for "-O2 -flto -fuse-linker-plugin -fno-fat-lto-objects"
don't produce any code.  Is that expected? 

If so, and if the required update is as for the test-cases you
updated, to add:
+ /* { dg-options "-ffat-lto-objects" } */

then IIUC you need to patch *all* torture tests that use
scan-assembler and scan-assembler-not.  Alternatively, patch
somewhere else, like not passing it if certain directives are
used, like scan-assembler{,-not}.  And either way, is it safe to
add that option always, not just when also passing "-flto" or
something?

brgds, H-P

Re: Breakage with "Update testsuite to run with slim LTO"

[Andi Kleen]

> Looks like this patch broke, for cris-elf with TOT binutils:
>
> Running /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.dg/torture/dg-torture.exp ...
> FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler in-asm: .mof
> FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler out-asm: .mof
> FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler in2-asm: .mof
> FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler out2-asm: .mof
>
> which for "-O2 -flto -fuse-linker-plugin -fno-fat-lto-objects"
> don't produce any code.  Is that expected?

Yes. -fno-fat-lto-objects does only produce code at the final link.

-Andi

Re: Breakage with "Update testsuite to run with slim LTO"

[Jan Hubicka]

> > Date: Tue, 27 Sep 2011 19:23:22 +0200
> > From: Jan Hubicka <hubicka@ucw.cz>
> 
> > this patch updates testsuite to cover both fat and slim LTO when linker plugin
> > is used and also both linker plugin and collect2 paths.  I didn't wanted to
> > slow down testing too much so I just distributes the flags across existing runs
> > with aim to maximize the coverage of testing matrix that is bit large now.
> > I believe it is sufficient and testsuite now runs a bit faster than previously
> > since slim LTO saves some effort.
> > 
> > sync and pr34850 tests doesn't pass with slim LTO. The reason is that they
> > excpects diagnostics that is output too late in compilation (usually at
> > expansion time).  These should be probably fixed as QOI issue but they are not
> > real bug - the diagnostics will be output at linktime.  I will open PR tracking
> > this.  We probably should output pretty much everything till end of early opts
> > except for stuff that really looks for optimization results.  Especially now
> > when we handle always inline in early inlining.
> > 
> > Honza
> > 
> > 	* lib/lto.exp: When linker plugin is available test both
> > 	plugin/non-plugin LTO paths as well as fat and slim LTO.
> > 	lib/c-torture.exp: Likewise.
> > 	lib/gcc-dg.exp: Likweise
> 
> Looks like this patch broke, for cris-elf with TOT binutils:
> 
> Running /tmp/hpautotest-gcc1/gcc/gcc/testsuite/gcc.dg/torture/dg-torture.exp ...
> FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler in-asm: .mof
> FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler out-asm: .mof
> FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler in2-asm: .mof
> FAIL: gcc.dg/torture/cris-asm-mof-1.c scan-assembler out2-asm: .mof
> 
> which for "-O2 -flto -fuse-linker-plugin -fno-fat-lto-objects"
> don't produce any code.  Is that expected? 
> 
> If so, and if the required update is as for the test-cases you
> updated, to add:
> + /* { dg-options "-ffat-lto-objects" } */

Yes, if we scan assembler, we likely want -fno-fat-lto-objects.
> 
> then IIUC you need to patch *all* torture tests that use
> scan-assembler and scan-assembler-not.  Alternatively, patch
> somewhere else, like not passing it if certain directives are
> used, like scan-assembler{,-not}.  And either way, is it safe to
> add that option always, not just when also passing "-flto" or
> something?

Hmm, some of assembler scans still works because they check for
presence of symbols we output anyway, but indeed, it would make more
sense to automatically imply -ffat-lto-object when scan-assembler
is used.  I am not sure if my dejagnu skill as on par here however.

Honza
> 
> brgds, H-P

[RFA:] fix breakage with "Update testsuite to run with slim LTO"

[Hans-Peter Nilsson]

> Date: Fri, 21 Oct 2011 00:19:32 +0200
> From: Jan Hubicka <hubicka@ucw.cz>
> Yes, if we scan assembler, we likely want -fno-fat-lto-objects.

> > then IIUC you need to patch *all* torture tests that use
> > scan-assembler and scan-assembler-not.  Alternatively, patch
> > somewhere else, like not passing it if certain directives are
> > used, like scan-assembler{,-not}.  And either way, is it safe to
> > add that option always, not just when also passing "-flto" or
> > something?
> 
> Hmm, some of assembler scans still works because they check for
> presence of symbols we output anyway, but indeed, it would make more
> sense to automatically imply -ffat-lto-object when scan-assembler
> is used.  I am not sure if my dejagnu skill as on par here however.

Maybe you could make amends ;) by testing the following, which
seems to work at least for dg-torture.exp and cris-elf/cris-sim,
in which -ffat-lto-object is automatically added for each
scan-assembler and scan-assembler-not test, extensible for other
dg-final actions without polluting with checking LTO options and
whatnot across the files.  I checked (and corrected) so it also
works when !check_effective_target_lto by commenting out the
setting in the second chunk.

gcc/testsuite:

	* lib/gcc-dg.exp (gcc_force_conventional_output): New global
	variable, default empty, -ffat-lto-objects for effective_target_lto.
	(gcc-dg-test-1): Add options from dg-final methods.
	* lib/scanasm.exp (scan-assembler_required_options)
	(scan-assembler-not_required_options): New procs.

Ok to commit?

Index: lib/gcc-dg.exp
===================================================================
--- lib/gcc-dg.exp	(revision 180270)
+++ lib/gcc-dg.exp	(working copy)
@@ -68,6 +68,13 @@ if [info exists ADDITIONAL_TORTURE_OPTIO
 }
 
 set LTO_TORTURE_OPTIONS ""
+
+# 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 ""
+
 if [check_effective_target_lto] {
     # When having plugin test both slim and fat LTO and plugin/nonplugin
     # path.
@@ -76,6 +83,7 @@ if [check_effective_target_lto] {
 	  { -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 } \
@@ -154,6 +162,19 @@ proc gcc-dg-test-1 { target_compile prog
 	default {
 	    perror "$do_what: not a valid dg-do keyword"
 	    return ""
+	}
+    }
+
+    # Let { dg-final { action } } force options as returned by an
+    # optional proc ${action}_required_options.
+    upvar 2 dg-final-code finalcode
+    foreach x [split $finalcode "\n"] {
+	set finalcmd [lindex $x 0]
+	if { [info procs ${finalcmd}_required_options] != "" } {
+	    set req [${finalcmd}_required_options]
+	    if { $req != "" } {
+		lappend extra_tool_flags $req
+	    }
 	}
     }
 
Index: lib/scanasm.exp
===================================================================
--- lib/scanasm.exp	(revision 180270)
+++ lib/scanasm.exp	(working copy)
@@ -85,6 +85,11 @@ proc scan-assembler { args } {
     dg-scan "scan-assembler" 1 $testcase $output_file $args
 }
 
+proc scan-assembler_required_options { args } {
+    global gcc_force_conventional_output
+    return $gcc_force_conventional_output
+}
+
 # Check that a pattern is not present in the .s file produced by the
 # compiler.  See dg-scan for details.
 
@@ -94,6 +99,11 @@ proc scan-assembler-not { args } {
     set output_file "[file rootname [file tail $testcase]].s"
 
     dg-scan "scan-assembler-not" 0 $testcase $output_file $args
+}
+
+proc scan-assembler-not_required_options { args } {
+    global gcc_force_conventional_output
+    return $gcc_force_conventional_output
 }
 
 # Return the scan for the assembly for hidden visibility. 

brgds, H-P

Re: [RFA:] fix breakage with "Update testsuite to run with slim LTO"

[Jan Hubicka]

> > Date: Fri, 21 Oct 2011 00:19:32 +0200
> > From: Jan Hubicka <hubicka@ucw.cz>
> > Yes, if we scan assembler, we likely want -fno-fat-lto-objects.
> 
> > > then IIUC you need to patch *all* torture tests that use
> > > scan-assembler and scan-assembler-not.  Alternatively, patch
> > > somewhere else, like not passing it if certain directives are
> > > used, like scan-assembler{,-not}.  And either way, is it safe to
> > > add that option always, not just when also passing "-flto" or
> > > something?
> > 
> > Hmm, some of assembler scans still works because they check for
> > presence of symbols we output anyway, but indeed, it would make more
> > sense to automatically imply -ffat-lto-object when scan-assembler
> > is used.  I am not sure if my dejagnu skill as on par here however.
> 
> Maybe you could make amends ;) by testing the following, which
> seems to work at least for dg-torture.exp and cris-elf/cris-sim,
> in which -ffat-lto-object is automatically added for each
> scan-assembler and scan-assembler-not test, extensible for other
> dg-final actions without polluting with checking LTO options and
> whatnot across the files.  I checked (and corrected) so it also
> works when !check_effective_target_lto by commenting out the
> setting in the second chunk.

Thanks. It looks good to me.  If we ever start scanning LTO assembler output,
we may simply add scan-lto-assembler variants or so...

Honza

Re: [RFA:] fix breakage with "Update testsuite to run with slim LTO"

[Iain Sandoe]

On 21 Oct 2011, at 10:31, Jan Hubicka wrote:

>>> Date: Fri, 21 Oct 2011 00:19:32 +0200
>>> From: Jan Hubicka <hubicka@ucw.cz>
>>> Yes, if we scan assembler, we likely want -fno-fat-lto-objects.
>>
>>>> then IIUC you need to patch *all* torture tests that use
>>>> scan-assembler and scan-assembler-not.  Alternatively, patch
>>>> somewhere else, like not passing it if certain directives are
>>>> used, like scan-assembler{,-not}.  And either way, is it safe to
>>>> add that option always, not just when also passing "-flto" or
>>>> something?
>>>
>>> Hmm, some of assembler scans still works because they check for
>>> presence of symbols we output anyway, but indeed, it would make more
>>> sense to automatically imply -ffat-lto-object when scan-assembler
>>> is used.  I am not sure if my dejagnu skill as on par here however.
>>
>> Maybe you could make amends ;) by testing the following, which
>> seems to work at least for dg-torture.exp and cris-elf/cris-sim,
>> in which -ffat-lto-object is automatically added for each
>> scan-assembler and scan-assembler-not test, extensible for other
>> dg-final actions without polluting with checking LTO options and
>> whatnot across the files.  I checked (and corrected) so it also
>> works when !check_effective_target_lto by commenting out the
>> setting in the second chunk.
>
> Thanks. It looks good to me.  If we ever start scanning LTO  
> assembler output,
> we may simply add scan-lto-assembler variants or so...

It looks like the gnat testsuite is also broken - but HP's fix doesn't  
recover that.
.. will try and take a look - but short on time today,
Iain

Re: [RFA:] fix breakage with "Update testsuite to run with slim LTO"

[Rainer Orth]

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

Iain Sandoe <developer@sandoe-acoustics.co.uk> writes:

> It looks like the gnat testsuite is also broken - but HP's fix doesn't
> recover that.
> .. will try and take a look - but short on time today,

I think I see what's going on: in gnat.log, I find

Running /vol/gcc/src/hg/trunk/local/gcc/testsuite/gnat.dg/dg.exp ...
ERROR: tcl error sourcing library file /vol/gcc/src/hg/trunk/local/gcc/testsuite/lib/gcc-dg.exp.
can't read "GCC_UNDER_TEST": no such variable
can't read "GCC_UNDER_TEST": no such variable
    while executing
"lappend options "compiler=$GCC_UNDER_TEST""
    (procedure "gcc_target_compile" line 37)
    invoked from within
"gcc_target_compile $source $dest $type $options"
    invoked from within
"if [ string match "*.c" $source ] then {
	return [gcc_target_compile $source $dest $type $options]
    }"
    (procedure "gnat_target_compile" line 12)
    invoked from within
"${tool}_target_compile $src $output $compile_type "$options""
    (procedure "check_compile" line 39)
    invoked from within
"check_compile linker_plugin executable {
     int main() { return 0; }
  } {-flto -fuse-linker-plugin}"
    ("eval" body line 1)
    invoked from within
"eval check_compile $args"
    (procedure "check_no_compiler_messages_nocache" line 2)
    invoked from within
"check_no_compiler_messages_nocache linker_plugin executable {
     int main() { return 0; }
  } "-flto -fuse-linker-plugin""
    (procedure "check_linker_plugin_available" line 2)
    invoked from within
"check_linker_plugin_available"
    invoked from within
"if [check_effective_target_lto] {
    # When having plugin test both slim and fat LTO and plugin/nonplugin
    # path.
    if [check_linker_plugin_ava..."
    (file "/vol/gcc/src/hg/trunk/local/gcc/testsuite/lib/gcc-dg.exp" line 71)
    invoked from within
"source /vol/gcc/src/hg/trunk/local/gcc/testsuite/lib/gcc-dg.exp"
    ("uplevel" body line 1)
    invoked from within
"uplevel #0 source /vol/gcc/src/hg/trunk/local/gcc/testsuite/lib/gcc-dg.exp"

If running the gnat.dg testsuite, lib/gcc-dg.exp is now calling
check_linker_plugin_available early, which ultimately calls
${tool}_target_compile.  For all languages but Ada,
${tool}_target_compile can compile .c files just fine, but
gnat_target_compile (which uses gnatmake) cannot, so it falls back to
directly calling gcc_target_compile in that case.  gcc_target_compile
relies on GCC_UNDER_TEST being set, which in this case hasn't yet
happened, thus the error.

My solution (a hack, actually) is to move the initialization of
GCC_UNDER_TEST in gcc-dg.exp before the calls to
check_linker_plugin_available.  x86_64-unknown-linux-gnu testing in
progress, will commit once that's finished.

Btw., the ChangeLog entry for Jan's patch was riddled with typos and
wrong pathnames.  I've corrected that with a separate checkin.

	Rainer


2011-10-21  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* lib/gcc-dg.exp (GCC_UNDER_TEST): Set before calling
	check_linker_plugin_available.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: testsuite-gnat-linker-plugin.patch --]
[-- Type: text/x-patch, Size: 800 bytes --]

# HG changeset patch
# Parent 9c45ed5cb653fa8053d3c7a9d6502a85b0ffbafc
Fix gnat.dg testing with linker plugin check

diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -41,6 +41,11 @@ if { [ishost "*-*-cygwin*"] } {
   setenv LANG C.ASCII
 }
 
+global GCC_UNDER_TEST
+if ![info exists GCC_UNDER_TEST] {
+    set GCC_UNDER_TEST "[find_gcc]"
+}
+
 if [info exists TORTURE_OPTIONS] {
     set DG_TORTURE_OPTIONS $TORTURE_OPTIONS
 } else {
@@ -84,12 +89,6 @@ if [check_effective_target_lto] {
     }
 }
 
-
-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

[-- Attachment #3: Type: text/plain, Size: 143 bytes --]


-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [RFA:] fix breakage with "Update testsuite to run with slim LTO"

[Jan Hubicka]

> If running the gnat.dg testsuite, lib/gcc-dg.exp is now calling
> check_linker_plugin_available early, which ultimately calls
> ${tool}_target_compile.  For all languages but Ada,
> ${tool}_target_compile can compile .c files just fine, but
> gnat_target_compile (which uses gnatmake) cannot, so it falls back to
> directly calling gcc_target_compile in that case.  gcc_target_compile
> relies on GCC_UNDER_TEST being set, which in this case hasn't yet
> happened, thus the error.
> 
> My solution (a hack, actually) is to move the initialization of
> GCC_UNDER_TEST in gcc-dg.exp before the calls to
> check_linker_plugin_available.  x86_64-unknown-linux-gnu testing in
> progress, will commit once that's finished.

Oops, I was under impression that GNAT testsuite is not really using dejagnu
lib so I did not expected a change here.
Thanks for fixing that!

I also noticed that tests scanning output of late optimization passes are
now getting UNRESOLVED state with slim LTO.  We don't really lose coverage
here because we test fat LTO with the other compilation, but probably easiest
is to enfore fat LTO all the time.

Does the following seem resonable?

Honza

	* gcc.dg/torture/pta-ptrarith-1.c: Force fat LTO.
	* gcc.dg/torture/pta-ptrarith-2.c: Likewise.
	* gcc.dg/torture/pr23821.c: Likewise.
	* gcc.dg/torture/pta-ptrarith-3.c: Likewise.
	* gcc.dg/torture/pr45704.c: Likewise.
	* gcc.dg/torture/pr50472.c: Likewise.
	* gcc.dg/torture/ipa-pta-1.c: Likewise.
	* gcc.dg/torture/pta-callused-1.c: Likewise.
	* gcc.dg/torture/pr39074-2.c: Likewise.
	* gcc.dg/torture/pr39074.c: Likewise.
	* gcc.dg/torture/pr42898-2.c: Likewise.
	* gcc.dg/torture/pr42898.c: Likewise.
	* gcc.dg/torture/pta-escape-1.c: Likewise.
	* gcc.dg/torture/ssa-pta-fn-1.c: Likewise.
Index: gcc.dg/torture/pta-ptrarith-1.c
===================================================================
*** gcc.dg/torture/pta-ptrarith-1.c	(revision 180289)
--- gcc.dg/torture/pta-ptrarith-1.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  struct Foo {
--- 1,5 ----
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias -ffat-lto-objects" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  struct Foo {
Index: gcc.dg/torture/pta-ptrarith-2.c
===================================================================
*** gcc.dg/torture/pta-ptrarith-2.c	(revision 180289)
--- gcc.dg/torture/pta-ptrarith-2.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  struct Foo {
--- 1,5 ----
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias -ffat-lto-objects" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  struct Foo {
Index: gcc.dg/torture/pr23821.c
===================================================================
*** gcc.dg/torture/pr23821.c	(revision 180289)
--- gcc.dg/torture/pr23821.c	(working copy)
***************
*** 3,9 ****
  /* At -O1 DOM threads a jump in a non-optimal way which leads to
     the bogus propagation.  */
  /* { dg-skip-if "" { *-*-* } { "-O1" } { "" } } */
! /* { dg-options "-fdump-tree-ivcanon-details" } */
  
  int a[199];
  
--- 3,9 ----
  /* At -O1 DOM threads a jump in a non-optimal way which leads to
     the bogus propagation.  */
  /* { dg-skip-if "" { *-*-* } { "-O1" } { "" } } */
! /* { dg-options "-fdump-tree-ivcanon-details -ffat-lto-objects" } */
  
  int a[199];
  
Index: gcc.dg/torture/pta-ptrarith-3.c
===================================================================
*** gcc.dg/torture/pta-ptrarith-3.c	(revision 180289)
--- gcc.dg/torture/pta-ptrarith-3.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  extern void abort (void);
--- 1,5 ----
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias -ffat-lto-objects" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  extern void abort (void);
Index: gcc.dg/torture/pr45704.c
===================================================================
*** gcc.dg/torture/pr45704.c	(revision 180289)
--- gcc.dg/torture/pr45704.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do compile } */
! /* { dg-options "-fdump-tree-optimized" } */
  
  struct st {
      int ptr;
--- 1,5 ----
  /* { dg-do compile } */
! /* { dg-options "-fdump-tree-optimized -ffat-lto-objects" } */
  
  struct st {
      int ptr;
Index: gcc.dg/torture/pr50472.c
===================================================================
*** gcc.dg/torture/pr50472.c	(revision 180289)
--- gcc.dg/torture/pr50472.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do compile } */
! /* { dg-options "-fdump-tree-optimized" } */
  
  static const unsigned int foo = 1;
  unsigned int test( void )
--- 1,5 ----
  /* { dg-do compile } */
! /* { dg-options "-fdump-tree-optimized -ffat-lto-objects" } */
  
  static const unsigned int foo = 1;
  unsigned int test( void )
Index: gcc.dg/torture/ipa-pta-1.c
===================================================================
*** gcc.dg/torture/ipa-pta-1.c	(revision 180289)
--- gcc.dg/torture/ipa-pta-1.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do compile { target { nonpic } } } */
! /* { dg-options "-fipa-pta -fdump-ipa-pta" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  struct X { char x; char y; };
--- 1,5 ----
  /* { dg-do compile { target { nonpic } } } */
! /* { dg-options "-fipa-pta -fdump-ipa-pta -ffat-lto-objects" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  struct X { char x; char y; };
Index: gcc.dg/torture/pta-callused-1.c
===================================================================
*** gcc.dg/torture/pta-callused-1.c	(revision 180289)
--- gcc.dg/torture/pta-callused-1.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  volatile int i;
--- 1,5 ----
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias -ffat-lto-objects" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  volatile int i;
Index: gcc.dg/torture/pr39074-2.c
===================================================================
*** gcc.dg/torture/pr39074-2.c	(revision 180289)
--- gcc.dg/torture/pr39074-2.c	(working copy)
***************
*** 1,6 ****
  /* { dg-do run } */
  /* { dg-require-effective-target stdint_types } */
! /* { dg-options "-fdump-tree-alias" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  #include <stdint.h>
--- 1,6 ----
  /* { dg-do run } */
  /* { dg-require-effective-target stdint_types } */
! /* { dg-options "-fdump-tree-alias -ffat-lto-objects" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  #include <stdint.h>
Index: gcc.dg/torture/pr39074.c
===================================================================
*** gcc.dg/torture/pr39074.c	(revision 180289)
--- gcc.dg/torture/pr39074.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  typedef __PTRDIFF_TYPE__ intptr_t;
--- 1,5 ----
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias -ffat-lto-objects" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  typedef __PTRDIFF_TYPE__ intptr_t;
Index: gcc.dg/torture/pr42898-2.c
===================================================================
*** gcc.dg/torture/pr42898-2.c	(revision 180289)
--- gcc.dg/torture/pr42898-2.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do compile } */
! /* { dg-options "-fdump-tree-optimized" } */
  
  struct hardware {
    int parm1:8;
--- 1,5 ----
  /* { dg-do compile } */
! /* { dg-options "-fdump-tree-optimized -ffat-lto-objects" } */
  
  struct hardware {
    int parm1:8;
Index: gcc.dg/torture/pr42898.c
===================================================================
*** gcc.dg/torture/pr42898.c	(revision 180289)
--- gcc.dg/torture/pr42898.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do compile } */
! /* { dg-options "-fdump-tree-optimized" } */
  
  struct hardware {
    int parm1:8;
--- 1,5 ----
  /* { dg-do compile } */
! /* { dg-options "-fdump-tree-optimized -ffat-lto-objects" } */
  
  struct hardware {
    int parm1:8;
Index: gcc.dg/torture/pta-escape-1.c
===================================================================
*** gcc.dg/torture/pta-escape-1.c	(revision 180289)
--- gcc.dg/torture/pta-escape-1.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  int *p;
--- 1,5 ----
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias -ffat-lto-objects" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  int *p;
Index: gcc.dg/torture/ssa-pta-fn-1.c
===================================================================
*** gcc.dg/torture/ssa-pta-fn-1.c	(revision 180289)
--- gcc.dg/torture/ssa-pta-fn-1.c	(working copy)
***************
*** 1,5 ****
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  extern void abort (void);
--- 1,5 ----
  /* { dg-do run } */
! /* { dg-options "-fdump-tree-alias -ffat-lto-objects" } */
  /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
  
  extern void abort (void);

Re: [RFA:] fix breakage with "Update testsuite to run with slim LTO"

[Hans-Peter Nilsson]

> Date: Fri, 21 Oct 2011 17:44:15 +0200
> From: Jan Hubicka <hubicka@ucw.cz>

> I also noticed that tests scanning output of late optimization passes are
> now getting UNRESOLVED state with slim LTO.  We don't really lose coverage
> here because we test fat LTO with the other compilation, but probably easiest
> is to enfore fat LTO all the time.
> 
> Does the following seem resonable?
> 
> Honza
> 
> 	* gcc.dg/torture/pta-ptrarith-1.c: Force fat LTO.
> 	* gcc.dg/torture/pta-ptrarith-2.c: Likewise.
> 	* gcc.dg/torture/pr23821.c: Likewise.
> 	* gcc.dg/torture/pta-ptrarith-3.c: Likewise.
> 	* gcc.dg/torture/pr45704.c: Likewise.
> 	* gcc.dg/torture/pr50472.c: Likewise.
> 	* gcc.dg/torture/ipa-pta-1.c: Likewise.
> 	* gcc.dg/torture/pta-callused-1.c: Likewise.
> 	* gcc.dg/torture/pr39074-2.c: Likewise.
> 	* gcc.dg/torture/pr39074.c: Likewise.
> 	* gcc.dg/torture/pr42898-2.c: Likewise.
> 	* gcc.dg/torture/pr42898.c: Likewise.
> 	* gcc.dg/torture/pta-escape-1.c: Likewise.
> 	* gcc.dg/torture/ssa-pta-fn-1.c: Likewise.

Meh...  Please no, this was the kind of scatter-patches my patch
aimed to avoid... for example, easy to miss some tests.

Instead, on top of my patch, just copy the
scan-assembler_required_options proc to a
scan-tree-dump_required_options.  ...no wait, should forcing
fat-lto be done for all tree-dumps?  If only for a subset of
tree-dumps augment the *_required_options proc API to take
arguments that let you check for that.

brgds, H-P

Re: [RFA:] fix breakage with "Update testsuite to run with slim LTO"

[Jan Hubicka]

> 
> Meh...  Please no, this was the kind of scatter-patches my patch
> aimed to avoid... for example, easy to miss some tests.
> 
> Instead, on top of my patch, just copy the
> scan-assembler_required_options proc to a
> scan-tree-dump_required_options.  ...no wait, should forcing
> fat-lto be done for all tree-dumps?  If only for a subset of

Yep, problem is that early tree passes and analysis part of IPA pases is  run
with fat-lto, while late and RTL passes and execution pass of IPA are not. 

I guess we could make ipa-dump/rtl-dump/tree-dump scanning to disable fat lto
and introduce variants intended to scan late tree dumps and ipa execution dumps...
Not sure if it would make more sense than just doing it explicitely in tests.
> tree-dumps augment the *_required_options proc API to take
> arguments that let you check for that.
Well, listing all late tree passes is quite long and changing with time...

Honza
> 
> brgds, H-P

Re: [RFA:] fix breakage with "Update testsuite to run with slim LTO"

[Hans-Peter Nilsson]

> Date: Fri, 21 Oct 2011 20:34:05 +0200
> From: Jan Hubicka <hubicka@ucw.cz>

> I guess we could make ipa-dump/rtl-dump/tree-dump scanning to disable fat lto
> and introduce variants intended to scan late tree dumps and ipa execution dumps...

Ok, sounds like a plan.  Are there any such
scan-tests-with-late-thin-lto at present?

> Not sure if it would make more sense than just doing it
> explicitely in tests.

Depends on the number of tests there'll eventually be; I just
see it as strictly increasing and even now a bit above
break-even for improving the machinery instead of patching
separate tests.

BTW, was your ok approval or do I need ok from a testsuite
maintainer?

brgds, H-P

Re: [RFA:] fix breakage with "Update testsuite to run with slim LTO"

[Richard Guenther]

On Fri, Oct 21, 2011 at 1:56 PM, Rainer Orth
<ro@cebitec.uni-bielefeld.de> wrote:
> Iain Sandoe <developer@sandoe-acoustics.co.uk> writes:
>
>> It looks like the gnat testsuite is also broken - but HP's fix doesn't
>> recover that.
>> .. will try and take a look - but short on time today,
>
> I think I see what's going on: in gnat.log, I find
>
> Running /vol/gcc/src/hg/trunk/local/gcc/testsuite/gnat.dg/dg.exp ...
> ERROR: tcl error sourcing library file /vol/gcc/src/hg/trunk/local/gcc/testsuite/lib/gcc-dg.exp.
> can't read "GCC_UNDER_TEST": no such variable
> can't read "GCC_UNDER_TEST": no such variable
>    while executing
> "lappend options "compiler=$GCC_UNDER_TEST""
>    (procedure "gcc_target_compile" line 37)
>    invoked from within
> "gcc_target_compile $source $dest $type $options"
>    invoked from within
> "if [ string match "*.c" $source ] then {
>        return [gcc_target_compile $source $dest $type $options]
>    }"
>    (procedure "gnat_target_compile" line 12)
>    invoked from within
> "${tool}_target_compile $src $output $compile_type "$options""
>    (procedure "check_compile" line 39)
>    invoked from within
> "check_compile linker_plugin executable {
>     int main() { return 0; }
>  } {-flto -fuse-linker-plugin}"
>    ("eval" body line 1)
>    invoked from within
> "eval check_compile $args"
>    (procedure "check_no_compiler_messages_nocache" line 2)
>    invoked from within
> "check_no_compiler_messages_nocache linker_plugin executable {
>     int main() { return 0; }
>  } "-flto -fuse-linker-plugin""
>    (procedure "check_linker_plugin_available" line 2)
>    invoked from within
> "check_linker_plugin_available"
>    invoked from within
> "if [check_effective_target_lto] {
>    # When having plugin test both slim and fat LTO and plugin/nonplugin
>    # path.
>    if [check_linker_plugin_ava..."
>    (file "/vol/gcc/src/hg/trunk/local/gcc/testsuite/lib/gcc-dg.exp" line 71)
>    invoked from within
> "source /vol/gcc/src/hg/trunk/local/gcc/testsuite/lib/gcc-dg.exp"
>    ("uplevel" body line 1)
>    invoked from within
> "uplevel #0 source /vol/gcc/src/hg/trunk/local/gcc/testsuite/lib/gcc-dg.exp"
>
> If running the gnat.dg testsuite, lib/gcc-dg.exp is now calling
> check_linker_plugin_available early, which ultimately calls
> ${tool}_target_compile.  For all languages but Ada,
> ${tool}_target_compile can compile .c files just fine, but
> gnat_target_compile (which uses gnatmake) cannot, so it falls back to
> directly calling gcc_target_compile in that case.  gcc_target_compile
> relies on GCC_UNDER_TEST being set, which in this case hasn't yet
> happened, thus the error.
>
> My solution (a hack, actually) is to move the initialization of
> GCC_UNDER_TEST in gcc-dg.exp before the calls to
> check_linker_plugin_available.  x86_64-unknown-linux-gnu testing in
> progress, will commit once that's finished.
>
> Btw., the ChangeLog entry for Jan's patch was riddled with typos and
> wrong pathnames.  I've corrected that with a separate checkin.

Still doesn't work for me:

rguenther@murzim:/abuild/rguenther/trunk-g/gcc> make check-gfortran
RUNTESTFLAGS="dg.exp=logical_dot_product.f90"
...
Running /space/rguenther/src/svn/trunk/gcc/testsuite/gfortran.dg/dg.exp ...
ERROR: (DejaGnu) proc "gcc_target_compile linker_plugin14146.o
linker_plugin14146 executable { additional_flags=-flto
additional_flags=-flto-partition=none additional_flags=-save-temps }"
does not exist.
The error code is NONE
The info on the error is:
close: spawn id exp6 not open
    while executing
"close -i exp6"
    invoked from within
"catch "close -i $spawn_id""

This is highly annoying for me ... (reverting Rainers patch doesn't help).

Richard.

ping: [RFA:] testsuite infrastructure for options implied by dg-final methods

[Hans-Peter Nilsson]

Ping.
Subject changed from '[RFA:] fix breakage with "Update testsuite
to run with slim LTO"' except it doesn't fix *all* breakage
introduced by that patch, only the one I observed and intended
to fix.

> Date: Fri, 21 Oct 2011 04:29:20 +0200
> From: Hans-Peter Nilsson <hp@axis.com>

> > Date: Fri, 21 Oct 2011 00:19:32 +0200
> > From: Jan Hubicka <hubicka@ucw.cz>
> > Yes, if we scan assembler, we likely want -fno-fat-lto-objects.
> 
> > > then IIUC you need to patch *all* torture tests that use
> > > scan-assembler and scan-assembler-not.  Alternatively, patch
> > > somewhere else, like not passing it if certain directives are
> > > used, like scan-assembler{,-not}.  And either way, is it safe to
> > > add that option always, not just when also passing "-flto" or
> > > something?
> > 
> > Hmm, some of assembler scans still works because they check for
> > presence of symbols we output anyway, but indeed, it would make more
> > sense to automatically imply -ffat-lto-object when scan-assembler
> > is used.  I am not sure if my dejagnu skill as on par here however.
> 
> Maybe you could make amends ;) by testing the following, which
> seems to work at least for dg-torture.exp and cris-elf/cris-sim,
> in which -ffat-lto-object is automatically added for each
> scan-assembler and scan-assembler-not test, extensible for other
> dg-final actions without polluting with checking LTO options and
> whatnot across the files.  I checked (and corrected) so it also
> works when !check_effective_target_lto by commenting out the
> setting in the second chunk.
> 
> gcc/testsuite:
> 
> 	* lib/gcc-dg.exp (gcc_force_conventional_output): New global
> 	variable, default empty, -ffat-lto-objects for effective_target_lto.
> 	(gcc-dg-test-1): Add options from dg-final methods.
> 	* lib/scanasm.exp (scan-assembler_required_options)
> 	(scan-assembler-not_required_options): New procs.

<http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01917.html>
Ok to commit?

brgds, H-P

ping*2: [RFA:] testsuite infrastructure for options implied by dg-final methods

[Hans-Peter Nilsson]

Ping again, CC:ing testsuite maintainers.

Honza thought this was a good idea, if that helps.

> From: Hans-Peter Nilsson <hp@axis.com>
> Date: Fri, 28 Oct 2011 16:34:07 +0200

> Subject changed from '[RFA:] fix breakage with "Update testsuite
> to run with slim LTO"' except it doesn't fix *all* breakage
> introduced by that patch, only the one I observed and intended
> to fix.
> 
> > Date: Fri, 21 Oct 2011 04:29:20 +0200
> > From: Hans-Peter Nilsson <hp@axis.com>
> 
> > > Date: Fri, 21 Oct 2011 00:19:32 +0200
> > > From: Jan Hubicka <hubicka@ucw.cz>
> > > Yes, if we scan assembler, we likely want -fno-fat-lto-objects.
> > 
> > > > then IIUC you need to patch *all* torture tests that use
> > > > scan-assembler and scan-assembler-not.  Alternatively, patch
> > > > somewhere else, like not passing it if certain directives are
> > > > used, like scan-assembler{,-not}.  And either way, is it safe to
> > > > add that option always, not just when also passing "-flto" or
> > > > something?
> > > 
> > > Hmm, some of assembler scans still works because they check for
> > > presence of symbols we output anyway, but indeed, it would make more
> > > sense to automatically imply -ffat-lto-object when scan-assembler
> > > is used.  I am not sure if my dejagnu skill as on par here however.
> > 
> > Maybe you could make amends ;) by testing the following, which
> > seems to work at least for dg-torture.exp and cris-elf/cris-sim,
> > in which -ffat-lto-object is automatically added for each
> > scan-assembler and scan-assembler-not test, extensible for other
> > dg-final actions without polluting with checking LTO options and
> > whatnot across the files.  I checked (and corrected) so it also
> > works when !check_effective_target_lto by commenting out the
> > setting in the second chunk.
> > 
> > gcc/testsuite:
> > 
> > 	* lib/gcc-dg.exp (gcc_force_conventional_output): New global
> > 	variable, default empty, -ffat-lto-objects for effective_target_lto.
> > 	(gcc-dg-test-1): Add options from dg-final methods.
> > 	* lib/scanasm.exp (scan-assembler_required_options)
> > 	(scan-assembler-not_required_options): New procs.
> 
> <http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01917.html>
> Ok to commit?
> 
> brgds, H-P
> 

Re: ping*2: [RFA:] testsuite infrastructure for options implied by dg-final methods

[Mike Stump]

On Nov 4, 2011, at 6:47 AM, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
> Ping again, CC:ing testsuite maintainers.

With the number of qualified individuals commenting in the thread, I was going to stay out of it.  If all the breakages and review point others had are resolved, Ok.  I kinda don't like the brittle upvars.  If someone has a substantially less brittle method that does not use upvar and is reasonably concise and readable...

Re: ping*2: [RFA:] testsuite infrastructure for options implied by dg-final methods

[Hans-Peter Nilsson]

> From: Mike Stump <mikestump@comcast.net>
> Date: Fri, 4 Nov 2011 17:53:05 +0100

> On Nov 4, 2011, at 6:47 AM, Hans-Peter Nilsson <hans-peter.nilsson@axis.com> wrote:
> > Ping again, CC:ing testsuite maintainers.
> 
> With the number of qualified individuals commenting in the
> thread, I was going to stay out of it.  If all the breakages
> and review point others had are resolved, Ok.

I know people responded to my patch message when reporting
problems after Honza's LTO_TORTURE_OPTIONS change but as I
mentioned, those breakages were unrelated to my patch, and
AFAICT mostly for load_lib ordering problems and for not setting
GCC_UNDER_TEST properly in Honza's original change for use in
other testsuites.  I should have guessed and I'll remember to
scream loudly next time I'm implicated that way. :)

I looked and it seemed that Richi's problems was unresolved,
running
 make check-gfortran RUNTESTFLAGS="dg.exp=logical_dot_product.f90"
supposedly in the obj/gcc subdir - use check-fortran from the
obj/ toplevel. But I can't repeat it neither with nor without my
patch.  Richi?

>  I kinda don't
> like the brittle upvars.

That's just the way it has to be done to inspect the necessary
dejagnu state and how other lib/*.exp infrastructure parts are
doing it, some for exactly the same "upvar" (lto and scanasm).
If this use breaks, they break too.

I thought about it, but found no other way.  Maybe as a future
dejagnu improvements introducing stable hooks in dejagnu would
be one way to do it.  (For dejagnu folks convenience, thread at
<http://gcc.gnu.org/ml/gcc-patches/2011-10/msg01917.html>)

>  If someone has a substantially less
> brittle method that does not use upvar and is reasonably
> concise and readable...

I'll wait a few days.

brgds, H-P

Re: ping*2: [RFA:] testsuite infrastructure for options implied by dg-final methods

[Mike Stump]

On Nov 4, 2011, at 10:41 AM, Hans-Peter Nilsson wrote:
> I'll wait a few days.

No need to wait.  :-)  If someone finds a way and is motivated, they will fix this and the other instances.  :-)  If the hook is at the top or bottom of a routine, one can source the dejagnu file, then one might be able to redefine the proc with a new proc that has a hook and calls the old routine.

Re: ping*2: [RFA:] testsuite infrastructure for options implied by dg-final methods

[Hans-Peter Nilsson]

> From: Mike Stump <mikestump@comcast.net>
> Date: Fri, 4 Nov 2011 18:54:22 +0100

> On Nov 4, 2011, at 10:41 AM, Hans-Peter Nilsson wrote:
> > I'll wait a few days.
> 
> No need to wait.  :-)  If someone finds a way and is
> motivated, they will fix this and the other instances.  :-)

Certainly the expected and preferred chain of events.

> If the hook is at the top or bottom of a routine, one can
> source the dejagnu file, then one might be able to redefine
> the proc with a new proc that has a hook and calls the old
> routine.

Hm.  I guess, but doesn't sound like it'd be pretty, and I'm not
even sure it'd be doable here...  And still the just-committed
approach would be better only for reasons of consistency. :)

Thanks for the review!

brgds, H-P

Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS' (was: Update testsuite to run with slim LTO)

[Thomas Schwinge]

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

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
"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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Let-each-lto_init-determine-the-default-LTO_OPTIONS-.patch --]
[-- Type: text/x-diff, Size: 8393 bytes --]

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

Re: Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS' (was: Update testsuite to run with slim LTO)

[Richard Biener]

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
> 

-- 
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)

Re: Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS' (was: Update testsuite to run with slim LTO)

[Christophe Lyon]

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

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)
>

Testsuite: Add missing 'torture-init'/'torture-finish' around 'LTO_TORTURE_OPTIONS' usage (was: Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS')

[Thomas Schwinge]

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

Hi Christophe!

On 2023-05-09T09:32:55+0200, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> 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:
>> > "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?

Sorry for that -- it means, the safe-guards I added are working as
expected.

Please test whether all these issues are gone with the attached
"Testsuite: Add missing 'torture-init'/'torture-finish' around 'LTO_TORTURE_OPTIONS' usage"?


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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Testsuite-Add-missing-torture-init-torture-finish-ar.patch --]
[-- Type: text/x-diff, Size: 2944 bytes --]

From 5f158fb7a5167e943e1410c7faa30e682ae85c4d Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Tue, 9 May 2023 10:35:27 +0200
Subject: [PATCH] Testsuite: Add missing 'torture-init'/'torture-finish' around
 'LTO_TORTURE_OPTIONS' usage

Recent commit d6654a4be3ba44c0d57be7c8a51d76d9721345e1
"Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS'"
made it a requirement that 'LTO_TORTURE_OPTIONS' usage be within
'torture-init'/'torture-finish', and missed a few cases that didn't have that.

	gcc/testsuite/
	* gcc.target/arm/acle/acle.exp: Add missing
	'torture-init'/'torture-finish' around 'LTO_TORTURE_OPTIONS'
	usage.
	* gcc.target/arm/cmse/cmse.exp: Likewise.
	* gcc.target/arm/pure-code/pure-code.exp: Likewise.
---
 gcc/testsuite/gcc.target/arm/acle/acle.exp           | 3 +++
 gcc/testsuite/gcc.target/arm/cmse/cmse.exp           | 2 ++
 gcc/testsuite/gcc.target/arm/pure-code/pure-code.exp | 2 ++
 3 files changed, 7 insertions(+)

diff --git a/gcc/testsuite/gcc.target/arm/acle/acle.exp b/gcc/testsuite/gcc.target/arm/acle/acle.exp
index 7b99dd72987..4d63ccc9554 100644
--- a/gcc/testsuite/gcc.target/arm/acle/acle.exp
+++ b/gcc/testsuite/gcc.target/arm/acle/acle.exp
@@ -26,6 +26,7 @@ load_lib gcc-dg.exp
 
 # Initialize `dg'.
 dg-init
+torture-init
 
 set saved-dg-do-what-default ${dg-do-what-default}
 set dg-do-what-default "assemble"
@@ -48,5 +49,7 @@ gcc-dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.\[cCS\]]] \
 # Restore globals
 set dg-do-what-default ${saved-dg-do-what-default}
 set LTO_TORTURE_OPTIONS ${saved-lto_torture_options}
+
 # All done.
+torture-finish
 dg-finish
diff --git a/gcc/testsuite/gcc.target/arm/cmse/cmse.exp b/gcc/testsuite/gcc.target/arm/cmse/cmse.exp
index 1d251a4fa1f..0baf8c5a504 100644
--- a/gcc/testsuite/gcc.target/arm/cmse/cmse.exp
+++ b/gcc/testsuite/gcc.target/arm/cmse/cmse.exp
@@ -32,6 +32,7 @@ if ![info exists DEFAULT_CFLAGS] then {
 
 # Initialize `dg'.
 dg-init
+torture-init
 
 set saved-dg-do-what-default ${dg-do-what-default}
 
@@ -104,4 +105,5 @@ set LTO_TORTURE_OPTIONS ${saved-lto_torture_options}
 set dg-do-what-default ${saved-dg-do-what-default}
 
 # All done.
+torture-finish
 dg-finish
diff --git a/gcc/testsuite/gcc.target/arm/pure-code/pure-code.exp b/gcc/testsuite/gcc.target/arm/pure-code/pure-code.exp
index c23392dcdfd..6d32e4a7f8d 100644
--- a/gcc/testsuite/gcc.target/arm/pure-code/pure-code.exp
+++ b/gcc/testsuite/gcc.target/arm/pure-code/pure-code.exp
@@ -35,6 +35,7 @@ if ![info exists DEFAULT_CFLAGS] then {
 if {[check_effective_target_arm_cortex_m]} then {
 # Initialize `dg'.
 dg-init
+torture-init
 
 set saved-dg-do-what-default ${dg-do-what-default}
 set dg-do-what-default "assemble"
@@ -58,5 +59,6 @@ set dg-do-what-default ${saved-dg-do-what-default}
 set LTO_TORTURE_OPTIONS ${saved-lto_torture_options}
 
 # All done.
+torture-finish
 dg-finish
 }
-- 
2.34.1

Re: Testsuite: Add missing 'torture-init'/'torture-finish' around 'LTO_TORTURE_OPTIONS' usage (was: Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS')

[Christophe Lyon]

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

Hi!

On Tue, 9 May 2023 at 11:00, Thomas Schwinge <thomas@codesourcery.com>
wrote:

> Hi Christophe!
>
> On 2023-05-09T09:32:55+0200, Christophe Lyon <christophe.lyon@linaro.org>
> wrote:
> > 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:
> >> > "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?
>
> Sorry for that -- it means, the safe-guards I added are working as
> expected.
>
> Please test whether all these issues are gone with the attached
> "Testsuite: Add missing 'torture-init'/'torture-finish' around
> 'LTO_TORTURE_OPTIONS' usage"?
>
>
Your patch seemed reasonable,  but it doesn't work :-(

Well now I get:
ERROR: torture-init: LTO_TORTURE_OPTIONS is not empty as expected
because gcc-dg-runtest itself calls torture-init

but I'm not sure where LTO_TORTURE_OPTIONS is set

Christophe


> 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
>

Re: Testsuite: Add missing 'torture-init'/'torture-finish' around 'LTO_TORTURE_OPTIONS' usage (was: Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS')

[Christophe Lyon]

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

Hi Thomas,

On Tue, 9 May 2023 at 17:17, Christophe Lyon <christophe.lyon@linaro.org>
wrote:

> Hi!
>
> On Tue, 9 May 2023 at 11:00, Thomas Schwinge <thomas@codesourcery.com>
> wrote:
>
>> Hi Christophe!
>>
>> On 2023-05-09T09:32:55+0200, Christophe Lyon <christophe.lyon@linaro.org>
>> wrote:
>> > 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:
>> >> > "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?
>>
>> Sorry for that -- it means, the safe-guards I added are working as
>> expected.
>>
>> Please test whether all these issues are gone with the attached
>> "Testsuite: Add missing 'torture-init'/'torture-finish' around
>> 'LTO_TORTURE_OPTIONS' usage"?
>>
>>
> Your patch seemed reasonable,  but it doesn't work :-(
>
> Well now I get:
> ERROR: torture-init: LTO_TORTURE_OPTIONS is not empty as expected
> because gcc-dg-runtest itself calls torture-init
>
> but I'm not sure where LTO_TORTURE_OPTIONS is set
>
>
Just checking, are you able to test your changes on arm (a cross toolchain
is OK) ?
The problem shows up even if running only acle.exp, so it's quick once you
have built the toolchain once.

I spent some time looking at it, and the conflict is that the .exp file
calls torture-init and gcc-dg-runtest, which in turn calls torture-init
again, leading to the error.

I haven't checked the details of why there are similar failures on aarch64.

Thanks,

Christophe




> Christophe
>
>
>> 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
>>
>

Testsuite: Add 'torture-init-done', and use it to conditionalize implicit 'torture-init' (was: Testsuite: Add missing 'torture-init'/'torture-finish' around 'LTO_TORTURE_OPTIONS' usage (was: Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS'))

[Thomas Schwinge]

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

Hi Christophe!

On 2023-05-09T21:14:07+0200, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> On Tue, 9 May 2023 at 17:17, Christophe Lyon <christophe.lyon@linaro.org>
> wrote:
>> On Tue, 9 May 2023 at 11:00, Thomas Schwinge <thomas@codesourcery.com>
>> wrote:
>>> On 2023-05-09T09:32:55+0200, Christophe Lyon <christophe.lyon@linaro.org>
>>> wrote:
>>> > 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:
>>> >> > "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?
>>>
>>> Sorry for that -- it means, the safe-guards I added are working as
>>> expected.
>>>
>>> Please test whether all these issues are gone with the attached
>>> "Testsuite: Add missing 'torture-init'/'torture-finish' around
>>> 'LTO_TORTURE_OPTIONS' usage"?
>>
>> Your patch seemed reasonable,  but it doesn't work :-(
>>
>> Well now I get:
>> ERROR: torture-init: LTO_TORTURE_OPTIONS is not empty as expected
>> because gcc-dg-runtest itself calls torture-init
>>
>> but I'm not sure where LTO_TORTURE_OPTIONS is set
>
> Just checking, are you able to test your changes on arm (a cross toolchain
> is OK) ?

Sorry, I don't currently have an arm/aarch64 toolchain built.

> The problem shows up even if running only acle.exp, so it's quick once you
> have built the toolchain once.

I did a quick hack:

    --- gcc/testsuite/gcc.target/aarch64/sls-mitigation/sls-mitigation.exp
    +++ gcc/testsuite/gcc.target/aarch64/sls-mitigation/sls-mitigation.exp
    @@ -22,3 +21,0 @@
    -if {![istarget aarch64*-*-*] } then {
    -  return
    -}
    --- gcc/testsuite/gcc.target/arm/acle/acle.exp
    +++ gcc/testsuite/gcc.target/arm/acle/acle.exp
    @@ -20,3 +19,0 @@
    -if ![istarget arm*-*-*] then {
    -  return
    -}

..., and confirm to run into the DejaGnu/TCL ERRORs in my
x86_64-pc-linux-gnu testing.

> I spent some time looking at it, and the conflict is that the .exp file
> calls torture-init and gcc-dg-runtest, which in turn calls torture-init
> again, leading to the error.

I see, thanks -- and sorry, once again.

> I haven't checked the details of why there are similar failures on aarch64.

I now understand that the problem is the following: most of all '*.exp'
files have 'torture-init' followed by 'set-torture-options' before
'gcc-dg-runtest' etc., and therefore don't run into the latter's
"Some callers set torture options themselves; don't override those."
code.  Some '*.exp' files however do 'torture-init' but not
'set-torture-options', and therefore we can't any longer conditionalize
the implicit 'torture-init' by '![torture-options-exist]'.
Please in addition to the earlier
"Testsuite: Add missing 'torture-init'/'torture-finish' around 'LTO_TORTURE_OPTIONS' usage"
also apply the attached
"Testsuite: Add 'torture-init-done', and use it to conditionalize implicit 'torture-init'".
That hopefully should restore sanity -- if not, I'll get arm/aarch64
toolchains built.


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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Testsuite-Add-torture-init-done-and-use-it-to-condit.patch --]
[-- Type: text/x-diff, Size: 6517 bytes --]

From 4a069073834c5b710e17315c0844f1212fa54164 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <thomas@codesourcery.com>
Date: Wed, 10 May 2023 09:17:47 +0200
Subject: [PATCH] Testsuite: Add 'torture-init-done', and use it to
 conditionalize implicit 'torture-init'

Recent commit d6654a4be3ba44c0d57be7c8a51d76d9721345e1
"Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS'"
made 'torture-init' non-idempotent re 'LTO_TORTURE_OPTIONS', in order to catch
certain classes of errors.  Now, most of all '*.exp' files have 'torture-init'
followed by 'set-torture-options' before 'gcc-dg-runtest' etc., and therefore
don't run into the latter's
"Some callers set torture options themselves; don't override those." code.
Some '*.exp' files however do 'torture-init' but not 'set-torture-options', and
therefore we can't any longer conditionalize the implicit 'torture-init' by
'![torture-options-exist]'.

	gcc/testsuite/
	* lib/torture-options.exp (torture-init-done): Add.
	* lib/gcc-dg.exp (gcc-dg-runtest): Use it to conditionalize
	implicit 'torture-init'.
	* lib/gfortran-dg.exp (gfortran-dg-runtest): Likewise.
	* lib/obj-c++-dg.exp (obj-c++-dg-runtest): Likewise.
	* lib/objc-dg.exp (objc-dg-runtest): Likewise.
---
 gcc/testsuite/lib/gcc-dg.exp          | 8 ++++++--
 gcc/testsuite/lib/gfortran-dg.exp     | 8 ++++++--
 gcc/testsuite/lib/obj-c++-dg.exp      | 8 ++++++--
 gcc/testsuite/lib/objc-dg.exp         | 8 ++++++--
 gcc/testsuite/lib/torture-options.exp | 6 ++++++
 5 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index 693cbdd7511..4ed4233efff 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -568,11 +568,15 @@ proc search_for { file pattern } {
 proc gcc-dg-runtest { testcases flags default-extra-flags } {
     global runtests
 
+    # Some callers initialize torture testing themselves; don't override those.
+    set existing_torture_init [torture-init-done]
+    if { $existing_torture_init == 0 } {
+	torture-init
+    }
     # Some callers set torture options themselves; don't override those.
     set existing_torture_options [torture-options-exist]
     if { $existing_torture_options == 0 } {
 	global DG_TORTURE_OPTIONS LTO_TORTURE_OPTIONS
-	torture-init
 	set-torture-options $DG_TORTURE_OPTIONS [list {}] $LTO_TORTURE_OPTIONS
     }
     dump-torture-options
@@ -603,7 +607,7 @@ proc gcc-dg-runtest { testcases flags default-extra-flags } {
 	}
     }
 
-    if { $existing_torture_options == 0 } {
+    if { $existing_torture_init == 0 } {
 	torture-finish
     }
 }
diff --git a/gcc/testsuite/lib/gfortran-dg.exp b/gcc/testsuite/lib/gfortran-dg.exp
index 3c813d3c8fb..e85f791fa94 100644
--- a/gcc/testsuite/lib/gfortran-dg.exp
+++ b/gcc/testsuite/lib/gfortran-dg.exp
@@ -126,11 +126,15 @@ proc gfortran-dg-runtest { testcases flags default-extra-flags } {
     global runtests
     global torture_with_loops
 
+    # Some callers initialize torture testing themselves; don't override those.
+    set existing_torture_init [torture-init-done]
+    if { $existing_torture_init == 0 } {
+	torture-init
+    }
     # Some callers set torture options themselves; don't override those.
     set existing_torture_options [torture-options-exist]
     if { $existing_torture_options == 0 } {
 	global DG_TORTURE_OPTIONS
-	torture-init
 	set-torture-options $DG_TORTURE_OPTIONS
     }
     dump-torture-options
@@ -160,7 +164,7 @@ proc gfortran-dg-runtest { testcases flags default-extra-flags } {
 	}
     }
 
-    if { $existing_torture_options == 0 } {
+    if { $existing_torture_init == 0 } {
 	torture-finish
     }
 }
diff --git a/gcc/testsuite/lib/obj-c++-dg.exp b/gcc/testsuite/lib/obj-c++-dg.exp
index 9123240f033..8deaed005dc 100644
--- a/gcc/testsuite/lib/obj-c++-dg.exp
+++ b/gcc/testsuite/lib/obj-c++-dg.exp
@@ -32,11 +32,15 @@ proc obj-c++-dg-prune { system text } {
 proc obj-c++-dg-runtest { testcases flags default-extra-flags } {
     global runtests
 
+    # Some callers initialize torture testing themselves; don't override those.
+    set existing_torture_init [torture-init-done]
+    if { $existing_torture_init == 0 } {
+	torture-init
+    }
     # Some callers set torture options themselves; don't override those.
     set existing_torture_options [torture-options-exist]
     if { $existing_torture_options == 0 } {
 	global DG_TORTURE_OPTIONS LTO_TORTURE_OPTIONS
-	torture-init
 	set-torture-options $DG_TORTURE_OPTIONS [list {}] $LTO_TORTURE_OPTIONS
     }
     dump-torture-options
@@ -67,7 +71,7 @@ proc obj-c++-dg-runtest { testcases flags default-extra-flags } {
 	}
     }
 
-    if { $existing_torture_options == 0 } {
+    if { $existing_torture_init == 0 } {
 	torture-finish
     }
 }
\ No newline at end of file
diff --git a/gcc/testsuite/lib/objc-dg.exp b/gcc/testsuite/lib/objc-dg.exp
index 9ca751c8f48..cf08cdcabbd 100644
--- a/gcc/testsuite/lib/objc-dg.exp
+++ b/gcc/testsuite/lib/objc-dg.exp
@@ -33,11 +33,15 @@ proc objc-dg-prune { system text } {
 proc objc-dg-runtest { testcases flags default-extra-flags } {
     global runtests
 
+    # Some callers initialize torture testing themselves; don't override those.
+    set existing_torture_init [torture-init-done]
+    if { $existing_torture_init == 0 } {
+	torture-init
+    }
     # Some callers set torture options themselves; don't override those.
     set existing_torture_options [torture-options-exist]
     if { $existing_torture_options == 0 } {
 	global DG_TORTURE_OPTIONS LTO_TORTURE_OPTIONS
-	torture-init
 	set-torture-options $DG_TORTURE_OPTIONS [list {}] $LTO_TORTURE_OPTIONS
     }
     dump-torture-options
@@ -68,7 +72,7 @@ proc objc-dg-runtest { testcases flags default-extra-flags } {
 	}
     }
 
-    if { $existing_torture_options == 0 } {
+    if { $existing_torture_init == 0 } {
 	torture-finish
     }
 }
diff --git a/gcc/testsuite/lib/torture-options.exp b/gcc/testsuite/lib/torture-options.exp
index 394418e9a02..d00d07e9378 100644
--- a/gcc/testsuite/lib/torture-options.exp
+++ b/gcc/testsuite/lib/torture-options.exp
@@ -51,6 +51,12 @@ proc torture-init { args } {
     }
 }
 
+# Return 1 if 'torture-init' has already been done, 0 otherwise.
+proc torture-init-done { args } {
+    global LTO_TORTURE_OPTIONS
+    return [info exists LTO_TORTURE_OPTIONS]
+}
+
 # Return 1 if torture options have already been set, 0 otherwise.
 proc torture-options-exist { args } {
     global torture_with_loops
-- 
2.34.1

Re: Testsuite: Add 'torture-init-done', and use it to conditionalize implicit 'torture-init' (was: Testsuite: Add missing 'torture-init'/'torture-finish' around 'LTO_TORTURE_OPTIONS' usage (was: Let each 'lto_init' determine the default 'LTO_OPTIONS', and 'torture-init' the 'LTO_TORTURE_OPTIONS'))

[Christophe Lyon]

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

Hi Thomas,


On Wed, 10 May 2023 at 09:52, Thomas Schwinge <thomas@codesourcery.com>
wrote:

> Hi Christophe!
>
> On 2023-05-09T21:14:07+0200, Christophe Lyon <christophe.lyon@linaro.org>
> wrote:
> > On Tue, 9 May 2023 at 17:17, Christophe Lyon <christophe.lyon@linaro.org
> >
> > wrote:
> >> On Tue, 9 May 2023 at 11:00, Thomas Schwinge <thomas@codesourcery.com>
> >> wrote:
> >>> On 2023-05-09T09:32:55+0200, Christophe Lyon <
> christophe.lyon@linaro.org>
> >>> wrote:
> >>> > 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:
> >>> >> > "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?
> >>>
> >>> Sorry for that -- it means, the safe-guards I added are working as
> >>> expected.
> >>>
> >>> Please test whether all these issues are gone with the attached
> >>> "Testsuite: Add missing 'torture-init'/'torture-finish' around
> >>> 'LTO_TORTURE_OPTIONS' usage"?
> >>
> >> Your patch seemed reasonable,  but it doesn't work :-(
> >>
> >> Well now I get:
> >> ERROR: torture-init: LTO_TORTURE_OPTIONS is not empty as expected
> >> because gcc-dg-runtest itself calls torture-init
> >>
> >> but I'm not sure where LTO_TORTURE_OPTIONS is set
> >
> > Just checking, are you able to test your changes on arm (a cross
> toolchain
> > is OK) ?
>
> Sorry, I don't currently have an arm/aarch64 toolchain built.
>
> > The problem shows up even if running only acle.exp, so it's quick once
> you
> > have built the toolchain once.
>
> I did a quick hack:
>
>     --- gcc/testsuite/gcc.target/aarch64/sls-mitigation/sls-mitigation.exp
>     +++ gcc/testsuite/gcc.target/aarch64/sls-mitigation/sls-mitigation.exp
>     @@ -22,3 +21,0 @@
>     -if {![istarget aarch64*-*-*] } then {
>     -  return
>     -}
>     --- gcc/testsuite/gcc.target/arm/acle/acle.exp
>     +++ gcc/testsuite/gcc.target/arm/acle/acle.exp
>     @@ -20,3 +19,0 @@
>     -if ![istarget arm*-*-*] then {
>     -  return
>     -}
>
> ..., and confirm to run into the DejaGnu/TCL ERRORs in my
> x86_64-pc-linux-gnu testing.
>
> > I spent some time looking at it, and the conflict is that the .exp file
> > calls torture-init and gcc-dg-runtest, which in turn calls torture-init
> > again, leading to the error.
>
> I see, thanks -- and sorry, once again.
>
> > I haven't checked the details of why there are similar failures on
> aarch64.
>
> I now understand that the problem is the following: most of all '*.exp'
> files have 'torture-init' followed by 'set-torture-options' before
> 'gcc-dg-runtest' etc., and therefore don't run into the latter's
> "Some callers set torture options themselves; don't override those."
> code.  Some '*.exp' files however do 'torture-init' but not
> 'set-torture-options', and therefore we can't any longer conditionalize
> the implicit 'torture-init' by '![torture-options-exist]'.
> Please in addition to the earlier
> "Testsuite: Add missing 'torture-init'/'torture-finish' around
> 'LTO_TORTURE_OPTIONS' usage"
> also apply the attached
> "Testsuite: Add 'torture-init-done', and use it to conditionalize implicit
> 'torture-init'".
> That hopefully should restore sanity -- if not, I'll get arm/aarch64
> toolchains built.
>
>
Thanks for the patch, it seems to work!

Christophe


>
> 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
>