On 14/06/17 18:03, Richard Earnshaw (lists) wrote:
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.preudho...@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?
Indeed, or rather "\\sit\[te\]\[te\]" once properly escaped. "\\sit\[te\]{2}"
also works and is even simpler so this is what this updated version uses.
Best regards,
Thomas
diff --git a/gcc/testsuite/gcc.target/arm/its.c b/gcc/testsuite/gcc.target/arm/its.c
index 5425f1e920592c911771d93a4620448b06d51394..f81a0df51cdb5fc26208c0a99e5c1cfb2ee4ed04 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\[te\]{2}" } } */
