| Message ID | 009d27e8-76dd-2cf5-4730-185d2def0dbc@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
On 15/06/17 09:18, Thomas Preudhomme wrote: > > > 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.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? > > 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. > OK. R. > 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..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}" } } */ >
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}" } } */