[llvm-dev] RFC: Mark BasicAA as a CFG-only pass.

Showing 1-6 of 6 messages
[llvm-dev] RFC: Mark BasicAA as a CFG-only pass. Alina Sbirlea via llvm-dev 2/10/20 11:34 AM
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.

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), PhiValuesAnalysis is also being updated in MemDepAnalysis (D48489and 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
Re: [llvm-dev] RFC: Mark BasicAA as a CFG-only pass. Alina Sbirlea via llvm-dev 2/10/20 12:36 PM
Hi,

Here's a tentative patch of the changes for this: D74353.

Thank you,
Alina

Re: [llvm-dev] RFC: Mark BasicAA as a CFG-only pass. Hal Finkel via llvm-dev 2/10/20 1:48 PM


On 2/10/20 2:35 PM, Alina Sbirlea wrote:
Hi,

Here's a tentative patch of the changes for this: 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

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory
Re: [llvm-dev] RFC: Mark BasicAA as a CFG-only pass. Alina Sbirlea via llvm-dev 2/10/20 2:10 PM
On Mon, Feb 10, 2020 at 1:48 PM Hal Finkel <hfi...@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.


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

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), PhiValuesAnalysis is also being updated in MemDepAnalysis (D48489and 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
Re: [llvm-dev] RFC: Mark BasicAA as a CFG-only pass. Hal Finkel via llvm-dev 2/10/20 3:10 PM


On 2/10/20 4:09 PM, Alina Sbirlea wrote:
On Mon, Feb 10, 2020 at 1:48 PM Hal Finkel <hfi...@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.


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

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), PhiValuesAnalysis is also being updated in MemDepAnalysis (D48489and 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
Re: [llvm-dev] RFC: Mark BasicAA as a CFG-only pass. Alina Sbirlea via llvm-dev 2/10/20 3:59 PM
Got it, I updated the patch. Please let me know fi I should detail it further.

Thank you,
Alina