On 05/12/2017 05:28 AM, Bin Cheng wrote: > Hi, > This patch computes register pressure information on TREE SSA by a backward > live > range data flow problem. The major motivation is to estimate register > pressure > for inner-most loop on TREE SSA, then other optimizations can use it. So far > the > information is used only in predcom later, but it could be useful to > implement a > tree level scheduler in order to shrink live ranges. Unfortunately the > example > live range shrink pass I implemented doesn't have obvious impact on > performance. > I think one reason is TER which effectively undoes its effect. Maybe it will > be > useful once TER/expanding is replaced with a better instruction selector, it > is > not included in this patch. > One fact I need to mention is David proposed a similar patch long time ago at > https://gcc.gnu.org/ml/gcc-patches/2008-12/msg01261.html. It tries to compute > register pressure information on tree ssa and shrink live ranges based on that > information. Unfortunately the patch wasn't merged in the end. There has > been > quite changes in GCC implementation, I didn't use its code directly. However, > I did read that patch and had it in mind when implementing this one. If there > is any issue in this patch, it would be me that should be blamed. I also sent > message to David about this patch and the possible relation with his. > > Bootstrap and test on x86_64 and AArch64. Is it OK? > > Thanks, > bin > > 2017-05-10 Xinliang David Li <davi...@google.com> > Bin Cheng <bin.ch...@arm.com> > > * Makefile.in (tree-ssa-regpressure.o): New object file. > * tree-ssa-regpressure.c: New file. > * tree-ssa-regpressure.h: New file. Any thoughts on tests, either end-to-end or unit testing?
At a high level does this make more sense as a pass or as a function that is called by other passes? I don't have a strong opinion here, just putting the question out there for discussion. You've got a live computation solver in here. Is there some reason you don't use the existing life analysis code? I'd prefer not have have another life analysis implementation if we can avoid it. And if you were using that code, I think you can easily get the coalescing data you're using as well. I haven't gone through all the detail in the patch as I think we need to make sure we've got the design issues right first. BUt there are a couple nits noted inline below. > > > 0003-tree-ssa-regpressure-20170504.txt > > > From bf6e51ff68d87c372719de567d4de49d77744f77 Mon Sep 17 00:00:00 2001 > From: Bin Cheng <binch...@e108451-lin.cambridge.arm.com> > Date: Mon, 8 May 2017 15:20:27 +0100 > Subject: [PATCH 3/6] tree-ssa-regpressure-20170504.txt > > --- > gcc/Makefile.in | 1 + > gcc/tree-ssa-regpressure.c | 829 > +++++++++++++++++++++++++++++++++++++++++++++ > gcc/tree-ssa-regpressure.h | 21 ++ > 3 files changed, 851 insertions(+) > create mode 100644 gcc/tree-ssa-regpressure.c > create mode 100644 gcc/tree-ssa-regpressure.h > > diff --git a/gcc/Makefile.in b/gcc/Makefile.in > index 97259ac..abfd4bc 100644 > --- a/gcc/Makefile.in > +++ b/gcc/Makefile.in > @@ -1534,6 +1534,7 @@ OBJS = \ > tree-ssa-pre.o \ > tree-ssa-propagate.o \ > tree-ssa-reassoc.o \ > + tree-ssa-regpressure.o \ > tree-ssa-sccvn.o \ > tree-ssa-scopedtables.o \ > tree-ssa-sink.o \ > diff --git a/gcc/tree-ssa-regpressure.c b/gcc/tree-ssa-regpressure.c > new file mode 100644 > index 0000000..ebc6576 > --- /dev/null > +++ b/gcc/tree-ssa-regpressure.c > @@ -0,0 +1,829 @@ > +/* Reg Pressure Model and Live Range Shrinking Optimization on TREE SSA. > + Copyright (C) 2017 Free Software Foundation, Inc. > + > +This file is part of GCC. > + > +GCC is free software; you can redistribute it and/or modify it > +under the terms of the GNU General Public License as published by the > +Free Software Foundation; either version 3, or (at your option) any > +later version. > + > +GCC is distributed in the hope that it will be useful, but WITHOUT > +ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License > +for more details. > + > +You should have received a copy of the GNU General Public License > +along with GCC; see the file COPYING3. If not see > +<http://www.gnu.org/licenses/>. */ > + > + > +#include "config.h" > +#include "system.h" > +#include "coretypes.h" > +#include "backend.h" > +#include "rtl.h" > +#include "memmodel.h" > +#include "ira.h" So I suspect what we need from ira.h is fairly narrow. Would it be possible to pull the externalized interfaces we need for register pressure at the gimple level into a new include file? My worry is that as we pull in ira.h (and rtl.h and who knows what else) into the gimple space we end up with a tangled mess of touching rtl things in gimple which we'd like to avoid. In fact, the first thing that jumped out at me when I scanned the patch was the use of N_REG_CLASSES. That's really a target property. I must have mis-read the earlier patch to ira.c as I thought we were only really tracking 3 classes. Integer, float and vector. Do we really need to track pressure on all the classes to do a reasonable job? + > +/* Reg_alloc structure, coalesced from ssa variables. */ > +typedef struct reg_alloc > +{ > + /* ID of this reg_alloc. */ > + unsigned id; > + /* The type of ssa variables of this reg_alloc. */ > + tree type; > + /* Ssa variables coalesced to this reg_alloc. */ Use "SSA" rather than "Ssa". > + for (bsi = gsi_last_bb (bb); !gsi_end_p (bsi); gsi_prev (&bsi)) > + { > + stmt = gsi_stmt (bsi); > + stmt_info = create_stmt_lr_info (region, stmt); > + /* No need to compute live range information for debug stmt. */ > + if (is_gimple_debug (stmt)) > + continue; ISTM you'd want to test for a debug stmt before creating the stmt_lr_info? > + > +/* Check if PRESSURE is high according to target information about available > + registers. */ > +static inline bool > +high_reg_pressure (unsigned *pressure) > +{ > + int k; > + > + gcc_assert (pressure != NULL); > + > + for (k = 0; k < N_REG_CLASSES; k++) > + if (pressure[k] > 8 && pressure[k] > target_avail_regs[k]) > + return true; Where does the magic "8" come from? ISTM that anytime the pressure is greater than the available registers that the pressure is high, regardless of the absolute number of live objects. Jeff
