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 A60893858425 for ; Thu, 22 Dec 2022 18:54:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A60893858425 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 2BMIrLYj007650; Thu, 22 Dec 2022 12:53:21 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id 2BMIrK23007649; Thu, 22 Dec 2022 12:53:20 -0600 X-Authentication-Warning: gate.crashing.org: segher set sender to segher@kernel.crashing.org using -f Date: Thu, 22 Dec 2022 12:53:20 -0600 From: Segher Boessenkool To: "Kewen.Lin" Cc: GCC Patches , Peter Bergner , Michael Meissner , David Edelsohn Subject: Re: [PATCH] rs6000: Fix some issues related to Power10 fusion [PR104024] Message-ID: <20221222185320.GX25951@gate.crashing.org> References: <009fda27-7119-6de8-8dbe-51126bdfca12@linux.ibm.com> <20221214222944.GR25951@gate.crashing.org> <20221220131904.GR25951@gate.crashing.org> <87345a42-055d-a104-bf43-7721e4b3bc9f@linux.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87345a42-055d-a104-bf43-7721e4b3bc9f@linux.ibm.com> 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,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 List-Id: On Wed, Dec 21, 2022 at 11:41:58AM +0800, Kewen.Lin wrote: > on 2022/12/20 21:19, Segher Boessenkool wrote: > > Sure, I understand that. What I don't like is the generator program is > > much too big and unstructured already, and this doesn't help at all; it > > makes it quite a bit worse even. > >> Good point, and I just noticed that we should check tune setting instead > >> of TARGET_POWER10 here? Something like: > >> > >> if (!(rs6000_isa_flags_explicit & OPTION_MASK_P10_FUSION)) > >> { > >> if (processor_target_table[tune_index].processor == PROCESSOR_POWER10) > >> rs6000_isa_flags |= OPTION_MASK_P10_FUSION; > >> else > >> rs6000_isa_flags &= ~OPTION_MASK_P10_FUSION; > >> } > > > > Yeah that looks better :-) > > I'm going to test this and commit it first. :) Thanks! > > Maybe you can restructure the Perl code a bit in a first patch, and then > > add the insn condition? If you're not comfortable with Perl, I'll deal > > with it, just update the patch. > > OK, I'll give it a try, TBH I just fixed the place for insn condition, didn't > look into this script, with a quick look, I'm going to factor out the main > body from the multiple level loop, do you have some suggestions on which other > candidates to be restructured? Anything that makes the code easier to understand, basically. This stuff is by nature pretty hard to read, but making the code shorter and/or less nested should make it easier to understand. You will need to have fewer local variables per function than there are total now, that will help. Btw, this script isn't so big at all, but the patches are hard to review without converting this to a side-by-side comparison first. There must be some way to improve that, that is what I'm looking for :-) Segher