public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch][Testsuite][libgomp] – Fix check_effective_target_offload_target_nvptx for remote execution
@ 2020-02-04 15:49 Tobias Burnus
  2020-02-05  9:17 ` [Patch][Testsuite] – More fixes for remote execution: check_gc_sections_available, … Tobias Burnus
  2020-02-05 16:31 ` [Patch][Testsuite][libgomp] – Fix check_effective_target_offload_target_nvptx for remote execution Richard Sandiford
  0 siblings, 2 replies; 5+ messages in thread
From: Tobias Burnus @ 2020-02-04 15:49 UTC (permalink / raw)
  To: gcc-patches, Rainer Orth, Mike Stump, Jakub Jelinek

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

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


[-- Attachment #2: fix-libgomp.diff --]
[-- Type: text/x-patch, Size: 1297 bytes --]

	libgomp/
	* testsuite/lib/libgomp.exp
	(check_effective_target_offload_target_nvptx): Pass flags as 'options'
	and not as 'source' argument to libgomp_target_compile.

diff --git a/libgomp/testsuite/lib/libgomp.exp b/libgomp/testsuite/lib/libgomp.exp
index 7e94527c7ca..cb7757b6a91 100644
--- a/libgomp/testsuite/lib/libgomp.exp
+++ b/libgomp/testsuite/lib/libgomp.exp
@@ -346,11 +346,11 @@ proc check_effective_target_offload_target_nvptx { } {
     # files; in particular, '-foffload', 'libgomp.oacc-*/*.exp'), which don't
     # get passed on to 'check_effective_target_*' functions.  (Not caching the
     # result due to that.)
-    set options [current_compiler_flags]
+    set options [concat "{additional_flags=-v" [current_compiler_flags] "}"]
     # Instead of inspecting command-line options, look what the compiler driver
     # decides.  This is somewhat modelled after
     # 'gcc/testsuite/lib/target-supports.exp:check_configured_with'.
-    set gcc_output [libgomp_target_compile "-v $options" "" "none" ""]
+    set gcc_output [libgomp_target_compile "" "" "none" $options]
     if [regexp "(?n)^OFFLOAD_TARGET_NAMES=(.*)" $gcc_output dummy offload_targets] {
 	verbose "compiling for offload targets: $offload_targets"
 	return [string match "*:nvptx*:*" ":$offload_targets:"]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Patch][Testsuite] – More fixes for remote execution: check_gc_sections_available, …
  2020-02-04 15:49 [Patch][Testsuite][libgomp] – Fix check_effective_target_offload_target_nvptx for remote execution Tobias Burnus
@ 2020-02-05  9:17 ` Tobias Burnus
  2020-02-05 16:36   ` Richard Sandiford
  2020-02-05 16:31 ` [Patch][Testsuite][libgomp] – Fix check_effective_target_offload_target_nvptx for remote execution Richard Sandiford
  1 sibling, 1 reply; 5+ messages in thread
From: Tobias Burnus @ 2020-02-05  9:17 UTC (permalink / raw)
  To: gcc-patches, Rainer Orth, Mike Stump, Jakub Jelinek

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

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

[-- Attachment #2: target-compile.diff --]
[-- Type: text/x-patch, Size: 2987 bytes --]

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch][Testsuite][libgomp] – Fix check_effective_target_offload_target_nvptx for remote execution
  2020-02-04 15:49 [Patch][Testsuite][libgomp] – Fix check_effective_target_offload_target_nvptx for remote execution Tobias Burnus
  2020-02-05  9:17 ` [Patch][Testsuite] – More fixes for remote execution: check_gc_sections_available, … Tobias Burnus
@ 2020-02-05 16:31 ` Richard Sandiford
  1 sibling, 0 replies; 5+ messages in thread
From: Richard Sandiford @ 2020-02-05 16:31 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: gcc-patches, Rainer Orth, Mike Stump, Jakub Jelinek

Tobias Burnus <tobias@codesourcery.com> writes:
> diff --git a/libgomp/testsuite/lib/libgomp.exp b/libgomp/testsuite/lib/libgomp.exp
> index 7e94527c7ca..cb7757b6a91 100644
> --- a/libgomp/testsuite/lib/libgomp.exp
> +++ b/libgomp/testsuite/lib/libgomp.exp
> @@ -346,11 +346,11 @@ proc check_effective_target_offload_target_nvptx { } {
>      # files; in particular, '-foffload', 'libgomp.oacc-*/*.exp'), which don't
>      # get passed on to 'check_effective_target_*' functions.  (Not caching the
>      # result due to that.)
> -    set options [current_compiler_flags]
> +    set options [concat "{additional_flags=-v" [current_compiler_flags] "}"]

Using:

    set options [list "additional_flags=[concat "-v" [current_compiler_flags]]"]

is maybe more obvious, and should get the { ... } quoting right for
unusual cases.

LGTM otherwise.

Thanks,
Richard

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch][Testsuite] – More fixes for remote execution: check_gc_sections_available, …
  2020-02-05  9:17 ` [Patch][Testsuite] – More fixes for remote execution: check_gc_sections_available, … Tobias Burnus
@ 2020-02-05 16:36   ` Richard Sandiford
  2020-02-06 12:29     ` Tobias Burnus
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Sandiford @ 2020-02-05 16:36 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: gcc-patches, Rainer Orth, Mike Stump, Jakub Jelinek

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Patch][Testsuite] – More fixes for remote execution: check_gc_sections_available, …
  2020-02-05 16:36   ` Richard Sandiford
@ 2020-02-06 12:29     ` Tobias Burnus
  0 siblings, 0 replies; 5+ messages in thread
From: Tobias Burnus @ 2020-02-06 12:29 UTC (permalink / raw)
  To: Tobias Burnus, gcc-patches, Rainer Orth, Mike Stump, richard.sandiford

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

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


[-- Attachment #2: committed.diff --]
[-- Type: text/x-patch, Size: 4056 bytes --]

commit 101baaee42afe05c3d271925e4d40f0f8f642bd5
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Thu Feb 6 13:27:45 2020 +0100

    [Testsuite] – More fixes for remote execution: check_gc_sections_available, …
    
            * 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/ChangeLog b/gcc/testsuite/ChangeLog
index 7b0b9c2c242..d0955e039b5 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2020-02-06  Tobias Burnus  <tobias@codesourcery.com>
+
+	* 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.
+
 2020-02-06  Jakub Jelinek  <jakub@redhat.com>
 
 	PR target/93594
diff --git a/gcc/testsuite/gcc.target/arm/multilib.exp b/gcc/testsuite/gcc.target/arm/multilib.exp
index 67d00266f6b..17111ee5257 100644
--- a/gcc/testsuite/gcc.target/arm/multilib.exp
+++ b/gcc/testsuite/gcc.target/arm/multilib.exp
@@ -40,7 +40,8 @@ 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 options [list "additional_flags=[concat "--print-multi-directory" [gcc_opts]]"]
+    set gcc_output [${tool}_target_compile "" "" "none" $options]
     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..d3b2798df3e 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -260,7 +260,8 @@ proc check_runtime {prop args} {
 proc check_configured_with { pattern } {
     global tool
 
-    set gcc_output [${tool}_target_compile "-v" "" "none" ""]
+    set options [list "additional_flags=-v"]
+    set gcc_output [${tool}_target_compile "" "" "none" $options]
     if { [ regexp "Configured with: \[^\n\]*$pattern" $gcc_output ] } {
         verbose "Matched: $pattern" 2
         return 1
@@ -504,7 +505,8 @@ 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 options [list "additional_flags=-print-prog-name=ld"]
+	set gcc_ld [lindex [${tool}_target_compile "" "" "none" $options] 0]
 	set ld_output [remote_exec host "$gcc_ld" "--help"]
 	if { [ string first "--gc-sections" $ld_output ] >= 0 } {
 	    return 1
@@ -8566,7 +8568,8 @@ 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 options [list "additional_flags=-print-prog-name=as"]
+	set gcc_as [lindex [${tool}_target_compile "" "" "none" $options] 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 +8591,8 @@ 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 options [list "additional_flags=-print-prog-name=ld"]
+	set gcc_ld [lindex [${tool}_target_compile "" "" "none" $options] 0]
 	set status [remote_exec host "$gcc_ld" "--version"]
 	set ld_output [lindex $status 1]
 	if { [ string first "GNU" $ld_output ] >= 0 } {

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-02-06 12:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-04 15:49 [Patch][Testsuite][libgomp] – Fix check_effective_target_offload_target_nvptx for remote execution Tobias Burnus
2020-02-05  9:17 ` [Patch][Testsuite] – More fixes for remote execution: check_gc_sections_available, … Tobias Burnus
2020-02-05 16:36   ` Richard Sandiford
2020-02-06 12:29     ` Tobias Burnus
2020-02-05 16:31 ` [Patch][Testsuite][libgomp] – Fix check_effective_target_offload_target_nvptx for remote execution Richard Sandiford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).