public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] PR52665 do not let .ident confuse assembler scan tests
@ 2016-06-18 19:31 Bernhard Reutner-Fischer
  2016-06-18 23:49 ` Hans-Peter Nilsson
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Bernhard Reutner-Fischer @ 2016-06-18 19:31 UTC (permalink / raw)
  To: gcc-patches; +Cc: Bernhard Reutner-Fischer, Mike Stump, Rainer Orth

A branch with a name matching scan-assembler pattern triggers
inappropriate FAIL.

E.g. branch fixups-testsuite and
- gcc.target/i386/pr65871-?.c (scan-assembler-not "test")
- gcc.target/i386/pr41442.c (scan-assembler-times "test|cmp" 2)
etc.

This is a recurring problem as can be seen by some -fno-ident additions
by commits from e.g. Michael Meissner over the years: builtins-58.c,
powerpc/pr46728-?.c

The patch below adds -fno-ident if a testcase contains one of
scan-assembler, scan-assembler-not or scan-assembler-times.

Regression tested on x86_64-unknown-linux on a fixups-testsuite branch
where it fixes several false FAILs without regressions.

gcc/testsuite/ChangeLog

2016-06-18  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>

	PR testsuite/52665
	* lib/gcc-dg.exp (gcc-dg-test-1): Iterate over _required_options.
	* lib/target-supports.exp (scan-assembler_required_options,
	scan-assembler-not_required_options,
	scan-assembler-times_required_options): Add -fno-ident.
	* lib/scanasm.exp (scan-assembler-times): Fix error message.
	* c-c++-common/ident-0a.c: New test.
	* c-c++-common/ident-0b.c: New test.
	* c-c++-common/ident-1a.c: New test.
	* c-c++-common/ident-1b.c: New test.
	* c-c++-common/ident-2a.c: New test.
	* c-c++-common/ident-2b.c: New test.

Ok for trunk?

PS: proc force_conventional_output_for would be a bit misnomed by this,
not sure if it should be renamed to maybe set_required_options_for or
the like?

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
Cc: Mike Stump <mikestump@comcast.net>
Cc: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
---
 gcc/testsuite/c-c++-common/ident-0a.c |  6 ++++++
 gcc/testsuite/c-c++-common/ident-0b.c | 10 ++++++++++
 gcc/testsuite/c-c++-common/ident-1a.c |  8 ++++++++
 gcc/testsuite/c-c++-common/ident-1b.c |  7 +++++++
 gcc/testsuite/c-c++-common/ident-2a.c |  6 ++++++
 gcc/testsuite/c-c++-common/ident-2b.c |  7 +++++++
 gcc/testsuite/lib/gcc-dg.exp          |  8 +++++---
 gcc/testsuite/lib/scanasm.exp         |  4 ++--
 gcc/testsuite/lib/target-supports.exp |  6 ++++++
 9 files changed, 57 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/ident-0a.c
 create mode 100644 gcc/testsuite/c-c++-common/ident-0b.c
 create mode 100644 gcc/testsuite/c-c++-common/ident-1a.c
 create mode 100644 gcc/testsuite/c-c++-common/ident-1b.c
 create mode 100644 gcc/testsuite/c-c++-common/ident-2a.c
 create mode 100644 gcc/testsuite/c-c++-common/ident-2b.c

diff --git a/gcc/testsuite/c-c++-common/ident-0a.c b/gcc/testsuite/c-c++-common/ident-0a.c
new file mode 100644
index 0000000..900d206
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ident-0a.c
@@ -0,0 +1,6 @@
+/* PR testsuite/52665
+ * Make sure scan-assembler-not turns off .ident  */
+/* { dg-do compile } */
+int i;
+
+/* { dg-final { scan-assembler-not "GCC: " } } */
diff --git a/gcc/testsuite/c-c++-common/ident-0b.c b/gcc/testsuite/c-c++-common/ident-0b.c
new file mode 100644
index 0000000..e08126d
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ident-0b.c
@@ -0,0 +1,10 @@
+/* PR testsuite/52665
+ * Make sure scan-assembler-not turns off .ident unless -fident in testcase */
+/* { dg-do compile } */
+/* { dg-options "-fident" } */
+int i;
+
+/* { dg-final { scan-assembler-not "GCC: " { xfail *-*-* } } } */
+/* The testsuite saw scan-assembler-not and turned off .ident so the above
+ * has to fail for proper operation since the testsuite itself forced
+ * -fident on again.  */
diff --git a/gcc/testsuite/c-c++-common/ident-1a.c b/gcc/testsuite/c-c++-common/ident-1a.c
new file mode 100644
index 0000000..867ee43
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ident-1a.c
@@ -0,0 +1,8 @@
+/* PR testsuite/52665
+ * Make sure scan-assembler turns off .ident  */
+/* { dg-do compile } */
+int i;
+
+/* { dg-final { scan-assembler "GCC: " { xfail *-*-* } } } */
+/* The testsuite saw scan-assembler and turned off .ident so the above
+ * has to fail for proper operation.  */
diff --git a/gcc/testsuite/c-c++-common/ident-1b.c b/gcc/testsuite/c-c++-common/ident-1b.c
new file mode 100644
index 0000000..2431086
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ident-1b.c
@@ -0,0 +1,7 @@
+/* PR testsuite/52665
+ * Make sure scan-assembler turns off .ident unless -fident in testcase */
+/* { dg-do compile } */
+/* { dg-options "-fident" } */
+int i;
+
+/* { dg-final { scan-assembler "GCC: " } } */
diff --git a/gcc/testsuite/c-c++-common/ident-2a.c b/gcc/testsuite/c-c++-common/ident-2a.c
new file mode 100644
index 0000000..131b867
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ident-2a.c
@@ -0,0 +1,6 @@
+/* PR testsuite/52665
+ * Make sure scan-assembler-times turns off .ident  */
+/* { dg-do compile } */
+int i;
+
+/* { dg-final { scan-assembler-times "GCC: " 0 } } */ /* internal test, keep -times 0 ! */
diff --git a/gcc/testsuite/c-c++-common/ident-2b.c b/gcc/testsuite/c-c++-common/ident-2b.c
new file mode 100644
index 0000000..a21e25f
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/ident-2b.c
@@ -0,0 +1,7 @@
+/* PR testsuite/52665
+ * Make sure scan-assembler-times turns off .ident unless -fident in testcase */
+/* { dg-do compile } */
+/* { dg-options "-fident" } */
+int ident;
+
+/* { dg-final { scan-assembler-times "GCC: " 1 } } */
diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp
index 7039140..081b1a8 100644
--- a/gcc/testsuite/lib/gcc-dg.exp
+++ b/gcc/testsuite/lib/gcc-dg.exp
@@ -270,9 +270,11 @@ proc gcc-dg-test-1 { target_compile prog do_what extra_tool_flags } {
     foreach x [split $finalcode "\n"] {
 	set finalcmd [lindex $x 0]
 	if { [info procs ${finalcmd}_required_options] != "" } {
-	    set req [${finalcmd}_required_options]
-	    if { $req != "" && [lsearch -exact $extra_tool_flags $req] == -1 } {
-		lappend extra_tool_flags $req
+	    foreach req [${finalcmd}_required_options] {
+		if { $req != ""
+		     && [lsearch -exact $extra_tool_flags $req] == -1 } {
+		    lappend extra_tool_flags $req
+		}
 	    }
 	}
     }
diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp
index 07b8f7d..a368474 100644
--- a/gcc/testsuite/lib/scanasm.exp
+++ b/gcc/testsuite/lib/scanasm.exp
@@ -212,11 +212,11 @@ proc scan-ada-spec-not { args } {
 # Call pass if pattern is present given number of times, otherwise fail.
 proc scan-assembler-times { args } {
     if { [llength $args] < 2 } {
-	error "scan-assembler: too few arguments"
+	error "scan-assembler-times: too few arguments"
         return
     }
     if { [llength $args] > 3 } {
-	error "scan-assembler: too many arguments"
+	error "scan-assembler-times: too many arguments"
 	return
     }
     if { [llength $args] >= 3 } {
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index ef5271a..d9ac988 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -6910,6 +6910,12 @@ proc force_conventional_output_for { test } {
     }
     proc ${test}_required_options {} {
 	global gcc_force_conventional_output
+	upvar 1 extra_tool_flags extra_tool_flags
+	if {[regexp -- "^scan-assembler" [info level 0]]
+	    && ![string match "*-fident*" $extra_tool_flags]} {
+	    # Do not let .ident confuse assembler scan tests
+	    return [list $gcc_force_conventional_output "-fno-ident"]
+	}
 	return $gcc_force_conventional_output
     }
 }
-- 
2.8.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests
  2016-06-18 19:31 [PATCH] PR52665 do not let .ident confuse assembler scan tests Bernhard Reutner-Fischer
@ 2016-06-18 23:49 ` Hans-Peter Nilsson
  2016-06-19 20:21 ` Mike Stump
  2016-06-20 22:20 ` [PATCH] PR52665 do not let .ident confuse assembler scan tests Jeff Law
  2 siblings, 0 replies; 10+ messages in thread
From: Hans-Peter Nilsson @ 2016-06-18 23:49 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer; +Cc: gcc-patches, Mike Stump, Rainer Orth

On Sat, 18 Jun 2016, Bernhard Reutner-Fischer wrote:
> A branch with a name matching scan-assembler pattern triggers
> inappropriate FAIL.
>
> E.g. branch fixups-testsuite and
> - gcc.target/i386/pr65871-?.c (scan-assembler-not "test")
> - gcc.target/i386/pr41442.c (scan-assembler-times "test|cmp" 2)
> etc.
>
> This is a recurring problem as can be seen by some -fno-ident additions
> by commits from e.g. Michael Meissner over the years: builtins-58.c,
> powerpc/pr46728-?.c
>
> The patch below adds -fno-ident if a testcase contains one of
> scan-assembler, scan-assembler-not or scan-assembler-times.

This reminds me of a related but not opposing idea that I have
never acted on (besides bouncing it off a global maintainer
quite some time ago, with a positive response), to make common
identifier-like and filename-like strings invalid for use in
scan-assembler; there'd have to be a meta-character such as
whitespace.  It would help for "test" but admittedly not for
"test|cmp" given e.g. an objdir named "/tmp/mytest/gccmp/nop1".

Then there'd be an error thrown for such test-cases, alerting
the author (well, hopefully) that the test-case is in error, and
a comment in the scan-assembler code would make it prominent
that there'd have to be a whitespace or meta-like character
(i.e. [^-a-zA-Z_\.]) included in the string.  Yes, I know
whitespace can be included in filenames and that's regularly
done at least on some systems, but I bet you really get into
problems trying that for srcdir or objdir in a gcc build.

Sorry, still not going to act on it for now, but feel free if so
inclined.

brgds, H-P

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests
  2016-06-18 19:31 [PATCH] PR52665 do not let .ident confuse assembler scan tests Bernhard Reutner-Fischer
  2016-06-18 23:49 ` Hans-Peter Nilsson
@ 2016-06-19 20:21 ` Mike Stump
  2018-02-02 13:25   ` Bernhard Reutner-Fischer
  2016-06-20 22:20 ` [PATCH] PR52665 do not let .ident confuse assembler scan tests Jeff Law
  2 siblings, 1 reply; 10+ messages in thread
From: Mike Stump @ 2016-06-19 20:21 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer; +Cc: gcc-patches, Rainer Orth

On Jun 18, 2016, at 12:31 PM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> 
> A branch with a name matching scan-assembler pattern triggers
> inappropriate FAIL.

> The patch below adds -fno-ident if a testcase contains one of
> scan-assembler, scan-assembler-not or scan-assembler-times.

Kinda gross.  I'd like to consensus build a little, as I don't know that I have a better solution than the solution you propose to fix the issue.  I'd love it if one or more of Jacob, Law and Richard can chime in on direction here.  I'll have to think about this some more and see if I can come up with something that I like better.

If no one has a better solution, I'll approve the proposed solution.  Let's give people a little time to chime in.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests
  2016-06-18 19:31 [PATCH] PR52665 do not let .ident confuse assembler scan tests Bernhard Reutner-Fischer
  2016-06-18 23:49 ` Hans-Peter Nilsson
  2016-06-19 20:21 ` Mike Stump
@ 2016-06-20 22:20 ` Jeff Law
  2018-09-05 15:32   ` Bernhard Reutner-Fischer
  2 siblings, 1 reply; 10+ messages in thread
From: Jeff Law @ 2016-06-20 22:20 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer, gcc-patches; +Cc: Mike Stump, Rainer Orth

On 06/18/2016 01:31 PM, Bernhard Reutner-Fischer wrote:
> A branch with a name matching scan-assembler pattern triggers
> inappropriate FAIL.
>
> E.g. branch fixups-testsuite and
> - gcc.target/i386/pr65871-?.c (scan-assembler-not "test")
> - gcc.target/i386/pr41442.c (scan-assembler-times "test|cmp" 2)
> etc.
>
> This is a recurring problem as can be seen by some -fno-ident additions
> by commits from e.g. Michael Meissner over the years: builtins-58.c,
> powerpc/pr46728-?.c
>
> The patch below adds -fno-ident if a testcase contains one of
> scan-assembler, scan-assembler-not or scan-assembler-times.
>
> Regression tested on x86_64-unknown-linux on a fixups-testsuite branch
> where it fixes several false FAILs without regressions.
>
> gcc/testsuite/ChangeLog
>
> 2016-06-18  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>
>
> 	PR testsuite/52665
> 	* lib/gcc-dg.exp (gcc-dg-test-1): Iterate over _required_options.
> 	* lib/target-supports.exp (scan-assembler_required_options,
> 	scan-assembler-not_required_options,
> 	scan-assembler-times_required_options): Add -fno-ident.
> 	* lib/scanasm.exp (scan-assembler-times): Fix error message.
> 	* c-c++-common/ident-0a.c: New test.
> 	* c-c++-common/ident-0b.c: New test.
> 	* c-c++-common/ident-1a.c: New test.
> 	* c-c++-common/ident-1b.c: New test.
> 	* c-c++-common/ident-2a.c: New test.
> 	* c-c++-common/ident-2b.c: New test.
>
> Ok for trunk?
>
> PS: proc force_conventional_output_for would be a bit misnomed by this,
> not sure if it should be renamed to maybe set_required_options_for or
> the like?
OK.

Changing force_conventional_output to set_required_options_for is 
pre-approved as well.

jeff

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests
  2016-06-19 20:21 ` Mike Stump
@ 2018-02-02 13:25   ` Bernhard Reutner-Fischer
  2018-02-02 18:56     ` Mike Stump
                       ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Bernhard Reutner-Fischer @ 2018-02-02 13:25 UTC (permalink / raw)
  To: Mike Stump; +Cc: GCC Patches, Rainer Orth, Jeff Law

On 19 June 2016 at 22:21, Mike Stump <mikestump@comcast.net> wrote:
> On Jun 18, 2016, at 12:31 PM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>>
>> A branch with a name matching scan-assembler pattern triggers
>> inappropriate FAIL.
>
>> The patch below adds -fno-ident if a testcase contains one of
>> scan-assembler, scan-assembler-not or scan-assembler-times.
>
> Kinda gross.  I'd like to consensus build a little, as I don't know that I have a better solution than the solution you propose to fix the issue.  I'd love it if one or more of Jacob, Law and Richard can chime in on direction here.  I'll have to think about this some more and see if I can come up with something that I like better.
>
> If no one has a better solution, I'll approve the proposed solution.  Let's give people a little time to chime in.

Given the overwhelming silence this proposal has received, i take it
for granted that folks are thrilled and even up until now speechless
:)

So how should we proceed with -fno-ident.
And, btw, -fno-file to inhibit .file directives for the same reason,
https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01785.html and all our
ugly filenames like gcc.target/powerpc/swps-p8-36.c which strive to
workaround the assembler-scans.

All those non-automatic hacks like naming swap() tests swps-,
uglifying scan-patterns (which are not terribly straight forward to
read anyway even without uglifying them) are error prone and costly.
IMHO we should make sure time is spent for useful stuff and not be
wasted to fight our very own testsuite.

-fno-ident ok for stage1?
What about -fno-file? Clever alternative suggestions? Don't care?

thanks,

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests
  2018-02-02 13:25   ` Bernhard Reutner-Fischer
@ 2018-02-02 18:56     ` Mike Stump
  2018-05-01 17:47     ` Jeff Law
  2022-04-25 23:00     ` testsuite -fno-file [was: Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests] Bernhard Reutner-Fischer
  2 siblings, 0 replies; 10+ messages in thread
From: Mike Stump @ 2018-02-02 18:56 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer; +Cc: GCC Patches, Rainer Orth, Jeff Law

On Feb 2, 2018, at 5:25 AM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> 
> Given the overwhelming silence this proposal has received, i take it
> for granted that folks are thrilled and even up until now speechless
> :)

> -fno-ident ok for stage1?
> What about -fno-file? Clever alternative suggestions? Don't care?

Sure.  We've had the bake time on the idea, no better solution has been proposed.  I think _solving_ the problem is a good thing, and that solution seems reasonable.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests
  2018-02-02 13:25   ` Bernhard Reutner-Fischer
  2018-02-02 18:56     ` Mike Stump
@ 2018-05-01 17:47     ` Jeff Law
  2022-04-25 23:00     ` testsuite -fno-file [was: Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests] Bernhard Reutner-Fischer
  2 siblings, 0 replies; 10+ messages in thread
From: Jeff Law @ 2018-05-01 17:47 UTC (permalink / raw)
  To: Bernhard Reutner-Fischer, Mike Stump; +Cc: GCC Patches, Rainer Orth

On 02/02/2018 06:25 AM, Bernhard Reutner-Fischer wrote:
> On 19 June 2016 at 22:21, Mike Stump <mikestump@comcast.net> wrote:
>> On Jun 18, 2016, at 12:31 PM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
>>>
>>> A branch with a name matching scan-assembler pattern triggers
>>> inappropriate FAIL.
>>
>>> The patch below adds -fno-ident if a testcase contains one of
>>> scan-assembler, scan-assembler-not or scan-assembler-times.
>>
>> Kinda gross.  I'd like to consensus build a little, as I don't know that I have a better solution than the solution you propose to fix the issue.  I'd love it if one or more of Jacob, Law and Richard can chime in on direction here.  I'll have to think about this some more and see if I can come up with something that I like better.
>>
>> If no one has a better solution, I'll approve the proposed solution.  Let's give people a little time to chime in.
> 
> Given the overwhelming silence this proposal has received, i take it
> for granted that folks are thrilled and even up until now speechless
> :)
> 
> So how should we proceed with -fno-ident.
> And, btw, -fno-file to inhibit .file directives for the same reason,
> https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01785.html and all our
> ugly filenames like gcc.target/powerpc/swps-p8-36.c which strive to
> workaround the assembler-scans.
> 
> All those non-automatic hacks like naming swap() tests swps-,
> uglifying scan-patterns (which are not terribly straight forward to
> read anyway even without uglifying them) are error prone and costly.
> IMHO we should make sure time is spent for useful stuff and not be
> wasted to fight our very own testsuite.
> 
> -fno-ident ok for stage1?
> What about -fno-file? Clever alternative suggestions? Don't care?
I thought I ack'd this back in 2016? :-)

jeff

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests
  2016-06-20 22:20 ` [PATCH] PR52665 do not let .ident confuse assembler scan tests Jeff Law
@ 2018-09-05 15:32   ` Bernhard Reutner-Fischer
  2023-05-31 18:50     ` Bernhard Reutner-Fischer
  0 siblings, 1 reply; 10+ messages in thread
From: Bernhard Reutner-Fischer @ 2018-09-05 15:32 UTC (permalink / raw)
  To: Jeff Law; +Cc: GCC Patches, Mike Stump, Rainer Orth

On Tue, 21 Jun 2016 at 00:19, Jeff Law <law@redhat.com> wrote:
>
> On 06/18/2016 01:31 PM, Bernhard Reutner-Fischer wrote:
> > A branch with a name matching scan-assembler pattern triggers
> > inappropriate FAIL.
> >
> > E.g. branch fixups-testsuite and
> > - gcc.target/i386/pr65871-?.c (scan-assembler-not "test")
> > - gcc.target/i386/pr41442.c (scan-assembler-times "test|cmp" 2)
> > etc.
> >
> > This is a recurring problem as can be seen by some -fno-ident additions
> > by commits from e.g. Michael Meissner over the years: builtins-58.c,
> > powerpc/pr46728-?.c
> >
> > The patch below adds -fno-ident if a testcase contains one of
> > scan-assembler, scan-assembler-not or scan-assembler-times.
> >
> > Regression tested on x86_64-unknown-linux on a fixups-testsuite branch
> > where it fixes several false FAILs without regressions.
> >
> > gcc/testsuite/ChangeLog
> >
> > 2016-06-18  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>
> >
> >       PR testsuite/52665
> >       * lib/gcc-dg.exp (gcc-dg-test-1): Iterate over _required_options.
> >       * lib/target-supports.exp (scan-assembler_required_options,
> >       scan-assembler-not_required_options,
> >       scan-assembler-times_required_options): Add -fno-ident.
> >       * lib/scanasm.exp (scan-assembler-times): Fix error message.
> >       * c-c++-common/ident-0a.c: New test.
> >       * c-c++-common/ident-0b.c: New test.
> >       * c-c++-common/ident-1a.c: New test.
> >       * c-c++-common/ident-1b.c: New test.
> >       * c-c++-common/ident-2a.c: New test.
> >       * c-c++-common/ident-2b.c: New test.
> >
> > Ok for trunk?
> >
> > PS: proc force_conventional_output_for would be a bit misnomed by this,
> > not sure if it should be renamed to maybe set_required_options_for or
> > the like?
> OK.

Now applied without the rename to trunk as r264128.

thanks,

>
> Changing force_conventional_output to set_required_options_for is
> pre-approved as well.
>
> jeff
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* testsuite -fno-file [was: Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests]
  2018-02-02 13:25   ` Bernhard Reutner-Fischer
  2018-02-02 18:56     ` Mike Stump
  2018-05-01 17:47     ` Jeff Law
@ 2022-04-25 23:00     ` Bernhard Reutner-Fischer
  2 siblings, 0 replies; 10+ messages in thread
From: Bernhard Reutner-Fischer @ 2022-04-25 23:00 UTC (permalink / raw)
  To: Mike Stump; +Cc: rep.dot.nop, GCC Patches, Rainer Orth, Jeff Law

On Fri, 2 Feb 2018 14:25:22 +0100
Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:

> On 19 June 2016 at 22:21, Mike Stump <mikestump@comcast.net> wrote:
> > On Jun 18, 2016, at 12:31 PM, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:  
> >>
> >> A branch with a name matching scan-assembler pattern triggers
> >> inappropriate FAIL.  
> >  
> >> The patch below adds -fno-ident if a testcase contains one of
> >> scan-assembler, scan-assembler-not or scan-assembler-times.  
> >
> > Kinda gross.  I'd like to consensus build a little, as I don't know that I have a better solution than the solution you propose to fix the issue.  I'd love it if one or more of Jacob, Law and Richard can chime in on direction here.  I'll have to think about this some more and see if I can come up with something that I like better.
> >
> > If no one has a better solution, I'll approve the proposed solution.  Let's give people a little time to chime in.  
> 
> Given the overwhelming silence this proposal has received, i take it
> for granted that folks are thrilled and even up until now speechless
> :)
> 
> So how should we proceed with -fno-ident.
> And, btw, -fno-file to inhibit .file directives for the same reason,

AFAICS we still do not pass -fno-file in tests.
Every now and then this still results in unnecessary, silly workarounds
like this one:

> https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01785.html and all our

which we should not have to afford.

> ugly filenames like gcc.target/powerpc/swps-p8-36.c which strive to
> workaround the assembler-scans.
> 
> All those non-automatic hacks like naming swap() tests swps-,
> uglifying scan-patterns (which are not terribly straight forward to
> read anyway even without uglifying them) are error prone and costly.
> IMHO we should make sure time is spent for useful stuff and not be
> wasted to fight our very own testsuite.
> 
> -fno-ident ok for stage1?

We've fixed the ident thing some time ago.

> What about -fno-file? Clever alternative suggestions? Don't care?

We still didn't fix the .file directives in assembler output.

It's about the same as always.
See if the target supports -fno-file and/or -ffile.
If the testcase does NOT specify -fno-file or -ffile ¹)
then pass an appropriate default like -fno-file, otherwise do not add
anything.

¹) The real complication seems to be that there is neither a -fno-file
nor a (let's say) -ffile=foo.c implemented anywhere. And TBH we do
not need it nor want it for the purpose at hand. That suggests
that it would probably be cheaper to run sed on the (remote) output
file -- or do it locally and only then copy it to the remote. The net
effect being that .file directives in the testsuite are gone and can no
longer compromise tests.

thanks,

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests
  2018-09-05 15:32   ` Bernhard Reutner-Fischer
@ 2023-05-31 18:50     ` Bernhard Reutner-Fischer
  0 siblings, 0 replies; 10+ messages in thread
From: Bernhard Reutner-Fischer @ 2023-05-31 18:50 UTC (permalink / raw)
  To: Jeff Law; +Cc: rep.dot.nop, GCC Patches, Mike Stump, Rainer Orth

On Wed, 5 Sep 2018 17:32:04 +0200
Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:

> On Tue, 21 Jun 2016 at 00:19, Jeff Law <law@redhat.com> wrote:
> >
> > On 06/18/2016 01:31 PM, Bernhard Reutner-Fischer wrote:  

> > > gcc/testsuite/ChangeLog
> > >
> > > 2016-06-18  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>
> > >
> > >       PR testsuite/52665
> > >       * lib/gcc-dg.exp (gcc-dg-test-1): Iterate over _required_options.
> > >       * lib/target-supports.exp (scan-assembler_required_options,
> > >       scan-assembler-not_required_options,
> > >       scan-assembler-times_required_options): Add -fno-ident.
> > >       * lib/scanasm.exp (scan-assembler-times): Fix error message.
> > >       * c-c++-common/ident-0a.c: New test.
> > >       * c-c++-common/ident-0b.c: New test.
> > >       * c-c++-common/ident-1a.c: New test.
> > >       * c-c++-common/ident-1b.c: New test.
> > >       * c-c++-common/ident-2a.c: New test.
> > >       * c-c++-common/ident-2b.c: New test.
> > >
> > > Ok for trunk?
> > >
> > > PS: proc force_conventional_output_for would be a bit misnomed by this,
> > > not sure if it should be renamed to maybe set_required_options_for or
> > > the like?  
> > OK.  
> 
> Now applied without the rename to trunk as r264128.
> 
> thanks,
> 
> >
> > Changing force_conventional_output to set_required_options_for is
> > pre-approved as well.

I've now applied the renaming as r14-1449-g994195b597ff20
thanks,

> >
> > jeff
> >  


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-05-31 18:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-18 19:31 [PATCH] PR52665 do not let .ident confuse assembler scan tests Bernhard Reutner-Fischer
2016-06-18 23:49 ` Hans-Peter Nilsson
2016-06-19 20:21 ` Mike Stump
2018-02-02 13:25   ` Bernhard Reutner-Fischer
2018-02-02 18:56     ` Mike Stump
2018-05-01 17:47     ` Jeff Law
2022-04-25 23:00     ` testsuite -fno-file [was: Re: [PATCH] PR52665 do not let .ident confuse assembler scan tests] Bernhard Reutner-Fischer
2016-06-20 22:20 ` [PATCH] PR52665 do not let .ident confuse assembler scan tests Jeff Law
2018-09-05 15:32   ` Bernhard Reutner-Fischer
2023-05-31 18:50     ` Bernhard Reutner-Fischer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).