Re: testsuite: Port 'check-function-bodies' to nvptx

[Richard Sandiford <richard.sandiford@arm.com>]

Thomas Schwinge <thomas@codesourcery.com> writes:
> Hi!
>
> On 2023-09-04T23:05:05+0200, I wrote:
>> On 2019-07-16T15:04:49+0100, Richard Sandiford <richard.sandiford@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 <thomas@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"] } {