Tobias Burnus <tob...@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
