llvm dev - Dec 2016 - llvm (the middle-end) is getting slower, December edition

> On Dec 21, 2016, at 1:57 PM, Hal Finkel via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> 
> On 12/21/2016 03:36 PM, Sanjay Patel via llvm-dev wrote:
>> I would have replied to this thread sooner, but I was busy adding more
instcombines. :)
>> 
>> The motivation for r289855 is in the commit msg (I'm out of the
office and can't look things up conveniently). Feel free to revert that and
the follow ups, however, if that patch caused a noticeable slowdown, then it
suggests we have a bigger problem?...that's a simple matcher (no value
tracking required).
> 
> I'd recommend against reverting this until we understand more about the
problem.
I tend to agree, but I’d also like that we get better understanding before 4.0
branches.
> This is not a critical compile-time regression,
I didn’t see in this thread a mention about how much alone this is causing? The
regression mentioned by Davide (20%) seems critical to me.

— 
Mehdi

> and we don't yet know the cause (it might just be enabling more
down-stream transformations), or whether there have been any corresponding
benefits to code side, performance, etc. It definitely looks like a pattern that
we should catch.
> 
>  -Hal
> 
>> 
>> On Wed, Dec 21, 2016 at 12:12 PM Sean Silva <chisophugis at
gmail.com <mailto:chisophugis at gmail.com>> wrote:
>> On Wed, Dec 21, 2016 at 8:07 AM, Davide Italiano via llvm-dev
<llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>>
wrote:
>> On Mon, Dec 19, 2016 at 4:24 PM, Mikhail Zolotukhin
>> 
>> 
>> <mzolotukhin at apple.com <mailto:mzolotukhin at
apple.com>> wrote:
>> 
>> 
>> > Hi Davide,
>> 
>> 
>> >
>> 
>> 
>> > Thanks for the analysis, it's really interesting! And I'm
really glad that we now put more and more attention at the compile time!
>> 
>> 
>> >
>> 
>> 
>> > Just recently I've been looking into historical compile time
data as well, and have had similar conclusions. The regressions you've found
are probably caused by:
>> 
>> 
>> > 1) r289813 and r289855 - new matchers in InstCombine
>> 
>> 
>> > 2) r286814 and r288024 - changes in Inlining cost model
>> 
>> 
>> >
>> 
>> 
>> 
>> 
>> 
>> Haven't looked at 2) yet, but I can confirm for 1). Sanjay/Ehsan,
can
>> 
>> 
>> you please explain what's the motivation behind the new
>> 
>> 
>> transformations you introduced? I'm tempted to ask a revert, but
I'd
>> 
>> 
>> like to understand the motivations first.
>> 
>> 
>> Both r289813 and r289855 add a very small amount of matching (it
seems?) compared to the rest of the size of instcombine. How is it that these
checks are causing such a disproportionate slowdown compared to the rest of
instcombine? (by "I can confirm for 1)" I assume you mean these two
patches have a pretty significant impact on compile time; not "0.1%"
each)
>> 
>> -- Sean Silva
>>  
>> 
>> 
>> 
>> --
>> 
>> 
>> Davide
>> 
>> 
>> 
>> 
>> 
>> "There are no solved problems; there are only problems that are
more
>> 
>> 
>> or less solved" -- Henri Poincare
>> 
>> 
>> _______________________________________________
>> 
>> 
>> LLVM Developers mailing list
>> 
>> 
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> 
>> 
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>> 
>> 
>> 
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
> 
> -- 
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161221/025a5b6e/attachment.html>

On 12/21/2016 04:40 PM, Mehdi Amini wrote:
>
>> On Dec 21, 2016, at 1:57 PM, Hal Finkel via llvm-dev 
>> <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
>>
>>
>> On 12/21/2016 03:36 PM, Sanjay Patel via llvm-dev wrote:
>>> I would have replied to this thread sooner, but I was busy adding 
>>> more instcombines. :)
>>>
>>> The motivation for r289855 is in the commit msg (I'm out of the
>>> office and can't look things up conveniently). Feel free to
revert
>>> that and the follow ups, however, if that patch caused a noticeable
>>> slowdown, then it suggests we have a bigger problem?...that's a
>>> simple matcher (no value tracking required).
>>
>> I'd recommend against reverting this until we understand more about
>> the problem.
>
> I tend to agree, but I’d also like that we get better understanding 
> before 4.0 branches.
>
>> This is not a critical compile-time regression,
>
> I didn’t see in this thread a mention about how much alone this is 
> causing? The regression mentioned by Davide (20%) seems critical to me.
Maybe I misread this. I thought it was much smaller. We should 
double-check the data too. If this one commit is responsible for a 20% 
compile-time regression, that is indeed serious.

  -Hal
>
> —
> Mehdi
>
>
>> and we don't yet know the cause (it might just be enabling more 
>> down-stream transformations), or whether there have been any 
>> corresponding benefits to code side, performance, etc. It definitely 
>> looks like a pattern that we should catch.
>>
>>  -Hal
>>
>>>
>>> On Wed, Dec 21, 2016 at 12:12 PM Sean Silva <chisophugis at
gmail.com
>>> <mailto:chisophugis at gmail.com>> wrote:
>>>
>>>     On Wed, Dec 21, 2016 at 8:07 AM, Davide Italiano via
>>>     llvm-dev<llvm-dev at lists.llvm.org
>>>     <mailto:llvm-dev at lists.llvm.org>>wrote:
>>>
>>>         On Mon, Dec 19, 2016 at 4:24 PM, Mikhail Zolotukhin
>>>
>>>
>>>         <mzolotukhin at apple.com <mailto:mzolotukhin at
apple.com>> wrote:
>>>
>>>
>>>         > Hi Davide,
>>>
>>>
>>>         >
>>>
>>>
>>>         > Thanks for the analysis, it's really interesting!
And I'm
>>>         really glad that we now put more and more attention at the
>>>         compile time!
>>>
>>>
>>>         >
>>>
>>>
>>>         > Just recently I've been looking into historical
compile
>>>         time data as well, and have had similar conclusions. The
>>>         regressions you've found are probably caused by:
>>>
>>>
>>>         > 1) r289813 and r289855 - new matchers in InstCombine
>>>
>>>
>>>         > 2) r286814 and r288024 - changes in Inlining cost
model
>>>
>>>
>>>         >
>>>
>>>
>>>
>>>
>>>
>>>         Haven't looked at 2) yet, but I can confirm for 1).
>>>         Sanjay/Ehsan, can
>>>
>>>
>>>         you please explain what's the motivation behind the new
>>>
>>>
>>>         transformations you introduced? I'm tempted to ask a
revert,
>>>         but I'd
>>>
>>>
>>>         like to understand the motivations first.
>>>
>>>
>>>
>>>     Both r289813 and r289855 add a very small amount of matching
(it
>>>     seems?) compared to the rest of the size of instcombine. How is
>>>     it that these checks are causing such a disproportionate
>>>     slowdown compared to the rest of instcombine? (by "I can
confirm
>>>     for 1)" I assume you mean these two patches have a pretty
>>>     significant impact on compile time; not "0.1%" each)
>>>
>>>     -- Sean Silva
>>>
>>>
>>>
>>>
>>>         --
>>>
>>>
>>>         Davide
>>>
>>>
>>>
>>>
>>>
>>>         "There are no solved problems; there are only problems
that
>>>         are more
>>>
>>>
>>>         or less solved" -- Henri Poincare
>>>
>>>
>>>
>>>         _______________________________________________
>>>
>>>
>>>         LLVM Developers mailing list
>>>
>>>
>>>         llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>
>>>
>>>
>>>         http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>> -- 
>> Hal Finkel
>> Lead, Compiler Technology and Programming Languages
>> Leadership Computing Facility
>> Argonne National Laboratory
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161221/c28f8e48/attachment-0001.html>

On Wed, Dec 21, 2016 at 2:57 PM, Hal Finkel <hfinkel at anl.gov>
wrote:
>
> On 12/21/2016 04:40 PM, Mehdi Amini wrote:
>
>
> On Dec 21, 2016, at 1:57 PM, Hal Finkel via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>
>
> On 12/21/2016 03:36 PM, Sanjay Patel via llvm-dev wrote:
>
> I would have replied to this thread sooner, but I was busy adding more
> instcombines. :)
>
> The motivation for r289855 is in the commit msg (I'm out of the office
and
> can't look things up conveniently). Feel free to revert that and the
follow
> ups, however, if that patch caused a noticeable slowdown, then it suggests
> we have a bigger problem?...that's a simple matcher (no value tracking
> required).
>
>
> I'd recommend against reverting this until we understand more about the
> problem.
>
>
> I tend to agree, but I’d also like that we get better understanding before
> 4.0 branches.
>
> This is not a critical compile-time regression,
>
>
> I didn’t see in this thread a mention about how much alone this is causing?
> The regression mentioned by Davide (20%) seems critical to me.
>
> Maybe I misread this. I thought it was much smaller. We should double-check
> the data too. If this one commit is responsible for a 20% compile-time
> regression, that is indeed serious.
>
Sorry if it wasn't clear, but this is not really responsible for the
whole regression. When I said "I can confirm this regresses", I meant
that this regressed by a certain amount on one of my testcases. In
particular, the two revisions together accounted for about 2% in
compile time. So, yeah, probably not worth reverting but still a
couple of revisions to keep in mind for the future.

Thanks,

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare