| Message ID | c27d7197-22e4-ac32-80de-5cec6dcd61ae@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Testsuite] – More fixes for remote execution: check_gc_sections_available, … | expand |
Tobias Burnus <tobias@codesourcery.com> writes: > Still pending: libgomp-Testsuite patch https://gcc.gnu.org/ml/gcc-patches/2020-02/msg00207.html > > This is the same fix – but for gcc/testsuite/. > > To illustrate the problem again. Using remote testing > (here: modified target_compile, but DejaGNU's default_target_compile is likewise), > and using check_gc_sections_available, I get the following result without the fix: > > call_remote download <hostname> -print-prog-name=ld -print-prog-name=ld > Invoking the compiler as powerpc64le-none-linux-gnu-gcc … /scratch/…/default/gcc.d/-print-prog-name=ld > … > UNSUPPORTED: gcc.dg/special/gcsec-1.c > > Which obvious fails both for uploading the flag as file and for compiling the > flag as filename. > > > WITH the patch, I get the expected: > > Invoking the compiler as powerpc64le-none-linux-gnu-gcc … -fdiagnostics-urls=never -print-prog-name=ld -I . > … > PASS: gcc.dg/special/gcsec-1.c (test for excess errors) > PASS: gcc.dg/special/gcsec-1.c execution test > > I tested the attached patch for check_gc_sections_available thoroughly > – esp. via RUNTESTFLAGS="-v -v -v -v -v special.exp=gcsec-1.c" > w/ and w/o remote but also using, intermittently, some debugging "puts". > > I also fixed check_effective_target_gld, check_effective_target_gas, check_runtime, > check_multi_dir, but tested them only lightly. (The first two are unused, the > others are only used by mips and arm.) > Regarding the '{ }' see below – or in the linked other patch. Without, only one > argument is actually passed. > > OK for the trunk? > > Tobias > > On 2/4/20 4:49 PM, Tobias Burnus wrote: >> DejaGNU uses in lib/target.exp's >> proc default_target_compile {source destfile type options} >> uses the following. >> >> When running locally, $source is simply used >> as argument. However, when run remotely, it is tried to be uploaded >> on the remote host – and otherwise skipped. That's fine if the >> argument is an actual file – but not if it is a flag. >> >> Hence, flags should be passed as $options not as $source. >> Quoting now from DejaGNU's default_target_compile#: >> >> set sources "" >> if {[isremote host]} { >> foreach x $source { >> set file [remote_download host $x] >> if { $file eq "" } { >> warning "Unable to download $x to host." >> return "Unable to download $x to host." >> } else { >> append sources " $file" >> } >> } >> } else { >> set sources $source >> } >> >> * * * >> >> FIRST, I think that affects the following calls, but I have not >> checked. — I assume that moving it from the first to the last >> argument should work and fix the "isremote" issue. >> >> gcc/testsuite/gcc.target/arm/multilib.exp: set gcc_output >> [${tool}_target_compile "--print-multi-directory $gcc_opts" "" "none" ""] >> gcc/testsuite/lib/target-supports.exp: set gcc_output >> [${tool}_target_compile "-v" "" "none" ""] >> gcc/testsuite/lib/target-supports.exp: set gcc_ld [lindex >> [${tool}_target_compile "-print-prog-name=ld" "" "none" ""] 0] >> gcc/testsuite/lib/target-supports.exp: set gcc_as [lindex >> [${tool}_target_compile "-print-prog-name=as" "" "none" ""] 0] >> gcc/testsuite/lib/target-supports.exp: set gcc_ld [lindex >> [${tool}_target_compile "-print-prog-name=ld" "" "none" ""] 0] >> >> TODO: One should probably change this. >> >> >> SECONDLY: I hit a very similar issue in libgomp, which I actually did >> debug. >> >> In check_effective_target_offload_target_nvptx, one has something >> similar, namely: >> set gcc_output [libgomp_target_compile "-v $options" "" "none" ""] >> This currently tries (w/o success) to upload the flags to the remote >> host and then >> fails as the required "-v" flag fails (i.e. no input). >> >> However, using "-v" as options argument does not work as seemingly >> only additional_flags= >> arguments are passed on. Additionally, if one does not only want to >> pass on the first >> argument, curly braces are needed. >> >> PATCH: The attached patch does this – and have successfully tested it >> both with local >> runs and with remote runs. (RUNTESTFLAGS="-v -v -v" did help a lot for >> studying the >> behavior in both cases.) >> >> OK for the trunk? >> >> Cheers, >> >> Tobias > > gcc/testsuite/ > * gcc.target/arm/multilib.exp (multilib_config): Pass flags to > …_target_compile as (additional_flags=) option and not as source > filename to make it work with remote execution. > * lib/target-supports.exp (check_runtime, check_gc_sections_available, > check_effective_target_gas, check_effective_target_gld): Likewise. > > diff --git a/gcc/testsuite/gcc.target/arm/multilib.exp b/gcc/testsuite/gcc.target/arm/multilib.exp > index 67d00266f6b..60b9edeebd0 100644 > --- a/gcc/testsuite/gcc.target/arm/multilib.exp > +++ b/gcc/testsuite/gcc.target/arm/multilib.exp > @@ -40,7 +40,7 @@ proc multilib_config {profile} { > proc check_multi_dir { gcc_opts multi_dir } { > global tool > > - set gcc_output [${tool}_target_compile "--print-multi-directory $gcc_opts" "" "none" ""] > + set gcc_output [${tool}_target_compile "" "" "none" "{additional_flags=--print-multi-directory $gcc_opts}"] > if { [string match "$multi_dir\n" $gcc_output] } { > pass "multilibdir $gcc_opts $multi_dir" > } else { > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > index 5377d7b11cb..5a18dbd85ea 100644 > --- a/gcc/testsuite/lib/target-supports.exp > +++ b/gcc/testsuite/lib/target-supports.exp > @@ -260,7 +260,7 @@ proc check_runtime {prop args} { > proc check_configured_with { pattern } { > global tool > > - set gcc_output [${tool}_target_compile "-v" "" "none" ""] > + set gcc_output [${tool}_target_compile "" "" "none" "additional_flags=-v"] > if { [ regexp "Configured with: \[^\n\]*$pattern" $gcc_output ] } { > verbose "Matched: $pattern" 2 > return 1 > @@ -504,7 +504,7 @@ proc check_gc_sections_available { } { > } > > # Check if the ld used by gcc supports --gc-sections. > - set gcc_ld [lindex [${tool}_target_compile "-print-prog-name=ld" "" "none" ""] 0] > + set gcc_ld [lindex [${tool}_target_compile "" "" "none" "additional_flags=-print-prog-name=ld"] 0] > set ld_output [remote_exec host "$gcc_ld" "--help"] > if { [ string first "--gc-sections" $ld_output ] >= 0 } { > return 1 > @@ -8566,7 +8566,7 @@ proc check_effective_target_gas { } { > > if {![info exists use_gas_saved]} { > # Check if the as used by gcc is GNU as. > - set gcc_as [lindex [${tool}_target_compile "-print-prog-name=as" "" "none" ""] 0] > + set gcc_as [lindex [${tool}_target_compile "" "" "none" "additional_flags=-print-prog-name=as"] 0] > # Provide /dev/null as input, otherwise gas times out reading from > # stdin. > set status [remote_exec host "$gcc_as" "-v /dev/null"] > @@ -8588,7 +8588,7 @@ proc check_effective_target_gld { } { > > if {![info exists use_gld_saved]} { > # Check if the ld used by gcc is GNU ld. > - set gcc_ld [lindex [${tool}_target_compile "-print-prog-name=ld" "" "none" ""] 0] > + set gcc_ld [lindex [${tool}_target_compile "" "" "none" "additional_flags=-print-prog-name=ld"] 0] > set status [remote_exec host "$gcc_ld" "--version"] > set ld_output [lindex $status 1] > if { [ string first "GNU" $ld_output ] >= 0 } { In each case it might be more obvious to use: [list "additional_flags=..."] with no explicit { ... } quoting. The .exp files are supposed to follow the 80 char limit where possible, so there should probably be a line break before [list ...]. OK with those changes, thanks. Richard
Hi Richard, On 2/5/20 5:36 PM, Richard Sandiford wrote: > In each case it might be more obvious to use: > [list "additional_flags=..."] > with no explicit { ... } quoting. > > The .exp files are supposed to follow the 80 char limit where possible, > so there should probably be a line break before [list ...]. > > OK with those changes, thanks. I concur that the "[list …" looks nicer. However, the line break does not work: I tried: # Check if the ld used by gcc supports --gc-sections. - set gcc_ld [lindex [${tool}_target_compile "-print-prog-name=ld" "" "none" ""] 0] + set gcc_ld [lindex [${tool}_target_compile "" "" "none" + [list "additional_flags=-print-prog-name=ld"]] 0] And that failed with: …/gcsec-1.c: wrong # args: should be "gcc_target_compile source dest type options" for " dg-require-gc-sections 4 "" " (Trying an hello-world example in tclsh, I can also reproduce it there.) Hence, I have installed the following, which uses the variable $optional for the value. In check_multi_dir, one line is still 87 characters, but that does not help. (At least: It is 5 characters shorter as before.) r10-6475-g101baaee42afe05c3d271925e4d40f0f8f642bd5 Tobias
gcc/testsuite/ * gcc.target/arm/multilib.exp (multilib_config): Pass flags to …_target_compile as (additional_flags=) option and not as source filename to make it work with remote execution. * lib/target-supports.exp (check_runtime, check_gc_sections_available, check_effective_target_gas, check_effective_target_gld): Likewise. diff --git a/gcc/testsuite/gcc.target/arm/multilib.exp b/gcc/testsuite/gcc.target/arm/multilib.exp index 67d00266f6b..60b9edeebd0 100644 --- a/gcc/testsuite/gcc.target/arm/multilib.exp +++ b/gcc/testsuite/gcc.target/arm/multilib.exp @@ -40,7 +40,7 @@ proc multilib_config {profile} { proc check_multi_dir { gcc_opts multi_dir } { global tool - set gcc_output [${tool}_target_compile "--print-multi-directory $gcc_opts" "" "none" ""] + set gcc_output [${tool}_target_compile "" "" "none" "{additional_flags=--print-multi-directory $gcc_opts}"] if { [string match "$multi_dir\n" $gcc_output] } { pass "multilibdir $gcc_opts $multi_dir" } else { diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 5377d7b11cb..5a18dbd85ea 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -260,7 +260,7 @@ proc check_runtime {prop args} { proc check_configured_with { pattern } { global tool - set gcc_output [${tool}_target_compile "-v" "" "none" ""] + set gcc_output [${tool}_target_compile "" "" "none" "additional_flags=-v"] if { [ regexp "Configured with: \[^\n\]*$pattern" $gcc_output ] } { verbose "Matched: $pattern" 2 return 1 @@ -504,7 +504,7 @@ proc check_gc_sections_available { } { } # Check if the ld used by gcc supports --gc-sections. - set gcc_ld [lindex [${tool}_target_compile "-print-prog-name=ld" "" "none" ""] 0] + set gcc_ld [lindex [${tool}_target_compile "" "" "none" "additional_flags=-print-prog-name=ld"] 0] set ld_output [remote_exec host "$gcc_ld" "--help"] if { [ string first "--gc-sections" $ld_output ] >= 0 } { return 1 @@ -8566,7 +8566,7 @@ proc check_effective_target_gas { } { if {![info exists use_gas_saved]} { # Check if the as used by gcc is GNU as. - set gcc_as [lindex [${tool}_target_compile "-print-prog-name=as" "" "none" ""] 0] + set gcc_as [lindex [${tool}_target_compile "" "" "none" "additional_flags=-print-prog-name=as"] 0] # Provide /dev/null as input, otherwise gas times out reading from # stdin. set status [remote_exec host "$gcc_as" "-v /dev/null"] @@ -8588,7 +8588,7 @@ proc check_effective_target_gld { } { if {![info exists use_gld_saved]} { # Check if the ld used by gcc is GNU ld. - set gcc_ld [lindex [${tool}_target_compile "-print-prog-name=ld" "" "none" ""] 0] + set gcc_ld [lindex [${tool}_target_compile "" "" "none" "additional_flags=-print-prog-name=ld"] 0] set status [remote_exec host "$gcc_ld" "--version"] set ld_output [lindex $status 1] if { [ string first "GNU" $ld_output ] >= 0 } {
Still pending: libgomp-Testsuite patch https://gcc.gnu.org/ml/gcc-patches/2020-02/msg00207.html This is the same fix – but for gcc/testsuite/. To illustrate the problem again. Using remote testing (here: modified target_compile, but DejaGNU's default_target_compile is likewise), and using check_gc_sections_available, I get the following result without the fix: call_remote download <hostname> -print-prog-name=ld -print-prog-name=ld Invoking the compiler as powerpc64le-none-linux-gnu-gcc … /scratch/…/default/gcc.d/-print-prog-name=ld … UNSUPPORTED: gcc.dg/special/gcsec-1.c Which obvious fails both for uploading the flag as file and for compiling the flag as filename. WITH the patch, I get the expected: Invoking the compiler as powerpc64le-none-linux-gnu-gcc … -fdiagnostics-urls=never -print-prog-name=ld -I . … PASS: gcc.dg/special/gcsec-1.c (test for excess errors) PASS: gcc.dg/special/gcsec-1.c execution test I tested the attached patch for check_gc_sections_available thoroughly – esp. via RUNTESTFLAGS="-v -v -v -v -v special.exp=gcsec-1.c" w/ and w/o remote but also using, intermittently, some debugging "puts". I also fixed check_effective_target_gld, check_effective_target_gas, check_runtime, check_multi_dir, but tested them only lightly. (The first two are unused, the others are only used by mips and arm.) Regarding the '{ }' see below – or in the linked other patch. Without, only one argument is actually passed. OK for the trunk? Tobias On 2/4/20 4:49 PM, Tobias Burnus wrote: > DejaGNU uses in lib/target.exp's > proc default_target_compile {source destfile type options} > uses the following. > > When running locally, $source is simply used > as argument. However, when run remotely, it is tried to be uploaded > on the remote host – and otherwise skipped. That's fine if the > argument is an actual file – but not if it is a flag. > > Hence, flags should be passed as $options not as $source. > Quoting now from DejaGNU's default_target_compile#: > > set sources "" > if {[isremote host]} { > foreach x $source { > set file [remote_download host $x] > if { $file eq "" } { > warning "Unable to download $x to host." > return "Unable to download $x to host." > } else { > append sources " $file" > } > } > } else { > set sources $source > } > > * * * > > FIRST, I think that affects the following calls, but I have not > checked. — I assume that moving it from the first to the last > argument should work and fix the "isremote" issue. > > gcc/testsuite/gcc.target/arm/multilib.exp: set gcc_output > [${tool}_target_compile "--print-multi-directory $gcc_opts" "" "none" ""] > gcc/testsuite/lib/target-supports.exp: set gcc_output > [${tool}_target_compile "-v" "" "none" ""] > gcc/testsuite/lib/target-supports.exp: set gcc_ld [lindex > [${tool}_target_compile "-print-prog-name=ld" "" "none" ""] 0] > gcc/testsuite/lib/target-supports.exp: set gcc_as [lindex > [${tool}_target_compile "-print-prog-name=as" "" "none" ""] 0] > gcc/testsuite/lib/target-supports.exp: set gcc_ld [lindex > [${tool}_target_compile "-print-prog-name=ld" "" "none" ""] 0] > > TODO: One should probably change this. > > > SECONDLY: I hit a very similar issue in libgomp, which I actually did > debug. > > In check_effective_target_offload_target_nvptx, one has something > similar, namely: > set gcc_output [libgomp_target_compile "-v $options" "" "none" ""] > This currently tries (w/o success) to upload the flags to the remote > host and then > fails as the required "-v" flag fails (i.e. no input). > > However, using "-v" as options argument does not work as seemingly > only additional_flags= > arguments are passed on. Additionally, if one does not only want to > pass on the first > argument, curly braces are needed. > > PATCH: The attached patch does this – and have successfully tested it > both with local > runs and with remote runs. (RUNTESTFLAGS="-v -v -v" did help a lot for > studying the > behavior in both cases.) > > OK for the trunk? > > Cheers, > > Tobias