Hi Alexandre,

On Thu, 23 May 2024 at 15:29, Alexandre Oliva <ol...@adacore.com> wrote:
>
> On Apr 30, 2024, Christophe Lyon <christophe.l...@linaro.org> wrote:
>
> > On Tue, 30 Apr 2024 at 01:31, Alexandre Oliva <ol...@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

Reply via email to