From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3645 invoked by alias); 17 Aug 2017 01:55:36 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 2941 invoked by uid 89); 17 Aug 2017 01:55:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=il, traditionally, Dates, HX-Received:10.84.241.207 X-HELO: mail-pg0-f44.google.com Received: from mail-pg0-f44.google.com (HELO mail-pg0-f44.google.com) (74.125.83.44) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 17 Aug 2017 01:55:34 +0000 Received: by mail-pg0-f44.google.com with SMTP id i12so32463488pgr.3 for ; Wed, 16 Aug 2017 18:55:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=EcLtm7WrqvBBJ0ENU6DVR/4qfh2iKyQ3dTaKXzulyTQ=; b=f0Vm8orSMy1Mf2FzTjf9IkXw7DU1RYKCQZr2gO+GOEur0+Z/hG/mxcLChs8pbcPE4w rCyB6HUlV+Ns00a+sOPHcw5q+6oGc/n5GOaRpkFvCCEhvZN12Gog7cHUPatIA6txTLaR w2CZXxzd1d400vZybbWXQCoJeUdS6WYHlnehIJdEf+Aqu8FcERWJWJG5004+dvjKIacn sr9Wtz6/KSTCggA9Hnkp7R4plE4JfZmNiiRkbRVbhej36YWiGw48/gHSTh05iWBsKbX+ ekyVjNIgNYaBzhkJpK3tpH1DbWnrcXEkK0a6yg8VdiwnqgndKmT/sSIGSpSBy0eJkoA2 KigQ== X-Gm-Message-State: AHYfb5jQNN/QZwhrCzqfm5BbJVwVIONrdXD/IK7vGJkyMThfBHomuYrE ug9Pvdcm9bOgcqAy X-Received: by 10.84.241.207 with SMTP id t15mr3951546plm.406.1502934932165; Wed, 16 Aug 2017 18:55:32 -0700 (PDT) Received: from bubble.grove.modra.org (CPE-58-160-71-80.tyqh2.lon.bigpond.net.au. [58.160.71.80]) by smtp.gmail.com with ESMTPSA id q9sm3741683pfa.177.2017.08.16.18.55.31 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 16 Aug 2017 18:55:31 -0700 (PDT) Received: by bubble.grove.modra.org (Postfix, from userid 1000) id 8BD45C285B; Thu, 17 Aug 2017 11:25:27 +0930 (ACST) Date: Thu, 17 Aug 2017 05:28:00 -0000 From: Alan Modra To: Segher Boessenkool Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com Subject: Re: [RS6000] PR 80938, Don't emit eh_frame for regs that don't need saving Message-ID: <20170817015527.GA20448@bubble.grove.modra.org> References: <21f6fe5be45ca917a46e204c4382c67ebfbb742f.1502310090.git.segher@kernel.crashing.org> <20170810010305.GI16312@bubble.grove.modra.org> <20170810022822.GZ13471@gate.crashing.org> <20170810044740.GK16312@bubble.grove.modra.org> <20170810133924.GA13471@gate.crashing.org> <20170811031011.GO16312@bubble.grove.modra.org> <20170815072823.GS13471@gate.crashing.org> <20170815223504.GP12821@bubble.grove.modra.org> <20170816232313.GA13471@gate.crashing.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170816232313.GA13471@gate.crashing.org> User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes X-SW-Source: 2017-08/txt/msg01036.txt.bz2 On Wed, Aug 16, 2017 at 06:23:13PM -0500, Segher Boessenkool wrote: > Hi! > > On Wed, Aug 16, 2017 at 08:05:04AM +0930, Alan Modra wrote: > > Repost with requested changes. I've extracted the logic that omits > > frame saves from rs6000_frame_related to a new function, because it's > > easier to document that way. The logic has been simplified a little > > too: fixed_reg_p doesn't need to be called there. > > > > PR target/80938 > > * config/rs6000/rs6000.c (rs6000_savres_strategy): Revert 2017-08-09. > > Don't use store multiple if only one reg needs saving. > > (interesting_frame_related_regno): New function. > > (rs6000_frame_related): Don't emit eh_frame for regs that > > don't need saving. > > (rs6000_emit_epilogue): Likewise. > > It's not just eh_frame, it's all call frame information. Sure, I meant to fix that. > > +/* This function is called when rs6000_frame_related is processing > > + SETs within a PARALLEL, and returns whether the REGNO save ought to > > + be marked RTX_FRAME_RELATED_P. The PARALLELs involved are those > > + for out-of-line register save functions, store multiple, and the > > + Darwin world_save. They may contain registers that don't really > > + need saving. */ > > + > > +static bool > > +interesting_frame_related_regno (unsigned int regno) > > +{ > > + /* Saves apparently of r0 are actually saving LR. */ > > + if (regno == 0) > > + return true; > > + /* If we see CR2 then we are here on a Darwin world save. Saves of > > + CR2 signify the whole CR is being saved. */ > > + if (regno == CR2_REGNO) > > + return true; > > + /* Omit frame info for any user-defined global regs. If frame info > > + is supplied for them, frame unwinding will restore a user reg. > > + Also omit frame info for any reg we don't need to save, as that > > + bloats eh_frame and can cause problems with shrink wrapping. > > + Since global regs won't be seen as needing to be saved, both of > > + these conditions are covered by save_reg_p. */ > > + return save_reg_p (regno); > > +} > > The function name isn't so great, doesn't say what it does at all. Not > that I can think of anything better. > > Maybe whatever is creating those instructions should set RTX_FRAME_RELATED_P > by itself? Not sure if that is nicer. > > Both this CR2 and R0 handling are pretty nasty hacks. Could you add a > comment saying that? I wouldn't say the R0 handling is a nasty hack at all. You can't save LR directly, storing to the stack must go via a gpr. I'm 100% sure you know that, and so would anyone else working on powerpc gcc support. It so happens that we use r0 in every case we hit this code. *That* fact is commented. I don't really know what else you want. Hmm, maybe I'm just too close to this code. I'll go with expounding some of the things known, as follows. /* Saves apparently of r0 are actually saving LR. It doesn't make sense to substitute the regno here to test save_reg_p (LR_REGNO). We *know* LR needs saving, and dwarf2cfi.c is able to deduce that (set (mem) (r0)) is saving LR from a prior (set (r0) (lr)) marked as frame related. */ (Incidentally, the dwarf2cfi.c behaviour is described by In addition, if a register has previously been saved to a different register, Yup, great comment that one! Dates back to 2004, commit 60ea93bb72.) The CR2 thing is a long-standing convention for frame info about CR, a wart fixed by ELFv2. See elsewhere /* CR register traditionally saved as CR2. */ and /* In other ABIs, by convention, we use a single CR regnum to represent the fact that all call-saved CR fields are saved. We use CR2_REGNO to be compatible with gcc-2.95 on Linux. */ I'l go with: /* If we see CR2 then we are here on a Darwin world save. Saves of CR2 signify the whole CR is being saved. This is a long-standing ABI wart fixed by ELFv2. As for r0/lr there is no need to check that CR needs to be saved. */ > Okay for trunk with those improvements (eh_frame and hack comment). > Thanks! > > > Segher -- Alan Modra Australia Development Lab, IBM