Re: C++ compile-time regressions

Re: C++ compile-time regressions

[Richard Kenner]

    I'm just completing some benchmark runs to see if our performance
    actually changes if i tell CSE to stop caring about memory (and run
    store motion after reload).

As far as I know, CSE is the only pass which knows the precise
semantics of when memory locations conflict.  Remember that they aren't
equivalence classes.

Re: C++ compile-time regressions

[Daniel Berlin]

--On Friday, August 03, 2001 9:58 AM -0400 Richard Kenner 
<kenner@vlsi1.ultra.nyu.edu> wrote:

>     I'm just completing some benchmark runs to see if our performance
>     actually changes if i tell CSE to stop caring about memory (and run
>     store motion after reload).
>
> As far as I know, CSE is the only pass which knows the precise
> semantics of when memory locations conflict.  Remember that they aren't
> equivalence classes.
'fraid not.
Both PRE and store motion know precisely when they conflict (I.E. they use 
true_dependence), and when they aren't available/antic (I.E. Their register 
operands changed).
It's faster because it doesn't require removing from a hash table, just 
setting or resetting a bit in a bitvector.

--Dan

Re: C++ compile-time regressions

[Gerald Pfeifer]

On Thu, 2 Aug 2001, Mark Mitchell wrote:
> I think we're tackling this from the wrong angle.  GCC uses about
> 3 to 4 times as much memory as it needs to, really, and our garbage
> collector touches too many pages.

Well, I think we need to tackle it from several angles. :-)   Right now,
both compile-time and run-time performance are worse than GCC 2.95 (even
if we tune for either one).

> I know how to fix these problems, I think, and I expect to start
> working on them soonish.

That's excellent news, thanks!

> Anyhow, I guess I think we've done enough for now.

Daniel seems to have a promising patch, (a first draft of) which seems
very non-invasive and simple; this might be an excellent candidate for
GCC 3.0.2.

On Thu, 2 Aug 2001, Daniel Berlin wrote:
> BTW, i've gotten the performance problem down using a slightly
> modified heuristic from integrate.c. On the last run, the compile
> times were about the same as 200 insns, but the performance was *much*
> better (we're down to about 10% speed loss).

Excellent.

> When your performance gets shot to hell, it's always being caused by
> not inlining things. I.E. at 100 insns, *::begin and *::end are taking
> >50% of the runtime, because they aren't being inlined.

I guessed that something extremely bad like this was going on, because
performance was getting *that* bad.

Good to see both issues (compile-time and run-time performance) being
addressed. :-)

Gerald
-- 
Gerald "Jerry" pfeifer@dbai.tuwien.ac.at http://www.dbai.tuwien.ac.at/~pfeifer/

Re: C++ compile-time regressions

[Daniel Berlin]

Gerald Pfeifer <pfeifer@dbai.tuwien.ac.at> writes:

> [ Includes patch. Okay for branch and mainline? ]
> 
> On Wed, 25 Jul 2001, Gerald Pfeifer wrote:
>> Yes, I've been working on this now.  To be honest, I don't have time to
>> do this, but I'll try to have something in time for 3.0.1 -- somehow.
> 
> So, here we go. (The first column is the value of PARAM_MAX_INLINE_INSNS.)
> 
>         -O2     -O3     -O2     -O3
>  100    8:29    8:48    4000228 3990276
>  500    8:24    8:53    4136996 4126148
>  600    8:33    8:59    4158820 4156068
>  700    8:52    9:32    4169028 4222436
>  800    8:34?  10:27    4179652 4315396
> 1000    9:09   11:27    4239076 4425860
> 1500    9:49   14:05    4336260 4637060
> 2000   10:47   23:47    4435428 4758052
> 
> To me, 600 seems like a definite and affordable improvement here; I'd
> be a bit hesitant to go over 700.
> 
>>> Realistically, I think we have to be willing to compromise here; the 3.0.1
>>> compiler is going to be slower *and* probably generate slower code than
>>> 2.95, which is too bad, but that seems to be where we're at.  If we could
>>> get to 10-25% on both figures that would be better than having one figure
>>> small and the other massive.
>> The problem is, on both ends of the scale (that is, either slower code
>> or slower generation) the *better* value is already around 25%, so a
>> compromise will be worse than that for *both* values.
> 
> While I still see what I wrote as quoted above as a problem, here is the
> patch I had promised.

BTW, i've gotten the performance problem down using a slightly
modified heuristic from
integrate.c.
On the last run, the compile times were about the same as 200 insns,
but the performance was *much* better (we're down to about 10% speed
loss).
When your performance gets shot to hell, it's always being caused by
not inlining things. I.E. at 100 insns, *::begin and *::end are taking
>50% of the runtime, because they aren't being inlined.

With a fixed store motion, we can turn off cse-skip-blocks and
cse-follow-jumps.
They buy us absolutely no gain, but cost a lot of time (In compiling
your app, Gerald, CSE accounts for > 20% of the compile time on the
files that take the longest to compile).
I've got statistics to back this up.
However, even with cse-skip-blocks and cse-follow-jumps turned off,
CSE is still >15% of the compile.
Mainly because it's trying to eliminate memory loads and stores, which
PRE and Store Motion do much faster than it (since they don't modify
the hash table when a store/load is killed, they just set a bit or two
in a bitvector), and on a global scale.
I'm just completing some benchmark runs to see if our performance
actually changes if i tell CSE to stop caring about memory (and run
store motion after reload).
I sincerely doubt it will, now that load and store motion should be
working.  
If it does, then PRE and store motion need to be improved. 

--Dan

-- 
"When I was a kid, I went to the store and asked the guy, "Do you
have any toy train schedules?"
"-Steven Wright

Re: C++ compile-time regressions

[Joe Buck]

Alexandre:
> > I suppose it would be desirable to inline pretty much the
> > same functions, regardless of the target.

Mark:
> Yes, I suppose -- although if a function really is going to take up
> twice as much icache on one platform as another, it might be a less
> promising inlining candidate.

The compiler doesn't know the size of the user's icache, so it's hard
to make these tradeoffs: processors with lower instruction density might
well use bigger icaches to compensate.

> If you run some numbers and can show it's a win, then I think that's
> good.  I wouldn't commit it without demonstrating the win, though.

Agreed, we need numbers.  Gerald's tests are one source of inputs; there
also may be some code in Boost that could be used.

Re: C++ compile-time regressions

[Mark Mitchell]

--On Thursday, August 02, 2001 04:25:22 PM -0300 "aoliva@redhat.com" 
<aoliva@redhat.com> wrote:

> I suppose it would be desirable to inline pretty much the
> same functions, regardless of the target.

Yes, I suppose -- although if a function really is going to take up
twice as much icache on one platform as another, it might be a less
promising inlining candidate.

If you run some numbers and can show it's a win, then I think that's
good.  I wouldn't commit it without demonstrating the win, though.

-- 
Mark Mitchell                mark@codesourcery.com
CodeSourcery, LLC            http://www.codesourcery.com

Re: C++ compile-time regressions

[Joe Buck]

Alexandre:
> >> But I wonder if the default value of PARAM_MAX_INLINE_INSNS should be
> >> a property of the target machine.  Different targets have different
> >> INSN densities.  I don't know how much this changes from one target to
> >> another in the early rtl stages used for rtl inlining, though...  Does
> >> anyone more experienced think it would be worth the trouble?  I could
> >> produce a patch to make the default target-modifiable.

Mark:
> > I think we're tackling this from the wrong angle.

Alexandre:
> I think I wasn't clear.  My suggestion didn't mean to affect compile
> time, but rather, to offer a reasonably similar behavior across
> multiple targets, in regards to inlining or not inlining functions.

But you're assuming that the current heuristic approach to inlining is
valid and only the parameter needs adjustment, and proposing a theory
about how it should be adjusted (normalize for instruction density)
without evidence that this is the right thing.

Re: C++ compile-time regressions

[aoliva]

On Aug  2, 2001, Mark Mitchell <mark@codesourcery.com> wrote:

>> But I wonder if the default value of PARAM_MAX_INLINE_INSNS should be
>> a property of the target machine.  Different targets have different
>> INSN densities.  I don't know how much this changes from one target to
>> another in the early rtl stages used for rtl inlining, though...  Does
>> anyone more experienced think it would be worth the trouble?  I could
>> produce a patch to make the default target-modifiable.

> I think we're tackling this from the wrong angle.

I think I wasn't clear.  My suggestion didn't mean to affect compile
time, but rather, to offer a reasonably similar behavior across
multiple targets, in regards to inlining or not inlining functions.
Using the same threshold on targets with very different INSN densities
is deemed to get fewer functions inlined on targets with smaller INSN
densities.  I suppose it would be desirable to inline pretty much the
same functions, regardless of the target.

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                  aoliva@{cygnus.com, redhat.com}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist    *Please* write to mailing lists, not to me

Re: C++ compile-time regressions

[Mark Mitchell]

> But I wonder if the default value of PARAM_MAX_INLINE_INSNS should be
> a property of the target machine.  Different targets have different
> INSN densities.  I don't know how much this changes from one target to
> another in the early rtl stages used for rtl inlining, though...  Does
> anyone more experienced think it would be worth the trouble?  I could
> produce a patch to make the default target-modifiable.

I think we're tackling this from the wrong angle.  GCC uses about
3 to 4 times as much memory as it needs to, really, and our garbage
collector touches too many pages.  That combination is what it causing
a lot of the problems, in my profiling.  I know how to fix these
problems, I think, and I expect to start working on them soonish.

Anyhow, I guess I think we've done enough for now.

-- 
Mark Mitchell                mark@codesourcery.com
CodeSourcery, LLC            http://www.codesourcery.com

Re: C++ compile-time regressions

[aoliva]

On Aug  2, 2001, Gerald Pfeifer <pfeifer@dbai.tuwien.ac.at> wrote:

> [ Includes patch. Okay for branch and mainline? ]
> On Wed, 25 Jul 2001, Gerald Pfeifer wrote:
>> Yes, I've been working on this now.  To be honest, I don't have time to
>> do this, but I'll try to have something in time for 3.0.1 -- somehow.

> So, here we go. (The first column is the value of PARAM_MAX_INLINE_INSNS.)

> To me, 600 seems like a definite and affordable improvement here

Great!  Thanks for doing this.

But I wonder if the default value of PARAM_MAX_INLINE_INSNS should be
a property of the target machine.  Different targets have different
INSN densities.  I don't know how much this changes from one target to
another in the early rtl stages used for rtl inlining, though...  Does
anyone more experienced think it would be worth the trouble?  I could
produce a patch to make the default target-modifiable.

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Red Hat GCC Developer                  aoliva@{cygnus.com, redhat.com}
CS PhD student at IC-Unicamp        oliva@{lsd.ic.unicamp.br, gnu.org}
Free Software Evangelist    *Please* write to mailing lists, not to me

Re: C++ compile-time regressions

[Mark Mitchell]

--On Thursday, August 02, 2001 09:19:42 AM +0200 Gerald Pfeifer 
<pfeifer@dbai.tuwien.ac.at> wrote:

> [ Includes patch. Okay for branch and mainline? ]
>
> On Wed, 25 Jul 2001, Gerald Pfeifer wrote:
>> Yes, I've been working on this now.  To be honest, I don't have time to
>> do this, but I'll try to have something in time for 3.0.1 -- somehow.

Approved for mainline and branch.  Thank you for doing the analysis.

Obviously, we all understand that this is a stopgap.

Thanks,

-- 
Mark Mitchell                mark@codesourcery.com
CodeSourcery, LLC            http://www.codesourcery.com

Re: C++ compile-time regressions

[Gerald Pfeifer]

[ Includes patch. Okay for branch and mainline? ]

On Wed, 25 Jul 2001, Gerald Pfeifer wrote:
> Yes, I've been working on this now.  To be honest, I don't have time to
> do this, but I'll try to have something in time for 3.0.1 -- somehow.

So, here we go. (The first column is the value of PARAM_MAX_INLINE_INSNS.)

        -O2     -O3     -O2     -O3
 100    8:29    8:48    4000228 3990276
 500    8:24    8:53    4136996 4126148
 600    8:33    8:59    4158820 4156068
 700    8:52    9:32    4169028 4222436
 800    8:34?  10:27    4179652 4315396
1000    9:09   11:27    4239076 4425860
1500    9:49   14:05    4336260 4637060
2000   10:47   23:47    4435428 4758052

To me, 600 seems like a definite and affordable improvement here; I'd
be a bit hesitant to go over 700.

>> Realistically, I think we have to be willing to compromise here; the 3.0.1
>> compiler is going to be slower *and* probably generate slower code than
>> 2.95, which is too bad, but that seems to be where we're at.  If we could
>> get to 10-25% on both figures that would be better than having one figure
>> small and the other massive.
> The problem is, on both ends of the scale (that is, either slower code
> or slower generation) the *better* value is already around 25%, so a
> compromise will be worse than that for *both* values.

While I still see what I wrote as quoted above as a problem, here is the
patch I had promised.

I cannot perform regression testing anytime soon, so if this is required,
someone else has to do that and install the patch for me. I'll also try to
run detailed performance tests, but it's clear from previous results that
we will recover at least a bit.

Gerald

2001-08-01  Gerald Pfeifer  <pfeifer@dbai.tuwien.ac.at>

	* params.def (PARAM_MAX_INLINE_INSNS): Change default to 600.
	Correct comment that had been missed in the previous change.

Index: params.def
===================================================================
RCS file: /cvs/gcc/egcs/gcc/params.def,v
retrieving revision 1.3.2.5
diff -u -3 -p -r1.3.2.5 params.def
--- params.def	2001/07/26 13:55:02	1.3.2.5
+++ params.def	2001/08/02 05:45:02
@@ -39,12 +39,12 @@ Boston, MA 02111-1307, USA.
    function.  Increasing values mean more agressive inlining.
    This affects currently only functions explicitly marked as
    inline (or methods defined within the class definition for C++).
-   The default value of 10000 is arbitrary but high to match the
-   previously unlimited gcc capabilities.  */
+   The original default value of 10000 was arbitrary and caused
+   significant compile-time performance regressions.  */
 DEFPARAM (PARAM_MAX_INLINE_INSNS,
 	  "max-inline-insns",
 	  "The maximum number of instructions in a function that is eligible for inlining",
-	  100)
+	  600)

 /* The maximum number of instructions to consider when looking for an
    instruction to fill a delay slot.  If more than this arbitrary