From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate.crashing.org (gate.crashing.org [63.228.1.57]) by sourceware.org (Postfix) with ESMTP id 7CAA53858415 for ; Tue, 23 Aug 2022 13:29:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7CAA53858415 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=kernel.crashing.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kernel.crashing.org Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 27NDSnAU029894; Tue, 23 Aug 2022 08:28:50 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 27NDSmEi029893; Tue, 23 Aug 2022 08:28:48 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Tue, 23 Aug 2022 08:28:47 -0500 From: Segher Boessenkool To: Peter Bergner Cc: Surya Kumari Jangala , gcc-patches@gcc.gnu.org, pthaugen@us.ibm.com Subject: Re: [PATCH] sched1: Fix -fcompare-debug issue in schedule_region [PR105586] Message-ID: <20220823132846.GT25951@gate.crashing.org> References: <92671542-b6b9-e39a-b483-514dffc4773a@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <92671542-b6b9-e39a-b483-514dffc4773a@linux.ibm.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.1 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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, 23 Aug 2022 13:29:53 -0000 Hi! On Tue, Aug 23, 2022 at 07:55:22AM -0500, Peter Bergner wrote: > It looks good to me, but I cannot approve it. Same here (both statements). > That said, you're missing > a ChangeLog entry for your new helper function. The ChangeLog mentions > what changed, not why it changed. And that is correct! Changelogs should not say that, that isn't their purpose (in GCC), not what they are used for. Explanations like that go in the email and/or the commit message. The main remaining usefulness of changelogs is to spot unintended commmits. > Maybe something like the following? > > gcc/ > PR rtl-optimization/105586 > * sched-rgn.cc (save_state_for_fallthru_edge): New function. > (schedule_region): Use it for all blocks. That looks perfect, it doesn't say "why" either :-) Segher