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 4E799388A836; Tue, 21 Jul 2020 12:44:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 4E799388A836 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=richard.sandiford@arm.com 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 B82CD101E; Tue, 21 Jul 2020 05:44:30 -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 2975A3F66E; Tue, 21 Jul 2020 05:44:30 -0700 (PDT) From: Richard Sandiford To: "Hu\, Jiangping" Mail-Followup-To: "Hu\, Jiangping" , "gcc-patches\@gcc.gnu.org" , "rguenth\@gcc.gnu.org" , richard.sandiford@arm.com Cc: "gcc-patches\@gcc.gnu.org" , "rguenth\@gcc.gnu.org" Subject: Re: [PATCH] target: fix default value checking of x_str_align_functions in aarch64.c References: <20200714075518.4653-1-hujiangping@cn.fujitsu.com> Date: Tue, 21 Jul 2020 13:44:28 +0100 In-Reply-To: (Jiangping Hu's message of "Tue, 21 Jul 2020 11:06:11 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.4 required=5.0 tests=BAYES_00, 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: Tue, 21 Jul 2020 12:44:33 -0000 "Hu, Jiangping" writes: >> If there isn't anywhere that handles zero in the way that the documentat= ion >> implies (i.e. with -falign-loops=3D0 being equivalent to -falign-loops) = then maybe >> we should instead change the documentation to match the actual behaviour. >> > Yes, I confirmed in source level that there is no target that handles > zero in the way that the documentation implies. OK. > But, I think just adjusting the documentation to reflect the > implementation behavior will cause -falign-functions=3D0 and > -falign-functions=3D1 to have the same meaning which is a bit confusing > from the user's perspective. Yeah, it isn't great. On the other hand, it seems more natural for -falign-functions=3D0 to be the same as -fno-align-functions than it is for -falign-functions=3D1 to be the same as -fno-align-functions. > In fact, I have issued a pr to Bugzilla, and got a comment from > Richard Biener. We are considering whether to reject 0, which means > both the source and the documentation should be modified a bit. Yeah, only saw that PR today, sorry. Got a lot of email to catch up on :-) > I found that function parse_and_check_align_values in gcc/opts.c maybe > the point we want to modify, values less than 0 are currently rejected > here. Maybe we can change the '<=E2=80=99 to '<=3D' in the if statement. = But I > need test, to make sure there are no regression issues. > > What do you think? Either's fine with me. I guess treating =3D0 the same as =3D1 is less likely to break existing Makefiles. E.g. the usual practice has been to keep old options that no longer have an effect, rather than dropping them and turning them into errors. On the other hand, people trying to use -falign-functions=3D0 for the first time might prefer an error instead of unexpected behaviour. I guess yet another option would be to handle -falign-functions=3D0 like -falign-functions in common_handle_option (i.e. set the x_str_align_functions to null and x_align_functions to true.) Thanks, Richard