Chandler Carruth via llvm-dev
2016-Apr-18 22:59 UTC
[llvm-dev] Move InlineCost.cpp out of Analysis?
On Mon, Apr 18, 2016 at 3:45 PM Xinliang David Li <davidxl at google.com> wrote:> On Mon, Apr 18, 2016 at 3:00 PM, Chandler Carruth <chandlerc at gmail.com> > 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: >> > > Not sure about what you mean by 'lost track of'. >I mean that we used to hold to these invariants but I think recently the code has started to not follow them as closely.> >> 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. >> > > > This is not the case as of today. >Again, my whole comment was that these are no longer being correctly followed.> BPI is a dedicated analysis pass to manage branch probability profile > information, but this pass is only used in limited situations (e.g, for > BFI, profile update in jump-threading etc) -- using it it requires more > memory as well as incremental update interfaces. Many transformation > passes simply skip it and directly access the meta data in IR. >Can you be more specific? BPI and BFI are used in *many* places, and on an initial inspection almost everywhere that accesses MD_prof directly appears to do so in order to *set* or *update* the profile information without doing detailed analysis on it. Which seems fine with my outline of the invariants?> >> Now, the original design only accounted for profile information *within* >> a function body, clearly it needs to be extended to support intraprocedural >> information. >> > > > Not sure what you mean. Profile data in general does not extend to IPA > (we will reopen discussion on that soon), but profile summary is > 'invariant'/readonly data, which should be available to IPA already. >I don't know what you mean by "invariant" or readonly data here. I think that whether or not the profile information is mutated shouldn't influence the design invariants I described above.>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160418/c82b4170/attachment.html>
Xinliang David Li via llvm-dev
2016-Apr-18 23:38 UTC
[llvm-dev] Move InlineCost.cpp out of Analysis?
On Mon, Apr 18, 2016 at 3:59 PM, Chandler Carruth <chandlerc at gmail.com> wrote:> On Mon, Apr 18, 2016 at 3:45 PM Xinliang David Li <davidxl at google.com> > wrote: > >> On Mon, Apr 18, 2016 at 3:00 PM, Chandler Carruth <chandlerc at gmail.com> >> 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: >>> >> >> Not sure about what you mean by 'lost track of'. >> > > I mean that we used to hold to these invariants but I think recently the > code has started to not follow them as closely. > > >> >>> 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. >>> >> >> >> This is not the case as of today. >> > > Again, my whole comment was that these are no longer being correctly > followed. > > >> BPI is a dedicated analysis pass to manage branch probability profile >> information, but this pass is only used in limited situations (e.g, for >> BFI, profile update in jump-threading etc) -- using it it requires more >> memory as well as incremental update interfaces. Many transformation >> passes simply skip it and directly access the meta data in IR. >> > > Can you be more specific? > > BPI and BFI are used in *many* places, and on an initial inspection almost > everywhere that accesses MD_prof directly appears to do so in order to > *set* or *update* the profile information without doing detailed analysis > on it. Which seems fine with my outline of the invariants? > >See my reply to Hal.> >>> Now, the original design only accounted for profile information *within* >>> a function body, clearly it needs to be extended to support intraprocedural >>> information. >>> >> >> >> Not sure what you mean. Profile data in general does not extend to IPA >> (we will reopen discussion on that soon), but profile summary is >> 'invariant'/readonly data, which should be available to IPA already. >> > > I don't know what you mean by "invariant" or readonly data here. I think > that whether or not the profile information is mutated shouldn't influence > the design invariants I described above. >I do not disagree with this. What I was saying is that the information can be made available to IPA in some form due to its readonly nature. David -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160418/d42d3cac/attachment.html>
Hal Finkel via llvm-dev
2016-Apr-18 23:43 UTC
[llvm-dev] Move InlineCost.cpp out of Analysis?
----- Original Message -----> From: "Xinliang David Li" <davidxl at google.com> > To: "Chandler Carruth" <chandlerc at gmail.com> > Cc: "Hal Finkel" <hfinkel at anl.gov>, "via llvm-dev" > <llvm-dev at lists.llvm.org>, "Mehdi Amini" <mehdi.amini at apple.com> > Sent: Monday, April 18, 2016 6:38:32 PM > Subject: Re: [llvm-dev] Move InlineCost.cpp out of Analysis?> On Mon, Apr 18, 2016 at 3:59 PM, Chandler Carruth < > chandlerc at gmail.com > wrote:> > On Mon, Apr 18, 2016 at 3:45 PM Xinliang David Li < > > davidxl at google.com > wrote: >> > > On Mon, Apr 18, 2016 at 3:00 PM, Chandler Carruth < > > > chandlerc at gmail.com > 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: > > > > > > > > > Not sure about what you mean by 'lost track of'. > > > > > I mean that we used to hold to these invariants but I think > > recently > > the code has started to not follow them as closely. >> > > > 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. > > > > > > > > > This is not the case as of today. > > > > > Again, my whole comment was that these are no longer being > > correctly > > followed. >> > > BPI is a dedicated analysis pass to manage branch probability > > > profile > > > information, but this pass is only used in limited situations > > > (e.g, > > > for BFI, profile update in jump-threading etc) -- using it it > > > requires more memory as well as incremental update interfaces. > > > Many > > > transformation passes simply skip it and directly access the meta > > > data in IR. > > > > > Can you be more specific? >> > BPI and BFI are used in *many* places, and on an initial inspection > > almost everywhere that accesses MD_prof directly appears to do so > > in > > order to *set* or *update* the profile information without doing > > detailed analysis on it. Which seems fine with my outline of the > > invariants? >> See my reply to Hal.> > > > Now, the original design only accounted for profile information > > > > *within* a function body, clearly it needs to be extended to > > > > support > > > > intraprocedural information. > > > > > > > > > Not sure what you mean. Profile data in general does not extend > > > to > > > IPA (we will reopen discussion on that soon), but profile summary > > > is > > > 'invariant'/readonly data, which should be available to IPA > > > already. > > > > > I don't know what you mean by "invariant" or readonly data here. I > > think that whether or not the profile information is mutated > > shouldn't influence the design invariants I described above. > > I do not disagree with this. What I was saying is that the > information can be made available to IPA in some form due to its > readonly nature.Can you please clarify what information you view as readonly? I can understand function entry counts being readonly, but branch information within a function seems mutable. Is this also what you're talking about? -Hal> David-- 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/907791eb/attachment.html>
Chandler Carruth via llvm-dev
2016-Apr-19 00:14 UTC
[llvm-dev] Move InlineCost.cpp out of Analysis?
On Mon, Apr 18, 2016 at 4:38 PM Xinliang David Li <davidxl at google.com> wrote:> >>>> Now, the original design only accounted for profile information >>>> *within* a function body, clearly it needs to be extended to support >>>> intraprocedural information. >>>> >>> >>> >>> Not sure what you mean. Profile data in general does not extend to IPA >>> (we will reopen discussion on that soon), but profile summary is >>> 'invariant'/readonly data, which should be available to IPA already. >>> >> >> I don't know what you mean by "invariant" or readonly data here. I think >> that whether or not the profile information is mutated shouldn't influence >> the design invariants I described above. >> > > I do not disagree with this. What I was saying is that the information can > be made available to IPA in some form due to its readonly nature. >While it can be made available, it is very hard to make it available even in a readonly form in the current pass manager. You essentially have to avoid caching anything and make the API always re-examine the IR annotations in order to reflect changes to them. There are a few other "Immutable" analysis passes that abuse the legacy pass manager in this way. That seems fine as a temporary thing. I don't *think* this kind of information would pose a significant compile time cost to recompute on each query, but I've not looked at the nature of the IPA queries you would want to make. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160419/7b50ce90/attachment.html>