On Wed, Jun 28, 2017 at 10:09 AM, Bin.Cheng <amker.ch...@gmail.com> wrote: > On Tue, Jun 27, 2017 at 6:40 PM, Jeff Law <l...@redhat.com> wrote: >> 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. >> >> > Hi Jeff, Thanks very much for the review. I plan to revisit this > after current tasks as well as study more benchmark cases for register > pressure.
Please also rename the files to ssa-regpressure.h and ssa-prepressure.cc (I think new files should get C++ extensions now). Richard. > Thanks, > bin >> >> >> >> >>> >>> >>> 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 >>
