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 87BC73857016 for ; Wed, 19 Aug 2020 10:52:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 87BC73857016 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 3A6D9101E; Wed, 19 Aug 2020 03:52:04 -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 679683F6CF; Wed, 19 Aug 2020 03:52:03 -0700 (PDT) From: Richard Sandiford To: Andrea Corallo Mail-Followup-To: Andrea Corallo , Segher Boessenkool , Richard Earnshaw , nd@arm.com, gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Cc: Segher Boessenkool , Richard Earnshaw , nd@arm.com, gcc-patches@gcc.gnu.org Subject: Re: [PATCH 1/2] Add new RTX instruction class FILLER_INSN References: <20200724211834.GS32057@gate.crashing.org> Date: Wed, 19 Aug 2020 11:52:01 +0100 In-Reply-To: (Andrea Corallo's message of "Wed, 19 Aug 2020 11:13:40 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-7.7 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: Wed, 19 Aug 2020 10:52:06 -0000 Andrea Corallo writes: > Segher Boessenkool writes: > >> Hi Andrea, >> >> On Wed, Jul 22, 2020 at 12:02:33PM +0200, Andrea Corallo wrote: >>> This first patch implements the addition of a new RTX instruction class >>> FILLER_INSN, which has been white listed to allow placement of NOPs >>> outside of a basic block. This is to allow padding after unconditional >>> branches. This is favorable so that any performance gained from >>> diluting branches is not paid straight back via excessive eating of >>> nops. >> >>> It was deemed that a new RTX class was less invasive than modifying >>> behavior in regards to standard UNSPEC nops. >> >> So I wonder if this cannot be done with some kind of NOTE, instead? >> > > Hi Segher, > > I was having a look into reworking this using an insn note as (IIUC) > suggested. The idea is appealing but looking into insn-notes.def I've > found the following comment: > > "We are slowly removing the concept of insn-chain notes from the > compiler. Adding new codes to this file is STRONGLY DISCOURAGED. > If you think you need one, look for other ways to express what you > mean, such as register notes or bits in the basic-block structure." > > Would still be justificated in this case to proceed this way? The other > option would be to add the information into the basic-block or into > struct rtx_jump_insn. > > My GCC experience is far from sufficient for having a formed opinion on > this, I'd probably bet on struct rtx_jump_insn as the better option. Adding it to the basic block structure wouldn't work because we need this information to survive until asm output time, and the cfg doesn't last that long. (Would be nice if it did, but that's a whole new can of worms.) Using REG_NOTES on the jump might be OK. I guess the note value could be the length in bytes. shorten_branches would then need to look for these notes and add the associated length after adding the length of the insn itself. There would then need to be some hook that final.c can call to emit nops of the given length. I guess there's also the option of representing this in the same way as a delayed branch sequence, which is to make the jump insn pattern: (sequence [(normal jump insn) (delayed insn 1) ...]) The members of the sequence are full insns, rather than just patterns. For this use case, the delayed insns would all be nops. However, not much is prepared to handle the sequence representation before the normal pass_machine_reorg position. (The main dbr pass itself is pass_delay_slots, but some targets run dbr within pass_machine_reorg instead.) There again, it isn't worth doing layout optimisations earlier than pass_machine_reorg anyway. Thanks, Richard