From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 126807 invoked by alias); 1 Jul 2016 15:24:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 126791 invoked by uid 89); 1 Jul 2016 15:24:57 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,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; Fri, 01 Jul 2016 15:24:45 +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 B903628; Fri, 1 Jul 2016 08:25:38 -0700 (PDT) Received: from e108577-lin.localnet (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id C4F163F25F; Fri, 1 Jul 2016 08:24:43 -0700 (PDT) From: Thomas Preudhomme To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] Build gdb.opt/inline-*.exp tests at -O0, rely on __attribute__((always_inline)) (was: Re: [PATCH v3 24/34] Push thread->control.command_interp to the struct thread_fsm) Date: Fri, 01 Jul 2016 15:24:00 -0000 Message-ID: <2455620.gRHMPhxaPl@e108577-lin> User-Agent: KMail/4.13.3 (Linux/3.13.0-85-generic; KDE/4.13.3; x86_64; ; ) In-Reply-To: <20144b4c-11ee-fc84-e3ad-b9992f14ce15@redhat.com> References: <1462538104-19109-25-git-send-email-palves@redhat.com> <8587009.ikB25uYBLk@e108577-lin> <20144b4c-11ee-fc84-e3ad-b9992f14ce15@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" X-IsSubscribed: yes X-SW-Source: 2016-07/txt/msg00014.txt.bz2 On Friday 01 July 2016 13:05:35 Pedro Alves wrote: > On 07/01/2016 12:02 PM, Thomas Preudhomme wrote: > > The new tests added by this patch fail for arm-none-eabi targets because > > -O2 leads to instructions to be reordered widely. In this case, the > > instruction that follows the first one for line 64 is related to line 70 > > so the test is failing. Shouldn't this test be compiled with -Og and > > probably also -finline- small-functions -findirect-inlining > > -fpartial-inlining which relates to inlining and are included in -O2. > > Or even just plain -O0. See the commit log below. WDYT? I'm not very familiar with GDB but the description looks great indeed. Thanks! Best regards, Thomas > > -------------- > Subject: [PATCH] Build gdb.opt/inline-*.exp tests at -O0, rely on > __attribute__((always_inline)) > > A test recently added to gdb.opt/inline-cmds.exp fails for > arm-none-eabi targets because -O2 leads to instructions to be > reordered widely. > > I guess it might have made sense years ago to enable optimization in > these tests, but I fail to see the need for that nowadays. > > Using -O0 while relying on __attribute__((always_inline)), which is > already used in the tests [1] [2], avoids this sort of trouble, while > still exercising the inlining-related use cases that are the focus of > these tests. > > I think that nowadays we can safely assume that all compilers we care > about support __attribute__((always_inline)) or similar. > > [1] - Except one spot that missed it. > > [2] - Note that the .exp files make sure the frames that should have > been inlined are indeed inlined, with "info frame". > > gdb/testsuite/ChangeLog: > 2016-07-01 Pedro Alves > > * gdb.opt/inline-break.exp: Remove optimize=-O2. > * gdb.opt/inline-bt.exp: Likewise. > * gdb.opt/inline-cmds.exp: Remove optimize=-O2 and add > additional_flags=-Winline. > * gdb.opt/inline-locals.exp: Likewise. > * gdb.opt/inline-markers.c (ATTR): Define. > (inlined_fn): Use it. > --- > gdb/testsuite/gdb.opt/inline-break.exp | 2 +- > gdb/testsuite/gdb.opt/inline-bt.exp | 2 +- > gdb/testsuite/gdb.opt/inline-cmds.exp | 2 +- > gdb/testsuite/gdb.opt/inline-locals.exp | 2 +- > gdb/testsuite/gdb.opt/inline-markers.c | 8 +++++++- > 5 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/gdb/testsuite/gdb.opt/inline-break.exp > b/gdb/testsuite/gdb.opt/inline-break.exp index b2aa22e..ac56b04 100644 > --- a/gdb/testsuite/gdb.opt/inline-break.exp > +++ b/gdb/testsuite/gdb.opt/inline-break.exp > @@ -20,7 +20,7 @@ > standard_testfile > > if { [prepare_for_testing $testfile.exp $testfile $srcfile \ > - {debug optimize=-O2 additional_flags=-Winline}] } { > + {debug additional_flags=-Winline}] } { > return -1 > } > > diff --git a/gdb/testsuite/gdb.opt/inline-bt.exp > b/gdb/testsuite/gdb.opt/inline-bt.exp index 63d76e2..13c6993 100644 > --- a/gdb/testsuite/gdb.opt/inline-bt.exp > +++ b/gdb/testsuite/gdb.opt/inline-bt.exp > @@ -17,7 +17,7 @@ standard_testfile .c inline-markers.c > > if {[prepare_for_testing $testfile.exp $testfile \ > [list $srcfile $srcfile2] \ > - {debug optimize=-O2 additional_flags=-Winline}]} { > + {debug additional_flags=-Winline}]} { > return -1 > } > > diff --git a/gdb/testsuite/gdb.opt/inline-cmds.exp > b/gdb/testsuite/gdb.opt/inline-cmds.exp index 684f4dd..6c84848 100644 > --- a/gdb/testsuite/gdb.opt/inline-cmds.exp > +++ b/gdb/testsuite/gdb.opt/inline-cmds.exp > @@ -19,7 +19,7 @@ set MIFLAGS "-i=mi" > standard_testfile .c inline-markers.c > > if {[prepare_for_testing $testfile.exp $testfile \ > - [list $srcfile $srcfile2] {debug optimize=-O2}]} { > + [list $srcfile $srcfile2] {debug additional_flags=-Winline}]} { > return -1 > } > > diff --git a/gdb/testsuite/gdb.opt/inline-locals.exp > b/gdb/testsuite/gdb.opt/inline-locals.exp index df2253a..36f7ed2 100644 > --- a/gdb/testsuite/gdb.opt/inline-locals.exp > +++ b/gdb/testsuite/gdb.opt/inline-locals.exp > @@ -16,7 +16,7 @@ > standard_testfile .c inline-markers.c > > if {[prepare_for_testing $testfile.exp $testfile \ > - [list $srcfile $srcfile2] {debug optimize=-O2}]} { > + [list $srcfile $srcfile2] {debug additional_flags=-Winline}]} { > return -1 > } > > diff --git a/gdb/testsuite/gdb.opt/inline-markers.c > b/gdb/testsuite/gdb.opt/inline-markers.c index cf92e79..41f8a38 100644 > --- a/gdb/testsuite/gdb.opt/inline-markers.c > +++ b/gdb/testsuite/gdb.opt/inline-markers.c > @@ -13,6 +13,12 @@ > You should have received a copy of the GNU General Public License > along with this program. If not, see . > */ > > +#ifdef __GNUC__ > +# define ATTR __attribute__((always_inline)) > +#else > +# define ATTR > +#endif > + > extern int x, y; > extern volatile int z; > > @@ -26,7 +32,7 @@ void marker(void) > x += y - z; /* set breakpoint 2 here */ > } > > -inline void inlined_fn(void) > +inline ATTR void inlined_fn(void) > { > x += y + z; > }