From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 62076 invoked by alias); 6 Sep 2018 17:05:29 -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 62063 invoked by uid 89); 6 Sep 2018 17:05:28 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=years, happily, hell, hppa X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 06 Sep 2018 17:05:27 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AB180300C231; Thu, 6 Sep 2018 17:05:25 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-8.rdu2.redhat.com [10.10.112.8]) by smtp.corp.redhat.com (Postfix) with ESMTP id E3E4560BE4; Thu, 6 Sep 2018 17:05:24 +0000 (UTC) Subject: Re: [PATCH]: Allow TARGET_SCHED_ADJUST_PRIORITY hook to reduce priority To: John David Anglin , GCC Patches References: <0339d94b-e63b-d4b5-ee11-04133ad2b418@bell.net> From: Jeff Law Openpgp: preference=signencrypt Message-ID: <175a9d44-219f-12c5-e669-60674e82ab2d@redhat.com> Date: Thu, 06 Sep 2018 17:05:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <0339d94b-e63b-d4b5-ee11-04133ad2b418@bell.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00409.txt.bz2 On 09/03/2018 08:32 AM, John David Anglin wrote: > The documentation for TARGET_SCHED_ADJUST_PRIORITY indicates that the > hook can > reduce the priority of INSN to execute it later.  The hppa hook only > reduces the priority > and it has been this way for years.  However, the assert in > sel_target_adjust_priority() > prevents reduction of the priority. > > The attached change revises the assert to allow the priority to be > reduced to zero. > > This fixes PR rtl-optimization/85458. > > Tested on hppa-unknown-linux-gnu, hppa2.0w-hp-hpux11.11 and > hppa64-hp-hpux11.11. > > I must admit that this happens so infrequently that I have to wonder if > the hook provides > any benefit on hppa.  It was supposed to keep addil instructions close > to the following instruction > to reduce pressure on register %r1. > > Okay? > > Dave > > --  > John David Anglin  dave.anglin@bell.net > > > sel-sched.c.d > > > 2018-09-03 John David Anglin > > PR rtl-optimization/85458 > * sel-sched.c (sel_target_adjust_priority): Allow backend adjust > priority hook to reduce the priority of EXPR. OK. And yes, this is to try and keep the addil and the next use of %r1 close to each other and hopefully avoid spilling %r1. I don't recall writing this code, but according to git blame I did :-) THe original is circa 1995. Way back then folks were really concerned about the quality of code the PA generated, particularly when accessing objects in static storage. We went to some great lengths to try and optimize that code. One of the significant issues was that in 1995, we didn't have IRA/LRA and in fact we didn't have localized spilling. So when we needed %r1 to satisfy a spill, we had to spill every pseudo that had been allocated to %r1 regardless of whether or not it conflicted with the need. The results looked truly horrific. At that time we were also consolidating all static objects within a translation unit into a structure. That allowed unrelated objects to share a single addil. This required deep copying tree nodes -- back when we still used obstacks and folks would happily stuff an integer object into a tree field. It was ultimately unmaintainable and scrapped. One of the conclusions after doing all that work was that it ultimately didn't really matter. While the code looked a hell of a lot better and was more compact, it didn't actually perform any better on the PA8000 generation hardware that was hitting the streets. There's various reasons for that, but the most important in my mind was that the addil is just a constant. So it's subject to LICM, CSE, PRE, etc. THere's just not that many from a dynamic instruction standpoint. Combine that with the capabilities of the PA8000 and how we scheduled PA8000 code and the %r1 spill avoidance and addil elimination just wasn't a real factor in code performance. Anyway, the patch is fine. Your call if you want to just kill the code in the target file, it's just not terribly important anymore. jeff