diff mbox series

testsuite: Generalize check_effective_target_lra

Message ID 20230207183813.978782042C@pchp3.se.axis.com
State Accepted
Headers
Series testsuite: Generalize check_effective_target_lra |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Hans-Peter Nilsson Feb. 7, 2023, 6:38 p.m. UTC 
  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(-)

Comments

Richard Sandiford Feb. 8, 2023, 4:54 p.m. UTC | #1 
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
Hans-Peter Nilsson Feb. 8, 2023, 5:24 p.m. UTC | #2 
> 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
>
diff mbox series

Patch

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