From patchwork Thu Jun 15 08:18:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Preudhomme X-Patchwork-Id: 776174 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3wpGbd6kTmz9s65 for ; Thu, 15 Jun 2017 18:18:21 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="r280UUsS"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=mHibVJuUkhdZsPuiP eTWYVUWiAEkgDQznrT3ys3GkR4RRdK2igS9/Th4Xv7ZTCWn7/rRbCyj8IKyEuujW rBvYrAFDbKR9XrEJ/c+HxKSfkwjeTOn9a8p8Zxte0WQTPIfDPuM29MbEiOT1yhfB mJcRjcixhKwNiN3s5d7CMbSgtY= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=WmxkCaHfWl81w+s0PBVRGp+ fjYg=; b=r280UUsS+BFhH1ZbIu5/q/mKk2NKYKyoqNCvAZv1Pqli1wiX5Bku5XH D3UQyBnaWoS8IO3P6bi9chsvNZ1M1tq9cRK72k78ZoPxzbCkdYPYzTpllX3HxJGZ SPdkwqLLTmGZmbZ2E5T/L7AFRLMlZuMKY6B6DXNK9bddjc9TqX18= Received: (qmail 35986 invoked by alias); 15 Jun 2017 08:18:08 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 35970 invoked by uid 89); 15 Jun 2017 08:18:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 15 Jun 2017 08:18:05 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3C12580D; Thu, 15 Jun 2017 01:18:09 -0700 (PDT) Received: from [10.2.206.52] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 74E7C3F581; Thu, 15 Jun 2017 01:18:08 -0700 (PDT) Subject: Re: [PATCH, GCC/testsuite/ARM] Make gcc.target/arm/its.c more robust To: "Richard Earnshaw (lists)" , Kyrill Tkachov , Ramana Radhakrishnan , "gcc-patches@gcc.gnu.org" References: <6af603a9-3824-0a43-444f-e68e84c8c9b7@arm.com> From: Thomas Preudhomme Message-ID: <009d27e8-76dd-2cf5-4730-185d2def0dbc@foss.arm.com> Date: Thu, 15 Jun 2017 09:18:07 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <6af603a9-3824-0a43-444f-e68e84c8c9b7@arm.com> X-IsSubscribed: yes 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 >> >> * 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}" } } */