| Message ID | oro78wd5de.fsf_-_@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | [testsuite] conditionalize dg-additional-sources on target and type | expand |
Hi Alexandre, On Thu, 23 May 2024 at 15:29, Alexandre Oliva <oliva@adacore.com> wrote: > > On Apr 30, 2024, Christophe Lyon <christophe.lyon@linaro.org> wrote: > > > On Tue, 30 Apr 2024 at 01:31, Alexandre Oliva <oliva@adacore.com> wrote: > >> >> for gcc/testsuite/ChangeLog > >> >> > >> >> * lib/target-supports.exp (check_vect_support_and_set_flags): > >> >> Decay to link rather than compile. > >> > >> Alas, linking may fail because of an incompatible libc, as reported by > >> Linaro with a link to their issue GNU-1206 (I'm not posting the link to > >> the fully-Javascrippled Jira web page; it shows nothing useful, and I > >> can't post feedback there) and to > >> https://ci.linaro.org/job/tcwg_gnu_embed_check_gcc--master-thumb_m7_hard_eabi-build/10/artifact/artifacts/00-sumfiles/ > >> (where I could get useful information) > >> > >> I'm reverting the patch, and I'll see about some alternate approach > > > Indeed, that's another instance of the tricky multilibs configuration issues. > > > - we run the tests with > > qemu/-mthumb/-march=armv7e-m+fp.dp/-mtune=cortex-m7/-mfloat-abi=hard/-mfpu=auto > > which matches the GCC configuration flags, > > but the vect.exp tests add -mfpu=neon -mfloat-abi=softfp -march=armv7-a > > and link fails because the toolchain does not support softfp libs > > Hello, Christophe, thanks for the info. > > I came up with an entirely different approach: > > > g++.dg/vect/pr95401.cc has dg-additional-sources, and that fails when > check_vect_support_and_set_flags finds vector support lacking for > execution tests: tests decay to compile tests, and additional sources > are rejected by the compiler when compiling to a named output file. > > At first I considered using some effective target to conditionalize > the additional sources. There was no support for target-specific > additional sources, so I added that. > > But then, I found that adding an effective target to check whether the > test involves linking would just make for busy work in this case, and > so I went ahead and adjusted the handling of additional sources to > refrain from adding them on compile tests, reporting them as > unsupported. > > That solves the problem without using the newly-added machinery for > per-target additional sources, but I figured since I'd implemented it > I might as well contribute it, since there might be other uses for it. Thanks for improving this, LGTM at quick glance, but I can't approve :-) Christophe > > Regstrapped on x86_64-linux-gnu. Also tested on ppc64-vx7r2 with > gcc-13. Ok to install? > > > for gcc/ChangeLog > > * doc/sourcebuild.texi (dg-additional-sources): Document > newly-added support for target selectors, and implicit discard > on non-linking tests that name the compiler output explicitly. > > for gcc/testsuite/ChangeLog > > * lib/gcc-defs.exp (dg-additional-sources): Support target > selectors. Make it cumulative. > (dg-additional-files-options): Take dest and type. Note > unsupported additional sources when not linking and naming the > compiler output. Adjust source dirname prepending to cope > with leading blanks. > * lib/g++.exp (g++_target_compile): Pass dest and type on to > dg-additional-files-options. > * lib/gcc.exp (gcc_target_compile): Likewise. > * lib/gdc.exp (gdb_target_compile): Likewise. > * lib/gfortran.exp (gfortran_target_compile): Likewise. > * lib/go.exp (go_target_compile): Likewise. > * lib/obj-c++.exp (obj-c++_target_compile): Likewise. > * lib/objc.exp (objc_target_compile): Likewise. > * lib/rust.exp (rust_target_compile): Likewise. > * lib/profopt.exp (profopt-execute): Likewise-ish. > --- > gcc/doc/sourcebuild.texi | 8 +++++++- > gcc/testsuite/lib/g++.exp | 2 +- > gcc/testsuite/lib/gcc-defs.exp | 35 ++++++++++++++++++++++++++++++----- > gcc/testsuite/lib/gcc.exp | 2 +- > gcc/testsuite/lib/gdc.exp | 2 +- > gcc/testsuite/lib/gfortran.exp | 2 +- > gcc/testsuite/lib/go.exp | 2 +- > gcc/testsuite/lib/obj-c++.exp | 2 +- > gcc/testsuite/lib/objc.exp | 2 +- > gcc/testsuite/lib/profopt.exp | 2 +- > gcc/testsuite/lib/rust.exp | 2 +- > 11 files changed, 46 insertions(+), 15 deletions(-) > > diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi > index 8e4e59ac44c74..e997dbec3334b 100644 > --- a/gcc/doc/sourcebuild.texi > +++ b/gcc/doc/sourcebuild.texi > @@ -1320,9 +1320,15 @@ to @var{var_value} before execution of the program created by the test. > Specify additional files, other than source files, that must be copied > to the system where the compiler runs. > > -@item @{ dg-additional-sources "@var{filelist}" @} > +@item @{ dg-additional-sources "@var{filelist}" [@{ target @var{selector} @}] @} > Specify additional source files to appear in the compile line > following the main test file. > +If the directive includes the optional @samp{@{ @var{selector} @}} > +then the additional sources are only added if the target system > +matches the @var{selector}. > +Additional sources are generally used only in @samp{link} and @samp{run} > +tests; they are reported as unsupported and discarded in other kinds of > +tests that direct the compiler to output to a single file. > @end table > > @subsubsection Add checks at the end of a test > diff --git a/gcc/testsuite/lib/g++.exp b/gcc/testsuite/lib/g++.exp > index 0e47769c25b8b..a6b34d5d3a2b7 100644 > --- a/gcc/testsuite/lib/g++.exp > +++ b/gcc/testsuite/lib/g++.exp > @@ -326,7 +326,7 @@ proc g++_target_compile { source dest type options } { > append board_info($tboard,multilib_flags) " $flags_to_postpone" > } > > - set options [dg-additional-files-options $options $source] > + set options [dg-additional-files-options $options $source $dest $type] > > if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { > lappend options "libs=${gluefile}" > diff --git a/gcc/testsuite/lib/gcc-defs.exp b/gcc/testsuite/lib/gcc-defs.exp > index 70215ed49052e..cdca4c254d6ec 100644 > --- a/gcc/testsuite/lib/gcc-defs.exp > +++ b/gcc/testsuite/lib/gcc-defs.exp > @@ -307,7 +307,22 @@ set additional_sources_used "" > > proc dg-additional-sources { args } { > global additional_sources > - set additional_sources [lindex $args 1] > + > + if { [llength $args] > 3 } { > + error "[lindex $args 0]: too many arguments" > + return > + } > + > + if { [llength $args] >= 3 } { > + switch [dg-process-target [lindex $args 2]] { > + "S" { append additional_sources " [lindex $args 1]" } > + "N" { } > + "F" { error "[lindex $args 0]: `xfail' not allowed here" } > + "P" { error "[lindex $args 0]: `xfail' not allowed here" } > + } > + } else { > + append additional_sources " [lindex $args 1]" > + } > } > > # Record additional files -- other than source files -- that must be > @@ -383,20 +398,30 @@ proc gcc_adjust_linker_flags {} { > > # Return an updated version of OPTIONS that mentions any additional > # source files registered with dg-additional-sources. SOURCE is the > -# name of the test case. > +# name of the test case. If DEST is given and TYPE does not require > +# linking, additional sources are noted as unsupported rather than > +# added, because the compiler rejects a single output for multiple > +# sources. > > -proc dg-additional-files-options { options source } { > +proc dg-additional-files-options { options source dest type } { > gcc_adjust_linker_flags > > global additional_sources > global additional_sources_used > global additional_files > set to_download [list] > - if { $additional_sources != "" } then { > + if { $additional_sources == "" } then { > + } elseif { $type != "executable" && $dest != "" } then { > + foreach s $additional_sources { > + unsupported "$s: additional-source will not be used to build $dest" > + } > + set additional_sources_used "" > + set additional_sources "" > + } else { > if [is_remote host] { > lappend options "additional_flags=$additional_sources" > } > - regsub -all "^| " $additional_sources " [file dirname $source]/" additional_sources > + regsub -all {^ *| *} $additional_sources " [file dirname $source]/" additional_sources > if ![is_remote host] { > lappend options "additional_flags=$additional_sources" > } > diff --git a/gcc/testsuite/lib/gcc.exp b/gcc/testsuite/lib/gcc.exp > index 63e61c3c9fd9c..3bcd98eb9a7ba 100644 > --- a/gcc/testsuite/lib/gcc.exp > +++ b/gcc/testsuite/lib/gcc.exp > @@ -159,7 +159,7 @@ proc gcc_target_compile { source dest type options } { > > lappend options "timeout=[timeout_value]" > lappend options "compiler=$GCC_UNDER_TEST" > - set options [dg-additional-files-options $options $source] > + set options [dg-additional-files-options $options $source $dest $type] > > if {[target_info needs_status_wrapper] != "" && \ > [target_info needs_status_wrapper] != "0" && \ > diff --git a/gcc/testsuite/lib/gdc.exp b/gcc/testsuite/lib/gdc.exp > index 3c284229609a9..fd9cea67d0a9f 100644 > --- a/gcc/testsuite/lib/gdc.exp > +++ b/gcc/testsuite/lib/gdc.exp > @@ -327,6 +327,6 @@ proc gdc_target_compile { source dest type options } { > lappend options "compiler=$GDC_UNDER_TEST" > > set options [concat "$always_dflags" $options] > - set options [dg-additional-files-options $options $source] > + set options [dg-additional-files-options $options $source $dest $type] > return [target_compile $source $dest $type $options] > } > diff --git a/gcc/testsuite/lib/gfortran.exp b/gcc/testsuite/lib/gfortran.exp > index 1ccb81ccec5a8..3a9e81b69a3d1 100644 > --- a/gcc/testsuite/lib/gfortran.exp > +++ b/gcc/testsuite/lib/gfortran.exp > @@ -295,7 +295,7 @@ proc gfortran_target_compile { source dest type options } { > lappend options "timeout=[timeout_value]" > > set options [concat "$ALWAYS_GFORTRANFLAGS" $options] > - set options [dg-additional-files-options $options $source] > + set options [dg-additional-files-options $options $source $dest $type] > set return_val [target_compile $source $dest $type $options] > > if {[board_info $tboard exists multilib_flags]} { > diff --git a/gcc/testsuite/lib/go.exp b/gcc/testsuite/lib/go.exp > index 714afb173d8e0..509d528d861fc 100644 > --- a/gcc/testsuite/lib/go.exp > +++ b/gcc/testsuite/lib/go.exp > @@ -221,6 +221,6 @@ proc go_target_compile { source dest type options } { > lappend options "compiler=$GOC_UNDER_TEST" > > set options [concat "$ALWAYS_GOCFLAGS" $options] > - set options [dg-additional-files-options $options $source] > + set options [dg-additional-files-options $options $source $dest $type] > return [target_compile $source $dest $type $options] > } > diff --git a/gcc/testsuite/lib/obj-c++.exp b/gcc/testsuite/lib/obj-c++.exp > index 854dc264f9d0e..1c5ef57ce8370 100644 > --- a/gcc/testsuite/lib/obj-c++.exp > +++ b/gcc/testsuite/lib/obj-c++.exp > @@ -388,7 +388,7 @@ proc obj-c++_target_compile { source dest type options } { > > set options [concat "$ALWAYS_OBJCXXFLAGS" $options]; > > - set options [dg-additional-files-options $options $source] > + set options [dg-additional-files-options $options $source $dest $type] > > set result [target_compile $source $dest $type $options] > > diff --git a/gcc/testsuite/lib/objc.exp b/gcc/testsuite/lib/objc.exp > index b02fd77289c45..3d2cc05c15d11 100644 > --- a/gcc/testsuite/lib/objc.exp > +++ b/gcc/testsuite/lib/objc.exp > @@ -248,7 +248,7 @@ proc objc_target_compile { source dest type options } { > > set_ld_library_path_env_vars > > - set options [dg-additional-files-options $options $source] > + set options [dg-additional-files-options $options $source $dest $type] > > return [target_compile $source $dest $type $options] > } > diff --git a/gcc/testsuite/lib/profopt.exp b/gcc/testsuite/lib/profopt.exp > index 6524d95f69d5a..6d7159b7a8691 100644 > --- a/gcc/testsuite/lib/profopt.exp > +++ b/gcc/testsuite/lib/profopt.exp > @@ -388,7 +388,7 @@ proc profopt-execute { src } { > # schedule removal of dump files et al > # Do this before the call below destroys additional_sources.. > append use_final_code [schedule-cleanups "$option $extra_flags"] > - set extra_options [dg-additional-files-options "" "$src"] > + set extra_options [dg-additional-files-options "" "$src" $execname1 "executable"] > > # Remove old profiling data files. Make sure additional_sources_used is > # valid, by running it after dg-additional-files-options. > diff --git a/gcc/testsuite/lib/rust.exp b/gcc/testsuite/lib/rust.exp > index 5ded0edf91464..4c296228fa2af 100644 > --- a/gcc/testsuite/lib/rust.exp > +++ b/gcc/testsuite/lib/rust.exp > @@ -182,7 +182,7 @@ proc rust_target_compile { source dest type options } { > lappend options "compiler=$RUST_UNDER_TEST" > > set options [concat "$ALWAYS_RUSTFLAGS" $options] > - set options [dg-additional-files-options $options $source] > + set options [dg-additional-files-options $options $source $dest $type] > > return [target_compile $source $dest $type $options] > } > > > -- > Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/ > Free Software Activist GNU Toolchain Engineer > More tolerance and less prejudice are key for inclusion and diversity > Excluding neuro-others for not behaving ""normal"" is *not* inclusive
On May 23, 2024, at 6:28 AM, Alexandre Oliva <oliva@adacore.com> wrote; > I came up with an entirely different approach: > > > g++.dg/vect/pr95401.cc has dg-additional-sources, and that fails when > check_vect_support_and_set_flags finds vector support lacking for > execution tests: tests decay to compile tests, and additional sources > are rejected by the compiler when compiling to a named output file. > > At first I considered using some effective target to conditionalize > the additional sources. There was no support for target-specific > additional sources, so I added that. > > But then, I found that adding an effective target to check whether the > test involves linking would just make for busy work in this case, and > so I went ahead and adjusted the handling of additional sources to > refrain from adding them on compile tests, reporting them as > unsupported. > > That solves the problem without using the newly-added machinery for > per-target additional sources, but I figured since I'd implemented it > I might as well contribute it, since there might be other uses for it. > > Regstrapped on x86_64-linux-gnu. Also tested on ppc64-vx7r2 with > gcc-13. Ok to install? Ok.
diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi index 8e4e59ac44c74..e997dbec3334b 100644 --- a/gcc/doc/sourcebuild.texi +++ b/gcc/doc/sourcebuild.texi @@ -1320,9 +1320,15 @@ to @var{var_value} before execution of the program created by the test. Specify additional files, other than source files, that must be copied to the system where the compiler runs. -@item @{ dg-additional-sources "@var{filelist}" @} +@item @{ dg-additional-sources "@var{filelist}" [@{ target @var{selector} @}] @} Specify additional source files to appear in the compile line following the main test file. +If the directive includes the optional @samp{@{ @var{selector} @}} +then the additional sources are only added if the target system +matches the @var{selector}. +Additional sources are generally used only in @samp{link} and @samp{run} +tests; they are reported as unsupported and discarded in other kinds of +tests that direct the compiler to output to a single file. @end table @subsubsection Add checks at the end of a test diff --git a/gcc/testsuite/lib/g++.exp b/gcc/testsuite/lib/g++.exp index 0e47769c25b8b..a6b34d5d3a2b7 100644 --- a/gcc/testsuite/lib/g++.exp +++ b/gcc/testsuite/lib/g++.exp @@ -326,7 +326,7 @@ proc g++_target_compile { source dest type options } { append board_info($tboard,multilib_flags) " $flags_to_postpone" } - set options [dg-additional-files-options $options $source] + set options [dg-additional-files-options $options $source $dest $type] if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { lappend options "libs=${gluefile}" diff --git a/gcc/testsuite/lib/gcc-defs.exp b/gcc/testsuite/lib/gcc-defs.exp index 70215ed49052e..cdca4c254d6ec 100644 --- a/gcc/testsuite/lib/gcc-defs.exp +++ b/gcc/testsuite/lib/gcc-defs.exp @@ -307,7 +307,22 @@ set additional_sources_used "" proc dg-additional-sources { args } { global additional_sources - set additional_sources [lindex $args 1] + + if { [llength $args] > 3 } { + error "[lindex $args 0]: too many arguments" + return + } + + if { [llength $args] >= 3 } { + switch [dg-process-target [lindex $args 2]] { + "S" { append additional_sources " [lindex $args 1]" } + "N" { } + "F" { error "[lindex $args 0]: `xfail' not allowed here" } + "P" { error "[lindex $args 0]: `xfail' not allowed here" } + } + } else { + append additional_sources " [lindex $args 1]" + } } # Record additional files -- other than source files -- that must be @@ -383,20 +398,30 @@ proc gcc_adjust_linker_flags {} { # Return an updated version of OPTIONS that mentions any additional # source files registered with dg-additional-sources. SOURCE is the -# name of the test case. +# name of the test case. If DEST is given and TYPE does not require +# linking, additional sources are noted as unsupported rather than +# added, because the compiler rejects a single output for multiple +# sources. -proc dg-additional-files-options { options source } { +proc dg-additional-files-options { options source dest type } { gcc_adjust_linker_flags global additional_sources global additional_sources_used global additional_files set to_download [list] - if { $additional_sources != "" } then { + if { $additional_sources == "" } then { + } elseif { $type != "executable" && $dest != "" } then { + foreach s $additional_sources { + unsupported "$s: additional-source will not be used to build $dest" + } + set additional_sources_used "" + set additional_sources "" + } else { if [is_remote host] { lappend options "additional_flags=$additional_sources" } - regsub -all "^| " $additional_sources " [file dirname $source]/" additional_sources + regsub -all {^ *| *} $additional_sources " [file dirname $source]/" additional_sources if ![is_remote host] { lappend options "additional_flags=$additional_sources" } diff --git a/gcc/testsuite/lib/gcc.exp b/gcc/testsuite/lib/gcc.exp index 63e61c3c9fd9c..3bcd98eb9a7ba 100644 --- a/gcc/testsuite/lib/gcc.exp +++ b/gcc/testsuite/lib/gcc.exp @@ -159,7 +159,7 @@ proc gcc_target_compile { source dest type options } { lappend options "timeout=[timeout_value]" lappend options "compiler=$GCC_UNDER_TEST" - set options [dg-additional-files-options $options $source] + set options [dg-additional-files-options $options $source $dest $type] if {[target_info needs_status_wrapper] != "" && \ [target_info needs_status_wrapper] != "0" && \ diff --git a/gcc/testsuite/lib/gdc.exp b/gcc/testsuite/lib/gdc.exp index 3c284229609a9..fd9cea67d0a9f 100644 --- a/gcc/testsuite/lib/gdc.exp +++ b/gcc/testsuite/lib/gdc.exp @@ -327,6 +327,6 @@ proc gdc_target_compile { source dest type options } { lappend options "compiler=$GDC_UNDER_TEST" set options [concat "$always_dflags" $options] - set options [dg-additional-files-options $options $source] + set options [dg-additional-files-options $options $source $dest $type] return [target_compile $source $dest $type $options] } diff --git a/gcc/testsuite/lib/gfortran.exp b/gcc/testsuite/lib/gfortran.exp index 1ccb81ccec5a8..3a9e81b69a3d1 100644 --- a/gcc/testsuite/lib/gfortran.exp +++ b/gcc/testsuite/lib/gfortran.exp @@ -295,7 +295,7 @@ proc gfortran_target_compile { source dest type options } { lappend options "timeout=[timeout_value]" set options [concat "$ALWAYS_GFORTRANFLAGS" $options] - set options [dg-additional-files-options $options $source] + set options [dg-additional-files-options $options $source $dest $type] set return_val [target_compile $source $dest $type $options] if {[board_info $tboard exists multilib_flags]} { diff --git a/gcc/testsuite/lib/go.exp b/gcc/testsuite/lib/go.exp index 714afb173d8e0..509d528d861fc 100644 --- a/gcc/testsuite/lib/go.exp +++ b/gcc/testsuite/lib/go.exp @@ -221,6 +221,6 @@ proc go_target_compile { source dest type options } { lappend options "compiler=$GOC_UNDER_TEST" set options [concat "$ALWAYS_GOCFLAGS" $options] - set options [dg-additional-files-options $options $source] + set options [dg-additional-files-options $options $source $dest $type] return [target_compile $source $dest $type $options] } diff --git a/gcc/testsuite/lib/obj-c++.exp b/gcc/testsuite/lib/obj-c++.exp index 854dc264f9d0e..1c5ef57ce8370 100644 --- a/gcc/testsuite/lib/obj-c++.exp +++ b/gcc/testsuite/lib/obj-c++.exp @@ -388,7 +388,7 @@ proc obj-c++_target_compile { source dest type options } { set options [concat "$ALWAYS_OBJCXXFLAGS" $options]; - set options [dg-additional-files-options $options $source] + set options [dg-additional-files-options $options $source $dest $type] set result [target_compile $source $dest $type $options] diff --git a/gcc/testsuite/lib/objc.exp b/gcc/testsuite/lib/objc.exp index b02fd77289c45..3d2cc05c15d11 100644 --- a/gcc/testsuite/lib/objc.exp +++ b/gcc/testsuite/lib/objc.exp @@ -248,7 +248,7 @@ proc objc_target_compile { source dest type options } { set_ld_library_path_env_vars - set options [dg-additional-files-options $options $source] + set options [dg-additional-files-options $options $source $dest $type] return [target_compile $source $dest $type $options] } diff --git a/gcc/testsuite/lib/profopt.exp b/gcc/testsuite/lib/profopt.exp index 6524d95f69d5a..6d7159b7a8691 100644 --- a/gcc/testsuite/lib/profopt.exp +++ b/gcc/testsuite/lib/profopt.exp @@ -388,7 +388,7 @@ proc profopt-execute { src } { # schedule removal of dump files et al # Do this before the call below destroys additional_sources.. append use_final_code [schedule-cleanups "$option $extra_flags"] - set extra_options [dg-additional-files-options "" "$src"] + set extra_options [dg-additional-files-options "" "$src" $execname1 "executable"] # Remove old profiling data files. Make sure additional_sources_used is # valid, by running it after dg-additional-files-options. diff --git a/gcc/testsuite/lib/rust.exp b/gcc/testsuite/lib/rust.exp index 5ded0edf91464..4c296228fa2af 100644 --- a/gcc/testsuite/lib/rust.exp +++ b/gcc/testsuite/lib/rust.exp @@ -182,7 +182,7 @@ proc rust_target_compile { source dest type options } { lappend options "compiler=$RUST_UNDER_TEST" set options [concat "$ALWAYS_RUSTFLAGS" $options] - set options [dg-additional-files-options $options $source] + set options [dg-additional-files-options $options $source $dest $type] return [target_compile $source $dest $type $options] }