Hal Finkel via llvm-dev
2020-Feb-10 21:48 UTC
[llvm-dev] RFC: Mark BasicAA as a CFG-only pass.
On 2/10/20 2:35 PM, Alina Sbirlea wrote:> Hi, > > Here's a tentative patch of the changes for this: D74353 > <https://reviews.llvm.org/D74353>.I suppose that, as expected, it's invalidated less often this way. Given that it's generally stateless, does this really represent a cost savings? -Hal> > Thank you, > Alina > > > On Mon, Feb 10, 2020 at 11:34 AM Alina Sbirlea > <alina.sbirlea at gmail.com <mailto:alina.sbirlea at gmail.com>> wrote: > > Hi, > > I'd like to understand if it makes sense to keep BasicAA as a not > CFG-only pass, or if it can be updated to CFG-only. The change was > made in D44564 <https://reviews.llvm.org/D44564>. > > From what I gathered the motivation was PhiValuesAnalysis not > being properly updated, and BasicAA having an instance of it. > PhiValuesAnalysis now uses callback values to invalidate deleted > values (r340613 > <https://reviews.llvm.org/rL340613>),PhiValuesAnalysis is also > being updated in MemDepAnalysis (D48489 > <https://reviews.llvm.org/D48489>) and BasicAA is invalidated if > PhiValuesAnalysis gets invalidated. > > I may not have the full context here, so I'd like some feedback: > does it make sense to make BasicAA a CFG-only pass again? > > Thank you, > Alina >-- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200210/c01de515/attachment.html>
Alina Sbirlea via llvm-dev
2020-Feb-10 22:09 UTC
[llvm-dev] RFC: Mark BasicAA as a CFG-only pass.
On Mon, Feb 10, 2020 at 1:48 PM Hal Finkel <hfinkel at anl.gov> wrote:> > On 2/10/20 2:35 PM, Alina Sbirlea wrote: > > Hi, > > Here's a tentative patch of the changes for this: D74353 > <https://reviews.llvm.org/D74353>. > > > I suppose that, as expected, it's invalidated less often this way. Given > that it's generally stateless, does this really represent a cost savings? > > -Hal >I don't think there is a cost savings now, the current patch shouldn't have much or any impact. But I think this can affect future infrastructure changes. I got here by looking at MemCpyOpt, as part of a longer term goal of having the pass sequence [GVN, MemCpyOpt, DSE] use MemorySSA instead of MemDepAnalysis. Currently we're explicitly checking that, after MemCpyOpt, BasicAA is freed. This wouldn't make sense when using MemorySSA in all 3 passes, as we'd end up rebuilding MemorySSA due to invalidating BasicAA. I kept the changes needed to MemCpyOpt to preserve passes separate from this discussion. Does this clarify some of the motivation? Alina> > > Thank you, > Alina > > > On Mon, Feb 10, 2020 at 11:34 AM Alina Sbirlea <alina.sbirlea at gmail.com> > wrote: > >> Hi, >> >> I'd like to understand if it makes sense to keep BasicAA as a not >> CFG-only pass, or if it can be updated to CFG-only. The change was made in >> D44564 <https://reviews.llvm.org/D44564>. >> >> From what I gathered the motivation was PhiValuesAnalysis not being >> properly updated, and BasicAA having an instance of it. >> PhiValuesAnalysis now uses callback values to invalidate deleted values ( >> r340613 <https://reviews.llvm.org/rL340613>), PhiValuesAnalysis is also >> being updated in MemDepAnalysis (D48489 <https://reviews.llvm.org/D48489> >> ) and BasicAA is invalidated if PhiValuesAnalysis gets invalidated. >> >> I may not have the full context here, so I'd like some feedback: does it >> make sense to make BasicAA a CFG-only pass again? >> >> Thank you, >> Alina >> > -- > Hal Finkel > Lead, Compiler Technology and Programming Languages > Leadership Computing Facility > Argonne National Laboratory > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200210/750c9d39/attachment.html>
Hal Finkel via llvm-dev
2020-Feb-10 23:09 UTC
[llvm-dev] RFC: Mark BasicAA as a CFG-only pass.
On 2/10/20 4:09 PM, Alina Sbirlea wrote:> On Mon, Feb 10, 2020 at 1:48 PM Hal Finkel <hfinkel at anl.gov > <mailto:hfinkel at anl.gov>> wrote: > > > On 2/10/20 2:35 PM, Alina Sbirlea wrote: >> Hi, >> >> Here's a tentative patch of the changes for this: D74353 >> <https://reviews.llvm.org/D74353>. > > > I suppose that, as expected, it's invalidated less often this way. > Given that it's generally stateless, does this really represent a > cost savings? > > -Hal > > > I don't think there is a cost savings now, the current patch shouldn't > have much or any impact. > But I think this can affect future infrastructure changes. > I got here by looking at MemCpyOpt, as part of a longer term goal of > having the pass sequence [GVN, MemCpyOpt, DSE] use MemorySSA instead > of MemDepAnalysis. Currently we're explicitly checking that, > after MemCpyOpt, BasicAA is freed. This wouldn't make sense when using > MemorySSA in all 3 passes, as we'd end up rebuilding MemorySSA due to > invalidating BasicAA. > I kept the changes needed to MemCpyOpt to preserve passes separate > from this discussion. > Does this clarify some of the motivation?Yes, please add this information to the patch description. I think that it makes sense to mark BasicAA as not being invalidated on non-CFG-altering IR changes if it's not. -Hal> > Alina > > >> >> Thank you, >> Alina >> >> >> On Mon, Feb 10, 2020 at 11:34 AM Alina Sbirlea >> <alina.sbirlea at gmail.com <mailto:alina.sbirlea at gmail.com>> wrote: >> >> Hi, >> >> I'd like to understand if it makes sense to keep BasicAA as a >> not CFG-only pass, or if it can be updated to CFG-only. The >> change was made in D44564 <https://reviews.llvm.org/D44564>. >> >> From what I gathered the motivation was PhiValuesAnalysis not >> being properly updated, and BasicAA having an instance of it. >> PhiValuesAnalysis now uses callback values to invalidate >> deleted values (r340613 >> <https://reviews.llvm.org/rL340613>),PhiValuesAnalysis is >> also being updated in MemDepAnalysis (D48489 >> <https://reviews.llvm.org/D48489>) and BasicAA is invalidated >> if PhiValuesAnalysis gets invalidated. >> >> I may not have the full context here, so I'd like some >> feedback: does it make sense to make BasicAA a CFG-only pass >> again? >> >> Thank you, >> Alina >> > -- > Hal Finkel > Lead, Compiler Technology and Programming Languages > Leadership Computing Facility > Argonne National Laboratory >-- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200210/9853ea10/attachment.html>