Thomas Schwinge <tho...@codesourcery.com> writes:
> Hi!
>
> On 2023-09-04T23:05:05+0200, I wrote:
>> On 2019-07-16T15:04:49+0100, Richard Sandiford <richard.sandif...@arm.com>
>> wrote:
>>> This patch therefore adds a new check-function-bodies dg-final test
>
>>> The regexps in parse_function_bodies are fairly general, but might
>>> still need to be extended in future for targets like Darwin or AIX.
>>
>> ..., or nvptx. [...]
>
>> number of TODO items.
>>
>> In particular how to parameterize regular expressions for the different
>> syntax used by nvptx: for example, parameterize via global variables,
>> initialized accordingly (where?)? Thinking about it, maybe simply
>> conditionalizing the current local initializations by
>> 'if { [istarget nvptx-*-*] } { [...] } else { [...] }' will do, simple
>> enough!
>
> Indeed that works fine.
>
>> Regarding whitespace prefixed, I think I'll go with the current
>> 'append function_regexp "\t" $line "\n"', that is, prefix expected output
>> lines with '\t' (as done in 'gcc.target/nvptx/abort.c'), and also for
>> nvptx handle labels as "fluff" (until we solve that issue generally).
>
> I changed my mind about that: instead of '\t', use '\t*' for nvptx, which
> means that both instructions emitted with additional whitespace prefixed
> and labels in column zero work nicely.
>
>> --- a/gcc/testsuite/lib/scanasm.exp
>> +++ b/gcc/testsuite/lib/scanasm.exp
>
>> @@ -907,7 +911,8 @@ proc check-function-bodies { args } {
>>
>> set count 0
>> set function_regexp ""
>> - set label {^(\S+):$}
>> + #TODO
>> + set label {^// BEGIN GLOBAL FUNCTION DEF: ([a-zA-Z_]\S+)$}
>
> There's actually no reason that the expected output syntax (this one) has
> to match the assembly -- so I restored that, to use the same syntax for
> nvptx here, too.
>
> Any comments before I push the attached
> "testsuite: Port 'check-function-bodies' to nvptx"?
>
>
> Grüße
> Thomas
>
>
> -----------------
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
> München, HRB 106955
>
> From bdaf7572d9d4c1988274405840de4071ded3733f Mon Sep 17 00:00:00 2001
> From: Thomas Schwinge <tho...@codesourcery.com>
> Date: Mon, 4 Sep 2023 22:28:12 +0200
> Subject: [PATCH] testsuite: Port 'check-function-bodies' to nvptx
>
> This extends commit 4d706ff86ea86868615558e92407674a4f4b4af9
> "Add dg test for matching function bodies" for nvptx.
>
> gcc/testsuite/
> * lib/scanasm.exp (configure_check-function-bodies): New proc.
> (parse_function_bodies, check-function-bodies): Use it.
> * gcc.target/nvptx/abort.c: Use 'check-function-bodies'.
> gcc/
> * doc/sourcebuild.texi (check-function-bodies): Update.
LGTM. Just a minor comment:
> ---
> gcc/doc/sourcebuild.texi | 9 ++-
> gcc/testsuite/gcc.target/nvptx/abort.c | 19 ++++++-
> gcc/testsuite/lib/scanasm.exp | 76 ++++++++++++++++++++------
> 3 files changed, 83 insertions(+), 21 deletions(-)
>
> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
> index 1a78b3c1abb..8aec6b6592c 100644
> --- a/gcc/doc/sourcebuild.texi
> +++ b/gcc/doc/sourcebuild.texi
> @@ -3327,9 +3327,12 @@ The first line of the expected output for a function
> @var{fn} has the form:
> Subsequent lines of the expected output also start with @var{prefix}.
> In both cases, whitespace after @var{prefix} is not significant.
>
> -The test discards assembly directives such as @code{.cfi_startproc}
> -and local label definitions such as @code{.LFB0} from the compiler's
> -assembly output. It then matches the result against the expected
> +Depending on the configuration (see
> +@code{gcc/testsuite/lib/scanasm.exp:configure_check-function-bodies}),
I can imagine such a long string wouldn't format well in the output.
How about: @code{configure_check-function-bodies} in
@filename{gcc/testsuite/lib/scanasm.exp}?
OK from my POV with that change.
Thanks,
Richard
> +the test may discard from the compiler's assembly output
> +directives such as @code{.cfi_startproc},
> +local label definitions such as @code{.LFB0}, and more.
> +It then matches the result against the expected
> output for a function as a single regular expression. This means that
> later lines can use backslashes to refer back to @samp{(@dots{})}
> captures on earlier lines. For example:
> diff --git a/gcc/testsuite/gcc.target/nvptx/abort.c
> b/gcc/testsuite/gcc.target/nvptx/abort.c
> index d3220687400..ae9dbf45a9b 100644
> --- a/gcc/testsuite/gcc.target/nvptx/abort.c
> +++ b/gcc/testsuite/gcc.target/nvptx/abort.c
> @@ -1,4 +1,6 @@
> /* { dg-do compile} */
> +/* { dg-final { check-function-bodies {**} {} } } */
> +
> /* Annotate no return functions with a trailing 'trap'. */
>
> extern void abort ();
> @@ -9,5 +11,18 @@ int main (int argc, char **argv)
> abort ();
> return 0;
> }
> -
> -/* { dg-final { scan-assembler "call abort;\[\r\n\t \]+trap;" } } */
> +/*
> +** main:
> +** ...
> +** \.reg\.pred (%r[0-9]+);
> +** ...
> +** @\1 bra (\$L[0-9]+);
> +** {
> +** call abort;
> +** trap; // \(noreturn\)
> +** exit; // \(noreturn\)
> +** }
> +** \2:
> +** \tmov\.u32 %r[0-9]+, 0;
> +** ...
> +*/
> diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
> index 0685de1d641..5df80325dff 100644
> --- a/gcc/testsuite/lib/scanasm.exp
> +++ b/gcc/testsuite/lib/scanasm.exp
> @@ -777,33 +777,73 @@ proc scan-lto-assembler { args } {
> dg-scan "scan-lto-assembler" 1 $testcase $output_file $args
> }
>
> -# Read assembly file FILENAME and store a mapping from function names
> -# to function bodies in array RESULT. FILENAME has already been uploaded
> -# locally where necessary and is known to exist.
>
> -proc parse_function_bodies { filename result } {
> - upvar $result up_result
> +# Set up CONFIG for check-function-bodies.
> +
> +proc configure_check-function-bodies { config } {
> + upvar $config up_config
>
> # Regexp for the start of a function definition (name in \1).
> - set label {^([a-zA-Z_]\S+):$}
> + if { [istarget nvptx*-*-*] } {
> + set up_config(start) {^// BEGIN(?: GLOBAL|) FUNCTION DEF:
> ([a-zA-Z_]\S+)$}
> + } else {
> + set up_config(start) {^([a-zA-Z_]\S+):$}
> + }
>
> # Regexp for the end of a function definition.
> - set terminator {^\s*\.size}
> -
> + if { [istarget nvptx*-*-*] } {
> + set up_config(end) {^\}$}
> + } else {
> + set up_config(end) {^\s*\.size}
> + }
> +
> # Regexp for lines that aren't interesting.
> - set fluff {^\s*(?:\.|//|@|$)}
> + if { [istarget nvptx*-*-*] } {
> + # Skip lines beginning with '//' comments ('-fverbose-asm', for
> + # example).
> + set up_config(fluff) {^\s*(?://)}
> + } else {
> + # Skip lines beginning with labels ('.L[...]:') or other directives
> + # ('.align', '.cfi_startproc', '.quad [...]', '.text', etc.), '//' or
> + # '@' comments ('-fverbose-asm' or ARM-style, for example), or empty
> + # lines.
> + set up_config(fluff) {^\s*(?:\.|//|@|$)}
> + }
> +
> + # Regexp for expected output lines prefix.
> + if { [istarget nvptx*-*-*] } {
> + # Certain instructions (such as predicable ones) are emitted with
> + # additional whitespace prefixed. On the other hand, labels don't get
> + # any whitespace prefixed, and we'd like to be able to match these,
> + # too. We thereare expect any amount of whitespace here.
> + set up_config(line_prefix) {\t*}
> + } else {
> + set up_config(line_prefix) {\t}
> + }
> +}
> +
> +# Per CONFIG, read assembly file FILENAME and store a mapping from function
> +# names to function bodies in array RESULT. FILENAME has already been
> uploaded
> +# locally where necessary and is known to exist.
> +
> +proc parse_function_bodies { config filename result } {
> + upvar $config up_config
> + upvar $result up_result
>
> set fd [open $filename r]
> set in_function 0
> while { [gets $fd line] >= 0 } {
> - if { [regexp $label $line dummy function_name] } {
> + if { [regexp $up_config(start) $line dummy function_name] } {
> set in_function 1
> set function_body ""
> } elseif { $in_function } {
> - if { [regexp $terminator $line] } {
> + if { [regexp $up_config(end) $line] } {
> + verbose "parse_function_bodies: $function_name:\n$function_body"
> set up_result($function_name) $function_body
> set in_function 0
> - } elseif { ![regexp $fluff $line] } {
> + } elseif { [regexp $up_config(fluff) $line] } {
> + verbose "parse_function_bodies: $function_name: ignoring fluff
> line: $line"
> + } else {
> append function_body $line "\n"
> }
> }
> @@ -893,13 +933,18 @@ proc check-function-bodies { args } {
> set terminator "*/"
> }
> set terminator_len [string length $terminator]
> + # Regexp for the start of a function definition in expected output lines
> + # (name in \1). This may be different from '$config(start)'.
> + set start_expected {^(\S+):$}
> +
> + configure_check-function-bodies config
>
> set have_bodies 0
> if { [is_remote host] } {
> remote_upload host "$filename"
> }
> if { [file exists $output_filename] } {
> - parse_function_bodies $output_filename functions
> + parse_function_bodies config $output_filename functions
> set have_bodies 1
> } else {
> verbose -log "$testcase: output file does not exist"
> @@ -907,7 +952,6 @@ proc check-function-bodies { args } {
>
> set count 0
> set function_regexp ""
> - set label {^(\S+):$}
>
> set lineno 1
> set fd [open $input_filename r]
> @@ -922,7 +966,7 @@ proc check-function-bodies { args } {
> } else {
> set selector "P"
> }
> - if { ![regexp $label $line dummy function_name] } {
> + if { ![regexp $start_expected $line dummy function_name] } {
> close $fd
> error "check-function-bodies: line $lineno does not have a
> function label"
> }
> @@ -937,7 +981,7 @@ proc check-function-bodies { args } {
> } elseif { [string equal $line "..."] } {
> append function_regexp ".*"
> } else {
> - append function_regexp "\t" $line "\n"
> + append function_regexp $config(line_prefix) $line "\n"
> }
> } elseif { [string equal -length $terminator_len $line $terminator] } {
> if { ![string equal $selector "N"] } {
