From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 53244385802B for ; Fri, 25 Jun 2021 11:13:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 53244385802B Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id C8717ED1; Fri, 25 Jun 2021 04:13:24 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1C4123F694; Fri, 25 Jun 2021 04:13:24 -0700 (PDT) From: Richard Sandiford To: Xi Ruoyao via Gcc-patches Mail-Followup-To: Xi Ruoyao via Gcc-patches , Jeff Law , Xi Ruoyao , Matthew Fortune , richard.sandiford@arm.com Cc: Jeff Law , Xi Ruoyao , Matthew Fortune Subject: Re: [PATCH] testsuite: mips: use noinline attribute instead of -fno-inline References: <8f047551a4694778606d615bd22ef619006d044e.camel@mengyan1223.wang> <9ae500112d9c23f025576c2443c639ba0d0bbc26.camel@mengyan1223.wang> Date: Fri, 25 Jun 2021 12:13:22 +0100 In-Reply-To: <9ae500112d9c23f025576c2443c639ba0d0bbc26.camel@mengyan1223.wang> (Xi Ruoyao via Gcc-patches's message of "Fri, 25 Jun 2021 01:02:29 +0800") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-5.0 required=5.0 tests=BAYES_00, BODY_8BITS, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Jun 2021 11:13:27 -0000 Xi Ruoyao via Gcc-patches writes: > On Thu, 2021-06-24 at 10:48 -0600, Jeff Law wrote: >>=20 >>=20 >> On 6/22/2021 1:05 AM, Xi Ruoyao via Gcc-patches wrote: >> > mips.exp does not support -fno-inline, causing the tests return >> > "ERROR: >> > Unrecognised option: -fno-inline for dg-options ... ". >> >=20 >> > Use noinline attribute like other mips target tests, to workaround >> > it. >> >=20 >> > gcc/testsuite/ >> >=20 >> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.target/mips/cfgc= leanup-jalr2.c: Remove -fno-inline and >> > add >> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 __attribute__((= noinline)). >> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0* gcc.target/mips/cfgc= leanup-jalr3.c: Likewise. >> I'd like to know a bit more here.=C2=A0 mips.exp shouldn't care about th= e=20 >> options passed to the compiler and to the best of my knowledge=20 >> -fno-inline is still a valid GCC option.=C2=A0 So while I don't think th= e=20 >> patch itself is wrong, I question if it's necessary and whether or not >> your just papering over some other issue. > > There is some logic processing options in mips.exp. Some options are > overrided for multilib. It seems the mips.exp was originally designed > as: > > * MIPS options should go in dg-options > * Other options should go in dg-additional-options > > In d2148424165 marxin merged some dg-additional-options into dg-options, > exploited the problem. > > And, the "origin" convention seems already broken: there is something > like -funroll-loops which is not a MIPS option, but accepted by mips.exp > in dg-options. Originally the idea was that all options would go into dg-options, not just MIPS ones. If a test wanted to pass an option that mips.exp doesn't currently understand then mips.exp should be extended to classify the new option appropriately. Some options require intervention from mips.exp to be usable across all configurations while others can be passed through as-is. The idea was that mips.exp would be the single place that knew which options were which, rather than having to hard-code that distinction in every test. So I think the fix for this would be to extend: # Add -ffoo/-fno-foo options to mips_option_groups. foreach option { common delayed-branch expensive-optimizations fast-math fat-lto-objects finite-math-only fixed-hi fixed-lo lax-vector-conversions omit-frame-pointer optimize-sibling-calls peephole2 schedule-insns2 split-wide-types tree-vectorize unroll-all-loops unroll-loops ipa-ra } { lappend mips_option_groups $option "-f(no-|)$option" } to include "inline" as well. A rule like =E2=80=9Call -f options can be passed through as-is=E2=80=9D is= n't safe because (at least) -fpic requires special handling. Thanks, Richard