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.ang...@bell.net > > > sel-sched.c.d > > > 2018-09-03 John David Anglin <dang...@gcc.gnu.org> > > 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
