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

[Hal Finkel via llvm-dev]

----- Original Message -----

> From: "Xinliang David Li" <davidxl at google.com>
> To: "Mehdi Amini" <mehdi.amini at apple.com>
> Cc: "Chandler Carruth" <chandlerc at gmail.com>, "Hal Finkel"
> <hfinkel at anl.gov>, "via llvm-dev" <llvm-dev at lists.llvm.org>
> Sent: Monday, April 18, 2016 4:38:12 PM
> Subject: Re: [llvm-dev] Move InlineCost.cpp out of Analysis?

> On Mon, Apr 18, 2016 at 2:33 PM, Mehdi Amini < mehdi.amini at apple.com
> > wrote:

> > > On 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) ? 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. For the
> > > same
> > > reason, we need to ask first if InlineCost is intended to be a
> > > shared utility?
> > 
> 
> > 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. 

-Hal 

> David

> > Moving something from Analysis to TransformUtils just because it
> > needs to access ProfileData is a slippery slope...
> 

> > --
> 
> > Mehdi
> 

> > > 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
> > > > 
> > > 
> > 
> 

-- 

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/fd9f985d/attachment.html>