diff mbox series

arm: [testuiste] fix ivopts.c target test [PR96372]

Message ID gkro8hk7sux.fsf@arm.com
State New
Headers show
Series arm: [testuiste] fix ivopts.c target test [PR96372] | expand

Commit Message

Andrea Corallo Jan. 19, 2021, 5:13 p.m. UTC
Hi all,

this patch is for PR96372, the fail was introduced by [1] where the
failing check went from using target 'arm_thumb2' to
'arm_thumb2_ok_no_arm_v8_1_lob'.  Unfortunately this is relying on
'arm_thumb2_ok' that has a different semantic compared to the original
'arm_thumb2'.

This patch is introducing then 'arm_thumb2_no_arm_v8_1_lob' relying on
'arm_thumb2' to restore the intended behavior.

Okay for trunk?

Thanks

  Andrea

[1] <https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=d2ed233cb940aa3eecc163d98b47979dd81dbc0a>
From 6199695364808c59202dfcb1b29df1e84dab5aa9 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Fri, 15 Jan 2021 15:34:19 +0100
Subject: [PATCH] arm: [testuiste] fix ivopts.c target test [PR96372]

gcc/
2021-01-15  Andrea Corallo  <andrea.corallo@arm.com>
	PR target/96372
	* doc/sourcebuild.texi (arm_thumb2_no_arm_v8_1_lob): Document.

gcc/testsuite/
2021-01-15  Andrea Corallo  <andrea.corallo@arm.com>
	PR target/96372
	* lib/target-supports.exp
	(check_effective_target_arm_thumb2_no_arm_v8_1_lob): Define proc.
	* gcc.target/arm/ivopts.c: Use target
	'arm_thumb2_no_arm_v8_1_lob'.
---
 gcc/doc/sourcebuild.texi              |  5 +++++
 gcc/testsuite/gcc.target/arm/ivopts.c |  2 +-
 gcc/testsuite/lib/target-supports.exp | 15 ++++++++++++++-
 3 files changed, 20 insertions(+), 2 deletions(-)

Comments

Kyrylo Tkachov Jan. 21, 2021, 9:13 a.m. UTC | #1
> -----Original Message-----
> From: Andrea Corallo <Andrea.Corallo@arm.com>
> Sent: 19 January 2021 17:13
> To: gcc-patches@gcc.gnu.org
> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; nd <nd@arm.com>;
> christophe.lyon@linaro.org
> Subject: [PATCH] arm: [testuiste] fix ivopts.c target test [PR96372]
> 
> Hi all,
> 
> this patch is for PR96372, the fail was introduced by [1] where the
> failing check went from using target 'arm_thumb2' to
> 'arm_thumb2_ok_no_arm_v8_1_lob'.  Unfortunately this is relying on
> 'arm_thumb2_ok' that has a different semantic compared to the original
> 'arm_thumb2'.
> 
> This patch is introducing then 'arm_thumb2_no_arm_v8_1_lob' relying on
> 'arm_thumb2' to restore the intended behavior.
> 
> Okay for trunk?
> 

We usually try to avoid having such negative options (target-no-feature). Dejagnu can use "!" to indicate negation of a target, can you use that?

Thanks,
Kyrill

> Thanks
> 
>   Andrea
> 
> [1]
> <https://gcc.gnu.org/git/gitweb.cgi?p=gcc.git;h=d2ed233cb940aa3eecc163d
> 98b47979dd81dbc0a>
Andrea Corallo Jan. 21, 2021, 11:12 a.m. UTC | #2
Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes:

>> -----Original Message-----
>> From: Andrea Corallo <Andrea.Corallo@arm.com>
>> Sent: 19 January 2021 17:13
>> To: gcc-patches@gcc.gnu.org
>> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard Earnshaw
>> <Richard.Earnshaw@arm.com>; nd <nd@arm.com>;
>> christophe.lyon@linaro.org
>> Subject: [PATCH] arm: [testuiste] fix ivopts.c target test [PR96372]
>> 
>> Hi all,
>> 
>> this patch is for PR96372, the fail was introduced by [1] where the
>> failing check went from using target 'arm_thumb2' to
>> 'arm_thumb2_ok_no_arm_v8_1_lob'.  Unfortunately this is relying on
>> 'arm_thumb2_ok' that has a different semantic compared to the original
>> 'arm_thumb2'.
>> 
>> This patch is introducing then 'arm_thumb2_no_arm_v8_1_lob' relying on
>> 'arm_thumb2' to restore the intended behavior.
>> 
>> Okay for trunk?
>> 
>
> We usually try to avoid having such negative options (target-no-feature). Dejagnu can use "!" to indicate negation of a target, can you use that?
>
> Thanks,
> Kyrill

Hi Kyrill,

thanks for reviewing. 

Not sure I get the suggestion: here 'arm_thumb2_no_arm_v8_1_lob' stands
for 'arm_thumb2' positive and 'arm_v8_1_lob' negative.  Similarly we
already have 'target_arm_thumb2_ok_no_arm_v8_1_lob'.

Am I missing something?

  Andrea
Kyrylo Tkachov Jan. 21, 2021, 11:16 a.m. UTC | #3
> -----Original Message-----
> From: Andrea Corallo <Andrea.Corallo@arm.com>
> Sent: 21 January 2021 11:13
> To: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>
> Cc: gcc-patches@gcc.gnu.org; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; nd <nd@arm.com>;
> christophe.lyon@linaro.org
> Subject: Re: [PATCH] arm: [testuiste] fix ivopts.c target test [PR96372]
> 
> Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes:
> 
> >> -----Original Message-----
> >> From: Andrea Corallo <Andrea.Corallo@arm.com>
> >> Sent: 19 January 2021 17:13
> >> To: gcc-patches@gcc.gnu.org
> >> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard Earnshaw
> >> <Richard.Earnshaw@arm.com>; nd <nd@arm.com>;
> >> christophe.lyon@linaro.org
> >> Subject: [PATCH] arm: [testuiste] fix ivopts.c target test [PR96372]
> >>
> >> Hi all,
> >>
> >> this patch is for PR96372, the fail was introduced by [1] where the
> >> failing check went from using target 'arm_thumb2' to
> >> 'arm_thumb2_ok_no_arm_v8_1_lob'.  Unfortunately this is relying on
> >> 'arm_thumb2_ok' that has a different semantic compared to the original
> >> 'arm_thumb2'.
> >>
> >> This patch is introducing then 'arm_thumb2_no_arm_v8_1_lob' relying on
> >> 'arm_thumb2' to restore the intended behavior.
> >>
> >> Okay for trunk?
> >>
> >
> > We usually try to avoid having such negative options (target-no-feature).
> Dejagnu can use "!" to indicate negation of a target, can you use that?
> >
> > Thanks,
> > Kyrill
> 
> Hi Kyrill,
> 
> thanks for reviewing.
> 
> Not sure I get the suggestion: here 'arm_thumb2_no_arm_v8_1_lob' stands
> for 'arm_thumb2' positive and 'arm_v8_1_lob' negative.  Similarly we
> already have 'target_arm_thumb2_ok_no_arm_v8_1_lob'.
> 

Ah ok, then it's fine to be consistent.
The patch is ok.
Thanks,
Kyrill

> Am I missing something?
> 
>   Andrea
Andrea Corallo Jan. 21, 2021, 1:37 p.m. UTC | #4
Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes:

[...]

> Ah ok, then it's fine to be consistent.
> The patch is ok.
> Thanks,
> Kyrill

Thanks, into master as 0568f801eff

  Andrea
diff mbox series

Patch

diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
index 3d0873dd074..d5cfe54304d 100644
--- a/gcc/doc/sourcebuild.texi
+++ b/gcc/doc/sourcebuild.texi
@@ -2058,6 +2058,11 @@  ARM Target supports executing the Armv8.1-M Mainline Low Overhead Loop
 instructions @code{DLS} and @code{LE}.
 Some multilibs may be incompatible with these options.
 
+@item arm_thumb2_no_arm_v8_1_lob
+ARM target where Thumb-2 is used without options but does not support
+executing the Armv8.1-M Mainline Low Overhead Loop instructions
+@code{DLS} and @code{LE}.
+
 @item arm_thumb2_ok_no_arm_v8_1_lob
 ARM target generates Thumb-2 code for @code{-mthumb} but does not
 support executing the Armv8.1-M Mainline Low Overhead Loop
diff --git a/gcc/testsuite/gcc.target/arm/ivopts.c b/gcc/testsuite/gcc.target/arm/ivopts.c
index 2733e66988e..d7d72a59d9c 100644
--- a/gcc/testsuite/gcc.target/arm/ivopts.c
+++ b/gcc/testsuite/gcc.target/arm/ivopts.c
@@ -11,6 +11,6 @@  tr5 (short array[], int n)
 }
 
 /* { dg-final { scan-tree-dump-times "PHI <" 1 "ivopts"} } */
-/* { dg-final { object-size text <= 20 { target { arm_thumb2_ok_no_arm_v8_1_lob } } } } */
+/* { dg-final { object-size text <= 20 { target { arm_thumb2_no_arm_v8_1_lob } } } } */
 /* { dg-final { object-size text <= 32 { target { arm_nothumb && { ! arm_iwmmxt_ok } } } } } */
 /* { dg-final { object-size text <= 36 { target { arm_nothumb && arm_iwmmxt_ok }  } } } */
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 47d4c45e9eb..0d351c8fbad 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -10658,7 +10658,20 @@  proc check_effective_target_arm_v8_1_lob_ok { } {
     }
 }
 
-# Return 1 is this is an ARM target where -mthumb causes Thumb-2 to be
+# Return 1 if this is an ARM target where Thumb-2 is used without
+# options added by the test and the target does not support executing
+# the Armv8.1-M Mainline Low Overhead Loop, 0 otherwise.  The test is
+# valid for ARM.
+
+proc check_effective_target_arm_thumb2_no_arm_v8_1_lob { } {
+    if { [check_effective_target_arm_thumb2]
+	 && ![check_effective_target_arm_v8_1_lob_ok] } {
+	return 1
+    }
+    return 0
+}
+
+# Return 1 if this is an ARM target where -mthumb causes Thumb-2 to be
 # used and the target does not support executing the Armv8.1-M
 # Mainline Low Overhead Loop, 0 otherwise.  The test is valid for ARM.