diff mbox

[GCC/testsuite/ARM] Make gcc.target/arm/its.c more robust

Message ID f6949dd9-e519-e106-f87c-5960ec17de80@foss.arm.com
State New
Headers show

Commit Message

Thomas Preudhomme June 14, 2017, 4:49 p.m. UTC
Hi,

Testcase gcc.target/arm/its.c was added as part of a patch [1] to limit
IT blocks to 2 instructions maximum. However, the patch was only tested
indirectly by *aiming* to check that the assembly output does not
contain a single IT block with all conditional code in it. This was
actually implemented by expecting exactly 2 IT blocks.

[1] https://gcc.gnu.org/ml/gcc-patches/2014-01/msg00764.html

This does not work as proved by the regression following code changes
brought by r248863: some of the instructions are conditionally executed
using a branch and thus there is only one IT block. This patch changes
the logic to look for an IT block with more than 2 conditions, ie. IT
followed by zero or one non space letter.

This patch also restrict the testcase to Thumb-only devices since the
patch the testcase was contributed with only concerned ARMv7-M targets.
Since tuning for ARMv7E-M targets is even more restrictive (only one
instruction per IT block), restricting the testcase to all Thumb-only
devices is sufficient.

ChangeLog entry is as follows:

*** gcc/testsuite/ChangeLog ***

2017-06-09  Thomas Preud'homme  <thomas.preudhomme@arm.com>

*	gcc.target/arm/its.c: Check that no IT blocks has more than 2
	instructions in it rather than the number of IT blocks being 2.
	Transfer scan directive arm_thumb2 restriction to the whole
	testcase and restrict further to Thumb-only targets.


Testing: Test is correctly skipped when targeting Thumb mode of Cortex-A15
and Cortex-M0 and PASS for Cortex-M7. Note that it FAILs for Cortex-M3
and Cortex-M4 and manual inspection does reveal that an IT block is
generated with more than 2 instructions in it.

Is this ok for trunk?

Best regards,

Thomas

Comments

Richard Earnshaw (lists) June 14, 2017, 5:03 p.m. UTC | #1
On 14/06/17 17:49, Thomas Preudhomme wrote:
> Hi,
> 
> Testcase gcc.target/arm/its.c was added as part of a patch [1] to limit
> IT blocks to 2 instructions maximum. However, the patch was only tested
> indirectly by *aiming* to check that the assembly output does not
> contain a single IT block with all conditional code in it. This was
> actually implemented by expecting exactly 2 IT blocks.
> 
> [1] https://gcc.gnu.org/ml/gcc-patches/2014-01/msg00764.html
> 
> This does not work as proved by the regression following code changes
> brought by r248863: some of the instructions are conditionally executed
> using a branch and thus there is only one IT block. This patch changes
> the logic to look for an IT block with more than 2 conditions, ie. IT
> followed by zero or one non space letter.
> 
> This patch also restrict the testcase to Thumb-only devices since the
> patch the testcase was contributed with only concerned ARMv7-M targets.
> Since tuning for ARMv7E-M targets is even more restrictive (only one
> instruction per IT block), restricting the testcase to all Thumb-only
> devices is sufficient.
> 
> ChangeLog entry is as follows:
> 
> *** gcc/testsuite/ChangeLog ***
> 
> 2017-06-09  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
> *    gcc.target/arm/its.c: Check that no IT blocks has more than 2
>     instructions in it rather than the number of IT blocks being 2.
>     Transfer scan directive arm_thumb2 restriction to the whole
>     testcase and restrict further to Thumb-only targets.
> 
> 
> Testing: Test is correctly skipped when targeting Thumb mode of Cortex-A15
> and Cortex-M0 and PASS for Cortex-M7. Note that it FAILs for Cortex-M3
> and Cortex-M4 and manual inspection does reveal that an IT block is
> generated with more than 2 instructions in it.
> 
> Is this ok for trunk?
> 
> Best regards,
> 
> Thomas
> 
> make_its_test_more_robust.patch
> 
> 
> diff --git a/gcc/testsuite/gcc.target/arm/its.c b/gcc/testsuite/gcc.target/arm/its.c
> index 5425f1e920592c911771d93a4620448b06d51394..4e07871b57886e210391db1a72d1bc5b465a49d0 100644
> --- a/gcc/testsuite/gcc.target/arm/its.c
> +++ b/gcc/testsuite/gcc.target/arm/its.c
> @@ -1,4 +1,6 @@
>  /* { dg-do compile } */
> +/* { dg-require-effective-target arm_cortex_m } */
> +/* { dg-require-effective-target arm_thumb2 } */
>  /* { dg-options "-O2" }  */
>  int test (int a, int b)
>  {
> @@ -17,4 +19,6 @@ int test (int a, int b)
>      r -= 3;
>    return r;
>  }
> -/* { dg-final { scan-assembler-times "\tit" 2 { target arm_thumb2 } } } */
> +/* Ensure there is no IT block with more than 2 instructions, ie. we only allow
> +   IT, ITT and ITE.  */
> +/* { dg-final { scan-assembler-not "\\sit\[^\\s\]{2,}\\s" } } */
> 

Wouldn't

{dg-final { scan-assembler-not "it[te][te]" } }

be easier to understand?

R.
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.target/arm/its.c b/gcc/testsuite/gcc.target/arm/its.c
index 5425f1e920592c911771d93a4620448b06d51394..4e07871b57886e210391db1a72d1bc5b465a49d0 100644
--- a/gcc/testsuite/gcc.target/arm/its.c
+++ b/gcc/testsuite/gcc.target/arm/its.c
@@ -1,4 +1,6 @@ 
 /* { dg-do compile } */
+/* { dg-require-effective-target arm_cortex_m } */
+/* { dg-require-effective-target arm_thumb2 } */
 /* { dg-options "-O2" }  */
 int test (int a, int b)
 {
@@ -17,4 +19,6 @@  int test (int a, int b)
     r -= 3;
   return r;
 }
-/* { dg-final { scan-assembler-times "\tit" 2 { target arm_thumb2 } } } */
+/* Ensure there is no IT block with more than 2 instructions, ie. we only allow
+   IT, ITT and ITE.  */
+/* { dg-final { scan-assembler-not "\\sit\[^\\s\]{2,}\\s" } } */