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 87BCA3857341 for ; Thu, 5 May 2022 19:13:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 87BCA3857341 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 245JChwe007560; Thu, 5 May 2022 14:12:43 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 245JChT8007557; Thu, 5 May 2022 14:12:43 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Thu, 5 May 2022 14:12:43 -0500 From: Segher Boessenkool To: Michael Meissner , gcc-patches@gcc.gnu.org, David Edelsohn , Peter Bergner , Will Schmidt Subject: Re: [PATCH, V4] Eliminate power8 fusion options, use power8 tuning, PR target/102059 Message-ID: <20220505191243.GF25951@gate.crashing.org> References: Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, KAM_NUMSUBJECT, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Thu, 05 May 2022 19:13:45 -0000 On Tue, Apr 12, 2022 at 09:14:55PM -0400, Michael Meissner wrote: > This is V4 of the patch. Compared to V3 of the patch, GCC will just > ignore -m{,no-}power8-fusion and -m{,no-}power8-fusion-sign. But incorrectly :-( > The splitting of signed halfword and word loads into unsigned load and > sign extension is now suppressed with -Os, but it is done normally if we > are not optimizing for space. I have no idea what that means. Other than that I asked to remove that. > This code makes the -mpower8-fusion option a nop. It is accepted without > warning, but it does nothing. Power8 fusion is only enabled if we are tuning > for a power8. It should *delete* the option, and have ;; This option existed in the past, but now is always off. mno-power8-fusion Target RejectNegative Undocumented Ignore > The undocumented -mpower8-fusion-sign option is also made into a nop. That one should be deleted. > + /* The Power8 fusion option was removed. We ignore using it in #pragma and > + attribute target. Users may have used the options to suppress errors if > + they declare an inline function to be specifically power8 and the function > + was included by power9 or power10 which turned off the power8 fusion > + support. */ > + { "power8-fusion", 0, false, true }, What does the comment mean? > + /* Don't print options that exist for backwards compatibility, but are > + ignored now like -mpower8-fusion. */ > + if (!mask) > + continue; No. Such options should not be in the mask at all. > +/* Power8 has special fusion operations that are enabled if we are tuning for > + power8. This used to be settable with an option (-mpower8-fusion), but that > + option has been removed. */ > +#define TARGET_P8_FUSION (rs6000_tune == PROCESSOR_POWER8) The plan was to not have p8 fusion at all. GCC never implemented any of the more useful p8 fusion things anyway, and those were only marginally beneficial anyway. > +/* Power8 fusion does not fuse loads with sign extends. If we are not > + optimizing for space, split loads with sign extension to loads with zero > + extension and an explicit sign extend operation, so that the zero extending > + load can be fused. */ > +#define TARGET_P8_FUSION_SIGN (TARGET_P8_FUSION \ > + && !optimize_function_for_size_p (cfun)) As I said before, don't do this. Just remove the whole thing. > +; The -mpower8-fusion and -mpower8-fusion-sign options existed in the past, but > +; they are ignored now. Don't put them together. It is much easier for everything if they are separate, boring, and exactly like everything else. > mpower8-fusion > -Target Mask(P8_FUSION) Var(rs6000_isa_flags) > -Fuse certain integer operations together for better performance on power8. > +Target Undocumented Ignore It should be deleted, instead, and be replaced with ;; This option existed in the past, but now is always off. mno-power8-fusion Target RejectNegative Undocumented Ignore mpower8-fusion Target RejectNegative Undocumented WarnRemoved i.e. just like all other removed flags. If someone explicitly tries to enable it he/she *should* get a warning. > mpower8-fusion-sign > -Target Undocumented Mask(P8_FUSION_SIGN) Var(rs6000_isa_flags) > -Allow sign extension in fusion operations. > +Target Undocumented Ignore And this one should be completely removed, since no one ever used it. Segher