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

[Xinliang David Li via llvm-dev]

What you said makes sense.

David


On Mon, Apr 18, 2016 at 2:31 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> 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/862552bf/attachment-0001.html>