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

[Mehdi Amini via llvm-dev]

> On Apr 18, 2016, at 2:24 PM, Easwaran Raman <eraman at google.com> wrote:
> 
> Thanks for the comments.
> 
> On Mon, Apr 18, 2016 at 2:13 PM, Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>> wrote:
> 
> > On Apr 18, 2016, at 2:07 PM, Hal Finkel via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> >
> > ----- Original Message -----
> >> From: "Easwaran Raman" <eraman at google.com <mailto:eraman at google.com>>
> >> To: "via llvm-dev" <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>>
> >> Cc: "Chandler Carruth" <chandlerc at gmail.com <mailto:chandlerc at gmail.com>>, "Hal Finkel" <hfinkel at anl.gov <mailto:hfinkel at anl.gov>>, "Philip Reames"
> >> <listmail at philipreames.com <mailto:listmail at philipreames.com>>, "David Li" <davidxl at google.com <mailto: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.
> Yes, I meant that it is not an analysis pass. It does perform analysis. I see one such example of something that performs analysis being added under Transforms/Utils: CmpInstAnalysis.cpp.

If CmpInstAnalysis.cpp is not mutating the IR, it shouldn't sit here in my opinion.


> 
> >
> >>
> >> 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 think that is due to ThinLTO's use of  getBlockProfileCount.

Yeah, I know but we should find a way to break this...



> I'm not sure about ProfileData that depends on Bitcode, do you know why?
> ProfileData (specifically CoverageMapping{Reader|Writer}) depends on Object which depends on Bitcode.

Some refactoring or split could help here, this does not seem desirable to me.

-- 
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 <mailto:llvm-dev at lists.llvm.org>
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <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/7b3343f7/attachment.html>