[llvm-dev] Move InlineCost.cpp out of Analysis?

Mon Apr 18 14:31:05 PDT 2016

On Mon, Apr 18, 2016 at 2:28 PM Xinliang David Li <davidxl at google.com>
wrote:

> On Mon, Apr 18, 2016 at 2:18 PM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
>> The difference between Analysis and Transforms is *not* about passes, but
>> about what the code *does*.
>>
>> Code for mutating the IR should be in Transforms, and code that analyzes
>> the IR without mutating it should be in Analysis. This is why, for example,
>> InstructionSimplify is in Analysis -- it does not mutate the IR in any way.
>>
>> So I think InlineCost and similar things should stay in the Analysis
>> library regardless of whether they are passes or not.
>>
>
> Is that the only criteria (IR mod or not) ?
>

Given a public API, yes, that should be the only criteria IMO.

> Most of the transformations have pass specific analysis (that are not
> shared with other clients) -- those code stay with the transformation -- it
> does not make sense to move those code in to Analysis.
>

But I would expect these to also not expose public APIs to the analyses.
Provided the public API is limited to the transformation, I think the code
be closely located makes sense.

>   For the same reason, we need to ask first if InlineCost is intended to
> be a shared utility?
>

Sure, but in fact at least some parts of it are shared.

There is also some reason to prefer surfacing nice high-level public APIs
when reasonable to do so. For example, if someone wanted to write a custom
inlining pass that re-used the cost analysis, having it separated seems
useful. So unless there is a significant problem solved by sinking this to
be a private API only used by the inliner, I would keep it as-is.

>
> David
>
>
>
>>
>> On Mon, Apr 18, 2016 at 2:14 PM Mehdi Amini via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>>
>>> > On Apr 18, 2016, at 2:07 PM, Hal Finkel via llvm-dev <
>>> llvm-dev at lists.llvm.org> wrote:
>>> >
>>> > ----- Original Message -----
>>> >> From: "Easwaran Raman" <eraman at google.com>
>>> >> To: "via llvm-dev" <llvm-dev at lists.llvm.org>
>>> >> Cc: "Chandler Carruth" <chandlerc at gmail.com>, "Hal Finkel" <
>>> hfinkel at anl.gov>, "Philip Reames"
>>> >> <listmail at philipreames.com>, "David Li" <davidxl at google.com>
>>> >> Sent: Monday, April 18, 2016 2:39:49 PM
>>> >> Subject: Move InlineCost.cpp out of Analysis?
>>> >>
>>> >>
>>> >> Hi,
>>> >>
>>> >>
>>> >> After r256521 - which removes InlineCostAnalysis class - I think
>>> >> there is no strong reason for InlineCost.cpp to be part of the
>>> >> Analysis library. Is it fine to make it part of TransformUtils?
>>> >>
>>> >
>>> > Given that InlineCost is not really an analysis any longer, I think
>>> this is fine.
>>>
>>> Isn't it? It is not a pass, but I see it as an analysis utils.
>>>
>>> >
>>> >>
>>> >> I submitted r266477 (which has now been reverted) that made Analysis
>>> >> depend on ProfileData in order to obtain ProfileSummary for the
>>> >> module, but there is an existing dependency of ProfileData on
>>> >> Analysis (through Object and BitCode).
>>>
>>> The real issue is that BitCode depends on Analysis I think.
>>> I'm not sure about ProfileData that depends on Bitcode, do you know why?
>>>
>>> --
>>> Mehdi
>>>
>>>
>>> >> Moving InlineCost.cpp under
>>> >> Transforms/Utils will fix this issue. There are other ways to fix
>>> >> this (make Inliner.cpp get the ProfileSummary and pass it to
>>> >> InlineCost, for example), but I think it makes sense to move
>>> >> InlineCost.
>>> >>
>>> >>
>>> >> Thoughts?
>>> >>
>>> >>
>>> >> Thanks,
>>> >> Easwaran
>>> >>
>>> >>
>>> >
>>> > --
>>> > Hal Finkel
>>> > Assistant Computational Scientist
>>> > Leadership Computing Facility
>>> > Argonne National Laboratory
>>> > _______________________________________________
>>> > LLVM Developers mailing list
>>> > 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
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160418/16c7f143/attachment.html>