Hi Frederik!
(We had internally discussed this.)
I can't formally approve testsuite patches, but did a review anyway:
On 2020-05-15T12:31:54+0200, Frederik Harwath <frede...@codesourcery.com> wrote:
> The test commands for scanning optimization dump files
> perform globbing on the argument that specifies the suffix
> of the dump files to be scanned. This behavior is currently
> undocumented. Furthermore, the current implementation of
> "scan-dump" and related procedures yields an error whenever
> the file name globbing matches more than one file (due to an
> attempt to call "open" on multiple files) while a failure to
> match any file at all results in an unresolved test.
>
> This patch documents the globbing behavior.
ACK.
> The dump
> scanning procedures are changed to make the test unresolved
> if globbing matches more than one file.
(The code changes look good, but I have not tested that specific aspect.)
> The procedures in scandump.exp all perform the file name expansion in
> essentially the same way and I have extracted this into a new
> procedure.
Thanks, that looks like I thought it should.
> But there is one very minor exception:
>
>> @@ -67,10 +95,10 @@ proc scan-dump { args } {
>> set dumpbase [dump-base $src [lindex $args 3]]
>> - set output_file "[glob -nocomplain $dumpbase.[lindex $args 2]]"
>> +
>> + set pattern "$dumpbase.[lindex $args 2]"
>> + set output_file "[glob-dump-file $testcase $pattern]"
>> if { $output_file == "" } {
>> - verbose -log "$testcase: dump file does not exist"
>> - verbose -log "dump file: $dumpbase.$suf"
>
> "scan-dump" is the only procedure that prints the "dump file: ..." line.
> Should this be kept or is it ok to remove this as I have done in the
> patch? $dumpbase.$suf does not emit the correct file name anyway
> (a random example from my testing: "dump file: stdatomic-init.c.dce*")
> and the name of the files can be inferred from the test name easily.
OK to remove, as far as I'm concerned.
> I have tested the changes by running "make check" (with a
> --enable-languages=C only build, but this covers lots of uses
> of the affected test procedures) and observed no regressions.
I did a full 'make check' on powerpc64le-unknown-linux-gnu -- no issues
found.
> Ok to commit this to master?
As I said, not an approval, and minor comments (see below), but still:
Reviewed-by: Thomas Schwinge <tho...@codesourcery.com>
Do we have to similarly also audit/alter other testsuite infrastructure
files, anything that uses '[glob [...]]'? (..., and then generalize
'glob-dump-file' into 'glob-one-file', or similar.) That can be done
incrementally, as far as I'm concerned.
> From 6912e03d51d360dbbcf7eb1dc8d77d08c2a6e54c Mon Sep 17 00:00:00 2001
> From: Frederik Harwath <frede...@codesourcery.com>
> Date: Fri, 15 May 2020 10:35:48 +0200
> Subject: [PATCH] testsuite: clarify scan-dump file globbing behavior
>
> The test commands for scanning optimization dump files
> perform globbing on the argument that specifies the suffix
> of the dump files to be scanned. This behavior is currently
> undocumented. Furthermore, the current implementation of
> "scan-dump" and similar procedures yields an error whenever
> the file name globbing matches more than one file (due to an
> attempt to call "open" on multiple files) while a failure to
> match any file results in an unresolved test.
>
> This commit documents the globbing behavior. The dump
> scanning procedures are changed to make the test unresolved
> if globbing matches more than one file.
>
> gcc/ChangeLog:
>
> 2020-05-15 Frederik Harwath <frede...@codesourcery.com>
>
> * doc/sourcebuild.texi: Describe globbing of the
> dump file scanning commands "suffix" argument.
>
> gcc/testsuite/ChangeLog:
>
> 2020-05-15 Frederik Harwath <frede...@codesourcery.com>
>
> * lib/scandump.exp (glob-dump-file): New proc.
> (scan-dump): Use glob-dump-file for file name expansion.
> (scan-dump-times): Likewise.
> (scan-dump-dem): Likewise.
> (scan-dump-dem-not): Likewise.
> ---
> gcc/doc/sourcebuild.texi | 4 ++-
> gcc/testsuite/lib/scandump.exp | 54 +++++++++++++++++++++++++++-------
> 2 files changed, 46 insertions(+), 12 deletions(-)
>
> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
> index 240d6e4b08e..b6c5a21cb71 100644
> --- a/gcc/doc/sourcebuild.texi
> +++ b/gcc/doc/sourcebuild.texi
> @@ -2888,7 +2888,9 @@ stands for zero or more unmatched lines; the whitespace
> after
>
> These commands are available for @var{kind} of @code{tree},
> @code{ltrans-tree},
> @code{offload-tree}, @code{rtl}, @code{offload-rtl}, @code{ipa}, and
> -@code{wpa-ipa}.
> +@code{wpa-ipa}. The @var{suffix} argument which describes the dump file
(You can add your new sentence addition on a new line.)
> +to be scanned may contain a glob pattern that must expand to exactly one
> +file name.
May also make this more useful/explicit:
This is useful if, for example, if a pass has several static
instances [correct terminology?], and depending on torture testing
command-line flags, a different instance executes and produces a dump
file, and so in the test case you can use a generic [put example
here] to scan the varying dump files names.
(Or similar.)
Grüße
Thomas
> diff --git a/gcc/testsuite/lib/scandump.exp b/gcc/testsuite/lib/scandump.exp
> index d6ba350acc8..f3a991b590a 100644
> --- a/gcc/testsuite/lib/scandump.exp
> +++ b/gcc/testsuite/lib/scandump.exp
> @@ -39,6 +39,34 @@ proc dump-base { args } {
> return $dumpbase
> }
>
> +# Expand dump file name pattern to exactly one file.
> +# Return a single dump file name or an empty string
> +# if the pattern matches no file or more than one file.
> +#
> +# Argument 0 is the testcase name
> +# Argument 1 is the dump file glob pattern
> +proc glob-dump-file { args } {
> +
> + set pattern [lindex $args 1]
> + set dump_file "[glob -nocomplain $pattern]"
> + set num_files [llength $dump_file]
> +
> + if { $num_files != 1 } {
> + set testcase [lindex $args 0]
> + if { $num_files == 0 } {
> + verbose -log "$testcase: dump file does not exist"
> + }
> +
> + if { $num_files > 1 } {
> + verbose -log "$testcase: multiple dump files found"
> + }
> +
> + return
> + }
> +
> + return $dump_file
> +}
> +
> # Utility for scanning compiler result, invoked via dg-final.
> # Call pass if pattern is present, otherwise fail.
> #
> @@ -67,10 +95,10 @@ proc scan-dump { args } {
> set testname "$testcase scan-[lindex $args 0]-dump $suf
> \"$printable_pattern\""
> set src [file tail $filename]
> set dumpbase [dump-base $src [lindex $args 3]]
> - set output_file "[glob -nocomplain $dumpbase.[lindex $args 2]]"
> +
> + set pattern "$dumpbase.[lindex $args 2]"
> + set output_file "[glob-dump-file $testcase $pattern]"
> if { $output_file == "" } {
> - verbose -log "$testcase: dump file does not exist"
> - verbose -log "dump file: $dumpbase.$suf"
> unresolved "$testname"
> return
> }
> @@ -113,9 +141,10 @@ proc scan-dump-times { args } {
> set testname "$testcase scan-[lindex $args 0]-dump-times $suf
> \"$printable_pattern\" [lindex $args 2]"
> set src [file tail $filename]
> set dumpbase [dump-base $src [lindex $args 4]]
> - set output_file "[glob -nocomplain $dumpbase.[lindex $args 3]]"
> +
> + set pattern "$dumpbase.[lindex $args 3]"
> + set output_file "[glob-dump-file $testcase $pattern]"
> if { $output_file == "" } {
> - verbose -log "$testcase: dump file does not exist"
> unresolved "$testname"
> return
> }
> @@ -159,9 +188,10 @@ proc scan-dump-not { args } {
> set testname "$testcase scan-[lindex $args 0]-dump-not $suf
> \"$printable_pattern\""
> set src [file tail $filename]
> set dumpbase [dump-base $src [lindex $args 3]]
> - set output_file "[glob -nocomplain $dumpbase.[lindex $args 2]]"
> +
> + set pattern "$dumpbase.[lindex $args 2]"
> + set output_file "[glob-dump-file $testcase $pattern]"
> if { $output_file == "" } {
> - verbose -log "$testcase: dump file does not exist"
> unresolved "$testname"
> return
> }
> @@ -216,9 +246,10 @@ proc scan-dump-dem { args } {
> set testname "$testcase scan-[lindex $args 0]-dump-dem $suf
> \"$printable_pattern\""
> set src [file tail $filename]
> set dumpbase [dump-base $src [lindex $args 3]]
> - set output_file "[glob -nocomplain $dumpbase.[lindex $args 2]]"
> +
> + set pattern "$dumpbase.[lindex $args 2]"
> + set output_file "[glob-dump-file $testcase $pattern]"
> if { $output_file == "" } {
> - verbose -log "$testcase: dump file does not exist"
> unresolved "$testname"
> return
> }
> @@ -272,9 +303,10 @@ proc scan-dump-dem-not { args } {
> set testname "$testcase scan-[lindex $args 0]-dump-dem-not $suf
> \"$printable_pattern\""
> set src [file tail $filename]
> set dumpbase [dump-base $src [lindex $args 3]]
> - set output_file "[glob -nocomplain $dumpbase.[lindex $args 2]]"
> +
> + set pattern "$dumpbase.[lindex $args 2]"
> + set output_file "[glob-dump-file $testcase $pattern]"
> if { $output_file == "" } {
> - verbose -log "$testcase: dump file does not exist"
> unresolved "$testname"
> return
> }
-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander
Walter
