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

[Easwaran Raman via llvm-dev]

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

> 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.
>
Sorry, I'm lost here. There is an IR annotation (module flag) called
ProfileSummary and this data is represented in memory by the ProfileSummary
class. . This class  provides methods to serialize/de-serialize  this data
into/from metadata. This class has methods to compute this summary, but
this is used only by the profile readers and writers. I am not sure what
you mean by "it isn't just* an annotation on the IR".

- Easwaran


>
> 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.
>
>
>
>>
>> - Easwaran
>>
>>> -Chandler
>>>
>>
>>> _______________________________________________
>>> 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/a4a8f5d1/attachment.html>