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 1D8BB3835693 for ; Thu, 21 Jul 2022 20:51:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1D8BB3835693 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 26LKooC7008234; Thu, 21 Jul 2022 15:50:50 -0500 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 26LKondo008233; Thu, 21 Jul 2022 15:50:49 -0500 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Thu, 21 Jul 2022 15:50:49 -0500 From: Segher Boessenkool To: Michael Meissner , gcc-patches@gcc.gnu.org, "Kewen.Lin" , David Edelsohn , Peter Bergner , Will Schmidt Subject: Re: [PATCH] Remove setting -mblock-ops-vector-pair on power10. Message-ID: <20220721205049.GL25951@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.1 required=5.0 tests=BAYES_00, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP 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: Thu, 21 Jul 2022 20:51:53 -0000 On Thu, Jul 21, 2022 at 02:42:29AM -0400, Michael Meissner wrote: > Testing has shown that using the load vector pair and store vector pair > instructions for block moves has some performance issues on power10. This > patch does not set this option by default. If it is a win in other > machines in the future, this flag can be set in the ISA options. This would make rs6000_isa_flags an even bigger misnomer than it already is, sigh. > * config/rs6000/rs6000.cc (rs6000_option_override_internal): > Do not enable -mblock-ops-vector-pair by default on power10. Do not wrap lines early, especially if that would mean leaving a colon at the end of a line. Changelog lines are 80 positions long (including the leading tab, which counts as eight). > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -4139,17 +4139,6 @@ rs6000_option_override_internal (bool global_init_p) > rs6000_isa_flags &= ~OPTION_MASK_BLOCK_OPS_UNALIGNED_VSX; > } > > - if (!(rs6000_isa_flags_explicit & OPTION_MASK_BLOCK_OPS_VECTOR_PAIR)) > - { > - /* Do not generate lxvp and stxvp on power10 since there are some > - performance issues. */ > - if (TARGET_MMA && TARGET_EFFICIENT_UNALIGNED_VSX > - && rs6000_tune != PROCESSOR_POWER10) > - rs6000_isa_flags |= OPTION_MASK_BLOCK_OPS_VECTOR_PAIR; > - else > - rs6000_isa_flags &= ~OPTION_MASK_BLOCK_OPS_VECTOR_PAIR; > - } How does this implement what the changelog says it does? With what it does the changelog should instead say to not touch it at all (your patch also disables the code that disables it!) It isn't clear what you intended: what your changelog says, or what the code does. Segher