llvm dev - Apr 2016 - 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?

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). 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20160418/d8090250/attachment.html>

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

 -Hal
> 
> 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). 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

> 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