[PATCH GCC][3/6]New file computing regional register pressure on TREE SSA

Wed Jun 28 11:02:00 GMT 2017

On Wed, Jun 28, 2017 at 10:09 AM, Bin.Cheng <amker.cheng@gmail.com> wrote:
> On Tue, Jun 27, 2017 at 6:40 PM, Jeff Law <law@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  <davidxl@google.com>
>>>           Bin Cheng  <bin.cheng@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 <binche01@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
>>