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

[Xinliang David Li via llvm-dev]

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

>
>
> ------------------------------
>
> *From: *"Chandler Carruth" <chandlerc at gmail.com>
> *To: *"Easwaran Raman" <eraman at google.com>
> *Cc: *"Hal Finkel" <hfinkel at anl.gov>, "Xinliang David Li" <
> davidxl at google.com>, "via llvm-dev" <llvm-dev at lists.llvm.org>
> *Sent: *Monday, April 18, 2016 5:25:03 PM
> *Subject: *Re: [llvm-dev] Move InlineCost.cpp out of Analysis?
>
> On Mon, Apr 18, 2016 at 3:20 PM Easwaran Raman <eraman at google.com> wrote:
>
>> On Mon, Apr 18, 2016 at 3:00 PM, Chandler Carruth via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> On Mon, Apr 18, 2016 at 2:48 PM Hal Finkel <hfinkel at anl.gov> wrote:
>>>
>>>>
>>>>
>>>> ------------------------------
>>>>
>>>> *From: *"Xinliang David Li" <davidxl at google.com>
>>>>
>>>> On Mon, Apr 18, 2016 at 2:33 PM, Mehdi Amini <mehdi.amini at apple.com>
>>>> wrote:
>>>>>
>>>>> In the current case at stake: the issue is that we can't make the
>>>>> Analysis library using anything from the ProfileData library. Conceptually
>>>>> there is a problem IMO.
>>>>>
>>>>
>>>>
>>>> Yes -- this is a very good point.
>>>>
>>>> Independent of anything else, +1.
>>>>
>>>
>>> The design of ProfileData and reading profile information in the entire
>>> middle end had a really fundamental invariant that folks seem to have lost
>>> track of:
>>>
>>> a) There is exactly *one* way to get at profile information from general
>>> analyses and transforms: a dedicated analysis pass that manages access to
>>> the profile info.
>>>
>>> b) There is exactly *one* way for this analysis to compute this
>>> information from an *external* profile source: profile metadata attached to
>>> the IR.
>>>
>>> c) There could be many external profile sources, but all of them should
>>> be read and then translated into metadata annotations on the IR so that
>>> serialization / deserialization preserve them in a common format and we can
>>> reason about how they work.
>>>
>>>
>>> This layering is why it is only a transform that accesses ProfileData --
>>> it is responsible for annotating the IR and nothing else. Then the
>>> analysis uses these annotations and never reads the data directly.
>>>
>>> I think this is a really important separation of concerns as it ensures
>>> that we don't get an explosion of different analyses supporting various
>>> different subsets of profile sources.
>>>
>>>
>>> Now, the original design only accounted for profile information *within*
>>> a function body, clearly it needs to be extended to support intraprocedural
>>> information. But I would still expect that to follow a similar layering
>>> where we first read the data into IR annotations, then have an analysis
>>> pass (this time a module analysis pass in all likelihood) that brokers
>>> access to these annotations through an API that can do intelligent things
>>> like synthesizing it from the "cold" attribute or whatever when missing.
>>>
>>>
>> Invariants b) and c) above are still true, but a) is not since InlineCost
>> directly calls ProfileSummary instead of through an analysis pass.
>>
>
> Not quite -- ProfileSummary seems to only exist in the profile *reading*
> code, so it isn't *just* an annotation on the IR.
>
> I think the problem here is that we have failed to build a proper separate
> abstraction around the interprocedural profile data that is extracted from
> IR annotations. That abstraction should have nothing to do with reading
> profile data and so shouldn't live in the ProifileData library, it should
> (IMO) live in the Analysis library.
>
>   I'll add a module level pass as you suggest, but that still needs
>> breaking the ProfileData->Analysis dependence chain.
>>
>
> Well, this will also re-highlight the fundamental pass manager problem as
> you won't have easy access to this analysis pass.
>
> Could this be an immutable pass?
>

I think yes, but this is independent of the issue at hand -- removing the
circular dependency introduced (due to dep of cov mapping on bitcode writer
....). Easwaran is working on that without requiring moving
InlineCostAnalysis.

David


>
>
>  -Hal
>
>
>
>>
>> - Easwaran
>>
>>> -Chandler
>>>
>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>>
>
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160418/30c9afec/attachment-0001.html>