public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] testsuite: Generalize check_effective_target_lra
@ 2023-02-07 18:38 Hans-Peter Nilsson
  2023-02-08 16:54 ` Richard Sandiford
  0 siblings, 1 reply; 3+ messages in thread
From: Hans-Peter Nilsson @ 2023-02-07 18:38 UTC (permalink / raw)
  To: gcc-patches; +Cc: vmakarov

Tested native x86_64-pc-linux-gnu and cris-elf (non-LRA and
also hacked to switch to LRA).

Ok to commit?

--- 8< ---
The LRA target list is incomplete.  Rather than syncing it with actual
LRA targets, better use existing infrastructure and look for a
LRA-specific pattern in the reload dump (which has the same name, but
completely different contents).

	* lib/target-supports.exp: Replace target list with looking for
	a LRA-specific string in the reload dump.
---
 gcc/testsuite/lib/target-supports.exp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 227e3004077a..e62b7a2c869d 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -12192,10 +12192,11 @@ proc check_effective_target_o_flag_in_section { } {
 # return 1 if LRA is supported.
 
 proc check_effective_target_lra { } {
-    if { [istarget hppa*-*-*] } {
-	return 0
-    }
-    return 1
+    # Iterating over extended basic blocks is new with LRA.  Also need
+    # a context to avoid spuriously matching a register name.
+    return [check_no_messages_and_pattern lra "EBB 2 3" rtl-reload {
+	void foo (void) { }
+    }]
 }
 
 # Test whether optimizations are enabled ('__OPTIMIZE__') per the
-- 
2.30.2


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

* Re: [PATCH] testsuite: Generalize check_effective_target_lra
  2023-02-07 18:38 [PATCH] testsuite: Generalize check_effective_target_lra Hans-Peter Nilsson
@ 2023-02-08 16:54 ` Richard Sandiford
  2023-02-08 17:24   ` Hans-Peter Nilsson
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Sandiford @ 2023-02-08 16:54 UTC (permalink / raw)
  To: Hans-Peter Nilsson via Gcc-patches; +Cc: Hans-Peter Nilsson, vmakarov

Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Tested native x86_64-pc-linux-gnu and cris-elf (non-LRA and
> also hacked to switch to LRA).

Since !LRA is hopefully not long for this world, I'd personally
prefer to keep it simple & obvious (at least for most targets).
There's a risk that we could lose testing on most targets through
an innocuous-looking change to one of LRA's fprintfs.

If this is for -mlra-like options, I think it would be OK to guard the
new code with a target check.  So the patch is OK with the existing
"return 0" replaced by the new return, and with the istarget list
extended if necessary, but with the outer structure the same.
That might not do what you want though...

Thanks,
Richard

>
> Ok to commit?
>
> --- 8< ---
> The LRA target list is incomplete.  Rather than syncing it with actual
> LRA targets, better use existing infrastructure and look for a
> LRA-specific pattern in the reload dump (which has the same name, but
> completely different contents).
>
> 	* lib/target-supports.exp: Replace target list with looking for
> 	a LRA-specific string in the reload dump.
> ---
>  gcc/testsuite/lib/target-supports.exp | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index 227e3004077a..e62b7a2c869d 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -12192,10 +12192,11 @@ proc check_effective_target_o_flag_in_section { } {
>  # return 1 if LRA is supported.
>  
>  proc check_effective_target_lra { } {
> -    if { [istarget hppa*-*-*] } {
> -	return 0
> -    }
> -    return 1
> +    # Iterating over extended basic blocks is new with LRA.  Also need
> +    # a context to avoid spuriously matching a register name.
> +    return [check_no_messages_and_pattern lra "EBB 2 3" rtl-reload {
> +	void foo (void) { }
> +    }]
>  }
>  
>  # Test whether optimizations are enabled ('__OPTIMIZE__') per the

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

* Re: [PATCH] testsuite: Generalize check_effective_target_lra
  2023-02-08 16:54 ` Richard Sandiford
@ 2023-02-08 17:24   ` Hans-Peter Nilsson
  0 siblings, 0 replies; 3+ messages in thread
From: Hans-Peter Nilsson @ 2023-02-08 17:24 UTC (permalink / raw)
  To: Richard Sandiford; +Cc: gcc-patches, vmakarov

> From: Richard Sandiford <richard.sandiford@arm.com>
> Date: Wed, 8 Feb 2023 17:54:15 +0100

> Hans-Peter Nilsson via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > Tested native x86_64-pc-linux-gnu and cris-elf (non-LRA and
> > also hacked to switch to LRA).
> 
> Since !LRA is hopefully not long for this world, I'd personally
> prefer to keep it simple & obvious (at least for most targets).
> There's a risk that we could lose testing on most targets through
> an innocuous-looking change to one of LRA's fprintfs.
> 
> If this is for -mlra-like options, I think it would be OK to guard the
> new code with a target check.  So the patch is OK with the existing
> "return 0" replaced by the new return, and with the istarget list
> extended if necessary, but with the outer structure the same.

If you, like me, are aiming for simple and obvious, why then
complicate what already works "for -mlra-like options" with
a target list?

> That might not do what you want though...

That assessment is correct.  I'd rather the test-suite
result be consistent for all targets.

brgds, H-P
PS. thanks for the review though

> 
> Thanks,
> Richard
> 
> >
> > Ok to commit?
> >
> > --- 8< ---
> > The LRA target list is incomplete.  Rather than syncing it with actual
> > LRA targets, better use existing infrastructure and look for a
> > LRA-specific pattern in the reload dump (which has the same name, but
> > completely different contents).
> >
> > 	* lib/target-supports.exp: Replace target list with looking for
> > 	a LRA-specific string in the reload dump.
> > ---
> >  gcc/testsuite/lib/target-supports.exp | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> > index 227e3004077a..e62b7a2c869d 100644
> > --- a/gcc/testsuite/lib/target-supports.exp
> > +++ b/gcc/testsuite/lib/target-supports.exp
> > @@ -12192,10 +12192,11 @@ proc check_effective_target_o_flag_in_section { } {
> >  # return 1 if LRA is supported.
> >  
> >  proc check_effective_target_lra { } {
> > -    if { [istarget hppa*-*-*] } {
> > -	return 0
> > -    }
> > -    return 1
> > +    # Iterating over extended basic blocks is new with LRA.  Also need
> > +    # a context to avoid spuriously matching a register name.
> > +    return [check_no_messages_and_pattern lra "EBB 2 3" rtl-reload {
> > +	void foo (void) { }
> > +    }]
> >  }
> >  
> >  # Test whether optimizations are enabled ('__OPTIMIZE__') per the
> 

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

end of thread, other threads:[~2023-02-08 17:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-07 18:38 [PATCH] testsuite: Generalize check_effective_target_lra Hans-Peter Nilsson
2023-02-08 16:54 ` Richard Sandiford
2023-02-08 17:24   ` Hans-Peter Nilsson

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).