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

[Chandler Carruth via llvm-dev]

On Mon, Apr 18, 2016 at 2:46 PM Hal Finkel <hfinkel at anl.gov> wrote:

>
>
> ------------------------------
>
> *From: *"Chandler Carruth" <chandlerc at gmail.com>
> *To: *"Xinliang David Li" <davidxl at google.com>
> *Cc: *"Mehdi Amini" <mehdi.amini at apple.com>, "Hal Finkel" <hfinkel at anl.gov>,
> "via llvm-dev" <llvm-dev at lists.llvm.org>
> *Sent: *Monday, April 18, 2016 4:31:05 PM
> *Subject: *Re: [llvm-dev] Move InlineCost.cpp out of Analysis?
>
>
>
> 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.
>
> I'm not sure the situation is as clear cut as you make it out to be. We
> can factor common components out of different transformations (e.g. out of
> different inliner implementations), including common components that don't
> modify the IR, without that component forming what I would generally
> consider an analysis. In this case, we're really talking about the
> encapsulation of the inliner's cost heuristic, and the generality of this
> information is questionable. To put it another way, while I'm fine with
> someone reusing this logic to build a new inliner, I'd be very skeptical of
> using it for any other purpose. That's why it does not really feel like an
> analysis to me. This is not a strong feeling, however, and I'm also fine
> with it staying in Analysis.
>

I don't think that everything that is an analysis needs to be completely
general though. Certainly, we need to be very clear and careful in what
terms we describe the API to make the quality and nature of the results
unsurprising, but I think that the API around the inline cost is reasonably
good about that -- it seems to pretty clearly indicate that this is a
threshold determining mechanism and not something that provides a holistic
view of the impact of a potential inlining decision.

I actually think it would be good if essentially *everything* we can put a
reasonable public API around and which doesn't mutate the IR were moved to
the analysis library. As an example of why I think this, without this it is
very hard to know what code can be re-used in analysis passes. And the
constraint there is really exactly this: it has a public API that can be
re-used, and it doesn't mutate the IR. Nothing more or less that I see.


Naturally, we don't need to go on a campaign to move everything *right
now*, and we can even be fairly lazy about it, but I do think that this
direction is the correct long-term design of the libraries here.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160418/064bae49/attachment.html>