| Message ID | 20240816172024.1648237-1-hjl.tools@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Extend check-function-bodies to cover directives | expand |
"H.J. Lu" <hjl.tools@gmail.com> writes: > As PR target/116174 shown, we may need to verify the directive order. > Extend check-function-bodies to cover directives. > > * gcc.target/i386/pr116174.c: Use check-function-bodies. > * lib/scanasm.exp (configure_check-function-bodies): Add an > argument for fluff. Set up_config(fluff) to $fluff if not > empty. > (check-function-bodies): Add an optional argument for fluff and > pass it to configure_check-function-bodies. Looks like a useful feature, but how about instead making the extra argument specify things that *should* be matched, rather than things that shouldn't? That argument would then take precedence over the current fluff regexp. That might be easier to maintain, since it wouldn't need to repeat the knowledge currently in configure_check-function-bodies. And it should cope more easily with multiple assembly dialects. The documentation in doc/sourcebuild.texi would need to be updated as well. Thanks, Richard > > Signed-off-by: H.J. Lu <hjl.tools@gmail.com> > --- > gcc/testsuite/gcc.target/i386/pr116174.c | 16 ++++++++++++++-- > gcc/testsuite/lib/scanasm.exp | 17 ++++++++++++----- > 2 files changed, 26 insertions(+), 7 deletions(-) > > diff --git a/gcc/testsuite/gcc.target/i386/pr116174.c b/gcc/testsuite/gcc.target/i386/pr116174.c > index 8877d0b51af..75c62964d97 100644 > --- a/gcc/testsuite/gcc.target/i386/pr116174.c > +++ b/gcc/testsuite/gcc.target/i386/pr116174.c > @@ -1,6 +1,20 @@ > /* { dg-do compile { target *-*-linux* } } */ > /* { dg-options "-O2 -fcf-protection=branch" } */ > +/* Keep directives ('.p2align', '.cfi_startproc'). > +/* { dg-final { check-function-bodies "**" "" "" { target "*-*-*" } {^\s*(?://|$)} } } */ > > +/* > +**foo: > +**... > +** .cfi_startproc > +** ( > +** endbr64 > +** | > +** endbr32 > +** ) > +** .p2align 5 > +**... > +*/ > char * > foo (char *dest, const char *src) > { > @@ -8,5 +22,3 @@ foo (char *dest, const char *src) > /* nothing */; > return --dest; > } > - > -/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n" } } */ > diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp > index 42c719c512c..5165284608f 100644 > --- a/gcc/testsuite/lib/scanasm.exp > +++ b/gcc/testsuite/lib/scanasm.exp > @@ -863,7 +863,7 @@ proc scan-lto-assembler { args } { > > # Set up CONFIG for check-function-bodies. > > -proc configure_check-function-bodies { config } { > +proc configure_check-function-bodies { config fluff } { > upvar $config up_config > > # Regexp for the start of a function definition (name in \1). > @@ -890,7 +890,9 @@ proc configure_check-function-bodies { config } { > } > > # Regexp for lines that aren't interesting. > - if { [istarget nvptx*-*-*] } { > + if {$fluff ne ""} then { > + set up_config(fluff) $fluff > + } elseif { [istarget nvptx*-*-*] } { > # Skip lines beginning with '//' comments ('-fverbose-asm', for > # example). > set up_config(fluff) {^\s*(?://)} > @@ -982,7 +984,7 @@ proc check_function_body { functions name body_regexp } { > > # Check the implementations of functions against expected output. Used as: > # > -# { dg-do { check-function-bodies PREFIX TERMINATOR[ OPTION[ SELECTOR]] } } > +# { dg-do { check-function-bodies PREFIX TERMINATOR[ OPTION[ SELECTOR [FLUFF]]] } } > # > # See sourcebuild.texi for details. > > @@ -990,7 +992,7 @@ proc check-function-bodies { args } { > if { [llength $args] < 2 } { > error "too few arguments to check-function-bodies" > } > - if { [llength $args] > 4 } { > + if { [llength $args] > 5 } { > error "too many arguments to check-function-bodies" > } > > @@ -1029,6 +1031,11 @@ proc check-function-bodies { args } { > } > } > > + set fluff "" > + if { [llength $args] >= 5 } { > + set fluff [lindex $args 4] > + } > + > set testcase [testname-for-summary] > # The name might include a list of options; extract the file name. > set filename [lindex $testcase 0] > @@ -1048,7 +1055,7 @@ proc check-function-bodies { args } { > # (name in \1). This may be different from '$config(start)'. > set start_expected {^(\S+):$} > > - configure_check-function-bodies config > + configure_check-function-bodies config $fluff > set have_bodies 0 > if { [is_remote host] } { > remote_upload host "$filename"
On Thu, Aug 22, 2024 at 9:42 AM Richard Sandiford <richard.sandiford@arm.com> wrote: > > "H.J. Lu" <hjl.tools@gmail.com> writes: > > As PR target/116174 shown, we may need to verify the directive order. > > Extend check-function-bodies to cover directives. > > > > * gcc.target/i386/pr116174.c: Use check-function-bodies. > > * lib/scanasm.exp (configure_check-function-bodies): Add an > > argument for fluff. Set up_config(fluff) to $fluff if not > > empty. > > (check-function-bodies): Add an optional argument for fluff and > > pass it to configure_check-function-bodies. > > Looks like a useful feature, but how about instead making the extra > argument specify things that *should* be matched, rather than things > that shouldn't? That argument would then take precedence over the > current fluff regexp. > > That might be easier to maintain, since it wouldn't need to repeat the > knowledge currently in configure_check-function-bodies. And it should > cope more easily with multiple assembly dialects. Fixed in v2. > The documentation in doc/sourcebuild.texi would need to be updated as well. Fixed in v2: https://gcc.gnu.org/pipermail/gcc-patches/2024-August/661226.html Thanks. H.J.
diff --git a/gcc/testsuite/gcc.target/i386/pr116174.c b/gcc/testsuite/gcc.target/i386/pr116174.c index 8877d0b51af..75c62964d97 100644 --- a/gcc/testsuite/gcc.target/i386/pr116174.c +++ b/gcc/testsuite/gcc.target/i386/pr116174.c @@ -1,6 +1,20 @@ /* { dg-do compile { target *-*-linux* } } */ /* { dg-options "-O2 -fcf-protection=branch" } */ +/* Keep directives ('.p2align', '.cfi_startproc'). +/* { dg-final { check-function-bodies "**" "" "" { target "*-*-*" } {^\s*(?://|$)} } } */ +/* +**foo: +**... +** .cfi_startproc +** ( +** endbr64 +** | +** endbr32 +** ) +** .p2align 5 +**... +*/ char * foo (char *dest, const char *src) { @@ -8,5 +22,3 @@ foo (char *dest, const char *src) /* nothing */; return --dest; } - -/* { dg-final { scan-assembler "\t\.cfi_startproc\n\tendbr(32|64)\n" } } */ diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp index 42c719c512c..5165284608f 100644 --- a/gcc/testsuite/lib/scanasm.exp +++ b/gcc/testsuite/lib/scanasm.exp @@ -863,7 +863,7 @@ proc scan-lto-assembler { args } { # Set up CONFIG for check-function-bodies. -proc configure_check-function-bodies { config } { +proc configure_check-function-bodies { config fluff } { upvar $config up_config # Regexp for the start of a function definition (name in \1). @@ -890,7 +890,9 @@ proc configure_check-function-bodies { config } { } # Regexp for lines that aren't interesting. - if { [istarget nvptx*-*-*] } { + if {$fluff ne ""} then { + set up_config(fluff) $fluff + } elseif { [istarget nvptx*-*-*] } { # Skip lines beginning with '//' comments ('-fverbose-asm', for # example). set up_config(fluff) {^\s*(?://)} @@ -982,7 +984,7 @@ proc check_function_body { functions name body_regexp } { # Check the implementations of functions against expected output. Used as: # -# { dg-do { check-function-bodies PREFIX TERMINATOR[ OPTION[ SELECTOR]] } } +# { dg-do { check-function-bodies PREFIX TERMINATOR[ OPTION[ SELECTOR [FLUFF]]] } } # # See sourcebuild.texi for details. @@ -990,7 +992,7 @@ proc check-function-bodies { args } { if { [llength $args] < 2 } { error "too few arguments to check-function-bodies" } - if { [llength $args] > 4 } { + if { [llength $args] > 5 } { error "too many arguments to check-function-bodies" } @@ -1029,6 +1031,11 @@ proc check-function-bodies { args } { } } + set fluff "" + if { [llength $args] >= 5 } { + set fluff [lindex $args 4] + } + set testcase [testname-for-summary] # The name might include a list of options; extract the file name. set filename [lindex $testcase 0] @@ -1048,7 +1055,7 @@ proc check-function-bodies { args } { # (name in \1). This may be different from '$config(start)'. set start_expected {^(\S+):$} - configure_check-function-bodies config + configure_check-function-bodies config $fluff set have_bodies 0 if { [is_remote host] } { remote_upload host "$filename"
As PR target/116174 shown, we may need to verify the directive order. Extend check-function-bodies to cover directives. * gcc.target/i386/pr116174.c: Use check-function-bodies. * lib/scanasm.exp (configure_check-function-bodies): Add an argument for fluff. Set up_config(fluff) to $fluff if not empty. (check-function-bodies): Add an optional argument for fluff and pass it to configure_check-function-bodies. Signed-off-by: H.J. Lu <hjl.tools@gmail.com> --- gcc/testsuite/gcc.target/i386/pr116174.c | 16 ++++++++++++++-- gcc/testsuite/lib/scanasm.exp | 17 ++++++++++++----- 2 files changed, 26 insertions(+), 7 deletions(-)