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

[Easwaran Raman via llvm-dev]

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

>
> 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>
> 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.
>>
> 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.
>
Does moving the CoverageMapping code to a Coverage library seems
reasonable? The rest of ProfileData does not depend on this or on Object
and thgis would break the ProfileData->Analysis dependence.


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