Message ID | 5a820893-e10d-8da9-8ab4-c2983a4d9e1e@suse.cz |
---|---|
State | New |
Headers | show |
Series | Improve FAIL message for dump-*-times functions. | expand |
On 10/11/2017 02:14 AM, Martin Liška wrote: > Hi. > > This patch helps to find why an expected number of scan patterns does not match: > > FAIL: gcc.dg/unroll-3.c scan-tree-dump-times cunrolli "loop with 3 iterations completely unrolled" 222 (found 1 times) > FAIL: c-c++-common/attr-simd-2.c -Wc++-compat scan-assembler-times _ZGVbN4_simd_attr: 111 (found 1 times) > > Ready to be installed after testing? > Thanks, > Martin > > gcc/testsuite/ChangeLog: > > 2017-10-11 Martin Liska <mliska@suse.cz> > > * lib/scanasm.exp: Print how many times a regex pattern is > found. > * lib/scandump.exp: Likewise. OK. Definitely helpful. jeff
Hi! On Wed, Oct 11, 2017 at 10:14:29AM +0200, Martin Liška wrote: > This patch helps to find why an expected number of scan patterns does not match: > > FAIL: gcc.dg/unroll-3.c scan-tree-dump-times cunrolli "loop with 3 iterations completely unrolled" 222 (found 1 times) > FAIL: c-c++-common/attr-simd-2.c -Wc++-compat scan-assembler-times _ZGVbN4_simd_attr: 111 (found 1 times) Cool, looks fine to me (but I can't approve it). Some suggestions: > diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp > index bab23e8e165..e90e61c29ae 100644 > --- a/gcc/testsuite/lib/scanasm.exp > +++ b/gcc/testsuite/lib/scanasm.exp > @@ -247,10 +247,11 @@ proc scan-assembler-times { args } { > set text [read $fd] > close $fd > > - if { [llength [regexp -inline -all -- $pattern $text]] == [lindex $args 1]} { > + set pattern_count [llength [regexp -inline -all -- $pattern $text]] > + if {$pattern_count == [lindex $args 1]} { > pass "$testcase scan-assembler-times $pp_pattern [lindex $args 1]" > } else { > - fail "$testcase scan-assembler-times $pp_pattern [lindex $args 1]" > + fail "$testcase scan-assembler-times $pp_pattern [lindex $args 1] (found $pattern_count times)" > } > } pattern_count is not such a great name (it's the result count, instead). You could factor out the [lindex $args 1] to a variable. You do both of these in scandump.exp already, so why not here :-) Segher > diff --git a/gcc/testsuite/lib/scandump.exp b/gcc/testsuite/lib/scandump.exp > index 2e6eebfaf33..4a64ac6e05d 100644 > --- a/gcc/testsuite/lib/scandump.exp > +++ b/gcc/testsuite/lib/scandump.exp > @@ -86,6 +86,7 @@ proc scan-dump-times { args } { > } > > set testcase [testname-for-summary] > + set times [lindex $args 2] > set suf [dump-suffix [lindex $args 3]] > set printable_pattern [make_pattern_printable [lindex $args 1]] > set testname "$testcase scan-[lindex $args 0]-dump-times $suf \"$printable_pattern\" [lindex $args 2]" > @@ -101,10 +102,11 @@ proc scan-dump-times { args } { > set text [read $fd] > close $fd > > - if { [llength [regexp -inline -all -- [lindex $args 1] $text]] == [lindex $args 2]} { > + set result_count [llength [regexp -inline -all -- [lindex $args 1] $text]] > + if {$result_count == $times} { > pass "$testname" > } else { > - fail "$testname" > + fail "$testname (found $result_count times)" > } > }
On 10/11/2017 06:56 PM, Segher Boessenkool wrote: > Hi! > > On Wed, Oct 11, 2017 at 10:14:29AM +0200, Martin Liška wrote: >> This patch helps to find why an expected number of scan patterns does not match: >> >> FAIL: gcc.dg/unroll-3.c scan-tree-dump-times cunrolli "loop with 3 iterations completely unrolled" 222 (found 1 times) >> FAIL: c-c++-common/attr-simd-2.c -Wc++-compat scan-assembler-times _ZGVbN4_simd_attr: 111 (found 1 times) > > Cool, looks fine to me (but I can't approve it). Some suggestions: > >> diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp >> index bab23e8e165..e90e61c29ae 100644 >> --- a/gcc/testsuite/lib/scanasm.exp >> +++ b/gcc/testsuite/lib/scanasm.exp >> @@ -247,10 +247,11 @@ proc scan-assembler-times { args } { >> set text [read $fd] >> close $fd >> >> - if { [llength [regexp -inline -all -- $pattern $text]] == [lindex $args 1]} { >> + set pattern_count [llength [regexp -inline -all -- $pattern $text]] >> + if {$pattern_count == [lindex $args 1]} { >> pass "$testcase scan-assembler-times $pp_pattern [lindex $args 1]" >> } else { >> - fail "$testcase scan-assembler-times $pp_pattern [lindex $args 1]" >> + fail "$testcase scan-assembler-times $pp_pattern [lindex $args 1] (found $pattern_count times)" >> } >> } > > pattern_count is not such a great name (it's the result count, instead). > > You could factor out the [lindex $args 1] to a variable. > > You do both of these in scandump.exp already, so why not here :-) Thanks for review, I fixed that and I'm going to install the patch. Martin > > > Segher > > >> diff --git a/gcc/testsuite/lib/scandump.exp b/gcc/testsuite/lib/scandump.exp >> index 2e6eebfaf33..4a64ac6e05d 100644 >> --- a/gcc/testsuite/lib/scandump.exp >> +++ b/gcc/testsuite/lib/scandump.exp >> @@ -86,6 +86,7 @@ proc scan-dump-times { args } { >> } >> >> set testcase [testname-for-summary] >> + set times [lindex $args 2] >> set suf [dump-suffix [lindex $args 3]] >> set printable_pattern [make_pattern_printable [lindex $args 1]] >> set testname "$testcase scan-[lindex $args 0]-dump-times $suf \"$printable_pattern\" [lindex $args 2]" >> @@ -101,10 +102,11 @@ proc scan-dump-times { args } { >> set text [read $fd] >> close $fd >> >> - if { [llength [regexp -inline -all -- [lindex $args 1] $text]] == [lindex $args 2]} { >> + set result_count [llength [regexp -inline -all -- [lindex $args 1] $text]] >> + if {$result_count == $times} { >> pass "$testname" >> } else { >> - fail "$testname" >> + fail "$testname (found $result_count times)" >> } >> }
diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp index bab23e8e165..e90e61c29ae 100644 --- a/gcc/testsuite/lib/scanasm.exp +++ b/gcc/testsuite/lib/scanasm.exp @@ -247,10 +247,11 @@ proc scan-assembler-times { args } { set text [read $fd] close $fd - if { [llength [regexp -inline -all -- $pattern $text]] == [lindex $args 1]} { + set pattern_count [llength [regexp -inline -all -- $pattern $text]] + if {$pattern_count == [lindex $args 1]} { pass "$testcase scan-assembler-times $pp_pattern [lindex $args 1]" } else { - fail "$testcase scan-assembler-times $pp_pattern [lindex $args 1]" + fail "$testcase scan-assembler-times $pp_pattern [lindex $args 1] (found $pattern_count times)" } } diff --git a/gcc/testsuite/lib/scandump.exp b/gcc/testsuite/lib/scandump.exp index 2e6eebfaf33..4a64ac6e05d 100644 --- a/gcc/testsuite/lib/scandump.exp +++ b/gcc/testsuite/lib/scandump.exp @@ -86,6 +86,7 @@ proc scan-dump-times { args } { } set testcase [testname-for-summary] + set times [lindex $args 2] set suf [dump-suffix [lindex $args 3]] set printable_pattern [make_pattern_printable [lindex $args 1]] set testname "$testcase scan-[lindex $args 0]-dump-times $suf \"$printable_pattern\" [lindex $args 2]" @@ -101,10 +102,11 @@ proc scan-dump-times { args } { set text [read $fd] close $fd - if { [llength [regexp -inline -all -- [lindex $args 1] $text]] == [lindex $args 2]} { + set result_count [llength [regexp -inline -all -- [lindex $args 1] $text]] + if {$result_count == $times} { pass "$testname" } else { - fail "$testname" + fail "$testname (found $result_count times)" } }