Page MenuHomePhabricator

asbirlea (Alina Sbirlea)
Google

Projects

User does not belong to any projects.

User Details

User Since
Feb 10 2016, 9:51 AM (333 w, 2 d)

Recent Activity

Wed, Jun 22

asbirlea accepted D128374: [docs][NewPM] Add more info on why accessing mutable outer analyses is disallowed.

Thank you for adding this information!!

Wed, Jun 22, 11:15 PM · Restricted Project, Restricted Project

Tue, Jun 21

asbirlea accepted D128128: [GlobalOpt] Perform store->dominated load forwarding for stored once globals.

Latest diff looks good to me.

Tue, Jun 21, 6:13 PM · Restricted Project, Restricted Project
asbirlea accepted D123686: [DSE] Revisit pointers that may no longer escape after removing another store.

LGTM.

Tue, Jun 21, 2:42 PM · Restricted Project, Restricted Project

May 24 2022

asbirlea accepted D125205: Replace the custom linked list in LeaderTableEntry with TinyPtrVector..

Changes LGTM with inline nits.
@nikic I have not verified compile time impact with the latest diff.

May 24 2022, 1:49 PM · Restricted Project, Restricted Project

May 17 2022

asbirlea accepted D125808: fix typo error in DivergenceAnalysis.h.
May 17 2022, 10:02 AM · Restricted Project, Restricted Project

May 16 2022

asbirlea added a comment to D100486: [COST]Improve cost model for shuffles in SLP..

Following up on the regression I was seeing with https://reviews.llvm.org/D114171, the only meaningful IR change seems to be:
Before the patch (after slp):

%65 = shufflevector <2 x float> %9, <2 x float> poison, <4 x i32> <i32 0, i32 1, i32 undef, i32 undef>
%66 = insertelement <4 x float> %65, float %16, i32 2  
%67 = insertelement <4 x float> %66, float %15, i32 3
%68 = call <4 x float> @llvm.fabs.v4f32(<4 x float> %67)
%69 = fcmp oeq <4 x float> %68, <float 0x7FF0000000000000, float 0x7FF0000000000000, float 0x7FF0000000000000, float 0x7FF0000000000000>
%70 = freeze <4 x i1> %69 
%71 = call i1 @llvm.vector.reduce.or.v4i1(<4 x i1> %70)
May 16 2022, 3:19 PM · Restricted Project, Restricted Project

May 12 2022

asbirlea added a comment to D100780: [Passes] Add extra LoopSimplifyCFG run after IndVarSimplify..

This could be done by either exposing a simplifyLoopCFG helper function ...

This variant sounds better, if possible.

+1, at least for this case where it's a simple cleanup

in other cases, such as D125293 and D115052 where we want to do more general cleanup due to added/lowered code, I believe @asbirlea has future plans to add new PM infra to conditionally run passes for simplification pipeline work

May 12 2022, 4:54 PM · Restricted Project, Restricted Project

May 10 2022

asbirlea added a comment to D100486: [COST]Improve cost model for shuffles in SLP..

We currently root-caused a 20% regression in eigen complex to this patch and it is blocking our compiler release.
I need some time to do additional testing, given there are at least a few patches on top of this. So far I see a crash fix, two NFCs and two additional changes that may also affect performance (https://reviews.llvm.org/D114171 and https://reviews.llvm.org/D115750).
Can you please hold off on any additional changes while we test further, or include them here for review so I can check if they resolve the regressions seen so far?

Most probably, some overoptimistic cost model estimations turned on some extra vectorization. The reproducer will help, if you can provide it.

The reproducer is open source: https://gitlab.com/libeigen/eigen
Regressions are on haswell platform, xfdo configuration; the compiler used is bootstrapped against itself; revision tested includes this patch and all follow-up changes to the SLPVectorizer including D114171 and excluding D115750.
A few number samples:
BM_MatrixVectorMultiply_Complex_EigenRowMajorDynamic_float_16x64_ 16%
BM_MatrixVectorMultiply_Complex_EigenRowMajorDynamic_float_64x64_ 17%
BM_MatrixVectorMultiply_Complex_EigenRowMajorFixed_float_64x64_ 15%
BM_MatrixVectorMultiply_Complex_EigenRowMajorFixed_float_16x64_ 14%

Could you at least extract the llvm ir code for one of these functions before the changes and after, so I coukd at least compare them?

May 10 2022, 12:05 PM · Restricted Project, Restricted Project
asbirlea accepted D125328: [BasicAA] Fix order in which we pass MemoryLocations to alias().
May 10 2022, 11:36 AM · Restricted Project, Restricted Project

May 9 2022

asbirlea added a comment to D124284: [SLP]Try partial store vectorization if supported by target..

@asbirlea How are you specifying the SSE/AVX level for your benchmark runs - are you running with -march=native the x86-64-v* levels or something else?

I believe the runs are using -target-cpu k8 and -target-cpu haswell.

The latest performance testing still shows one regression in a benchmark from singlesource, but there are many improvements that offset it in non-public benchmarks. So this latest diff is good to go from my side.

Hi, thanks for the testing. What's the name of the regressed single source test?

May 9 2022, 11:26 AM · Restricted Project, Restricted Project

May 8 2022

asbirlea added a comment to D124284: [SLP]Try partial store vectorization if supported by target..

@asbirlea How are you specifying the SSE/AVX level for your benchmark runs - are you running with -march=native the x86-64-v* levels or something else?

May 8 2022, 10:14 PM · Restricted Project, Restricted Project

May 5 2022

asbirlea added a comment to D100486: [COST]Improve cost model for shuffles in SLP..

We currently root-caused a 20% regression in eigen complex to this patch and it is blocking our compiler release.
I need some time to do additional testing, given there are at least a few patches on top of this. So far I see a crash fix, two NFCs and two additional changes that may also affect performance (https://reviews.llvm.org/D114171 and https://reviews.llvm.org/D115750).
Can you please hold off on any additional changes while we test further, or include them here for review so I can check if they resolve the regressions seen so far?

Most probably, some overoptimistic cost model estimations turned on some extra vectorization. The reproducer will help, if you can provide it.

May 5 2022, 2:46 PM · Restricted Project, Restricted Project
asbirlea added a comment to D100486: [COST]Improve cost model for shuffles in SLP..

We currently root-caused a 20% regression in eigen complex to this patch and it is blocking our compiler release.
I need some time to do additional testing, given there are at least a few patches on top of this. So far I see a crash fix, two NFCs and two additional changes that may also affect performance (https://reviews.llvm.org/D114171 and https://reviews.llvm.org/D115750).
Can you please hold off on any additional changes while we test further, or include them here for review so I can check if they resolve the regressions seen so far?

May 5 2022, 11:20 AM · Restricted Project, Restricted Project
asbirlea added a comment to D124284: [SLP]Try partial store vectorization if supported by target..

@asbirlea Do you have an update on the regressions you were seeing vs latest patch?

May 5 2022, 10:03 AM · Restricted Project, Restricted Project

May 3 2022

asbirlea added a comment to D124284: [SLP]Try partial store vectorization if supported by target..

I'm seeing some fairly big regressions with this patch, specifically on Rome (AMD) architecture.
A couple of examples that are public in the test suite: SingleSource/Benchmarks/Shootout: for sieve I'm seeing a 20% performance regression in an opt build and an xfdo one, and for MicroBenchmarks/ImageProcessing/Dither 10% regression (opt, thinlto and xfdo).
I'm seeing also a couple on Skylake, opt build, in the range of 5-13 %, an example being eigen with 13% regression; this may be harder to track down as it's in a specific configuration but let me know if you want to reproduce this one.

May 3 2022, 10:52 AM · Restricted Project, Restricted Project

Apr 25 2022

asbirlea added a comment to D124284: [SLP]Try partial store vectorization if supported by target..

Noted, I will run some testing this week.

Apr 25 2022, 5:04 PM · Restricted Project, Restricted Project
asbirlea accepted D124376: [Passes] Remove legacy LoopUnswitch pass..

Thank you for the cleanup!
Maybe worth a mention that this effectively enables SimpleLoopUnswitch for the legacy PM, in case any users need this clarification; more of a nit, considering the LPM is being removed.

Apr 25 2022, 10:38 AM · Restricted Project, Restricted Project

Apr 22 2022

asbirlea added a comment to D124251: [SimpleLoopUnswitch] Run LICM for nested unswitching tests..

As a side note, the fact the freeze instruction needs licm and simple loop unswitch interleaved, will likely affect the plan we had around splitting off the non-trivial unswitch functionality into a separate function pass (the context for that is regarding using divergence analysis: https://reviews.llvm.org/D109762)
It seems like tracking the added freeze instructions and including the functionality of hoisting them as part of the "new" non-trivial SLU pass will resolve this, but I'd like to hear folks feedback on this.

Apr 22 2022, 1:01 PM · Restricted Project, Restricted Project
asbirlea added a comment to D124251: [SimpleLoopUnswitch] Run LICM for nested unswitching tests..

+ 1 to please clarify in the test why this is necessary.
Without the context, the question will be: why not generate the IR after LICM and only test SLU? Clarify the test needs the sequence SLU on inner loop, licm to hoist freeze condition and then SLU on outer loop can be done.

Apr 22 2022, 12:18 PM · Restricted Project, Restricted Project

Apr 7 2022

asbirlea added a comment to D123288: [LoopSink] Require MemorySSA.

Added nits, but happy if this goes in as is too.

Apr 7 2022, 2:51 PM · Restricted Project, Restricted Project
asbirlea accepted D123288: [LoopSink] Require MemorySSA.

This cleanup is beautiful!
Thank you!!

Apr 7 2022, 1:30 PM · Restricted Project, Restricted Project

Apr 6 2022

asbirlea committed rG50d41f3e0daa: [MSSA] Print memory phis when inspecting walker. (authored by asbirlea).
[MSSA] Print memory phis when inspecting walker.
Apr 6 2022, 4:07 PM · Restricted Project, Restricted Project
asbirlea added a reverting change for rGf7381a795ab2: Revert 29fada4a3d3db309f11f7fa7a0c61cd4021e9947: rG08075a7ee881: Revert f7381a795ab235d34c94eaf01dc880eb5b89619d.
Apr 6 2022, 4:07 PM · Restricted Project, Restricted Project
asbirlea committed rG08075a7ee881: Revert f7381a795ab235d34c94eaf01dc880eb5b89619d (authored by asbirlea).
Revert f7381a795ab235d34c94eaf01dc880eb5b89619d
Apr 6 2022, 4:07 PM · Restricted Project, Restricted Project
asbirlea added a reverting change for D121987: [EarlyCSE] Don't eagerly optimize MemoryUses: rG08075a7ee881: Revert f7381a795ab235d34c94eaf01dc880eb5b89619d.
Apr 6 2022, 4:07 PM · Restricted Project, Restricted Project
asbirlea accepted D123216: [LoopSink] Use MemorySSA with legacy pass manager.

+1 to the additional cleanup.

Apr 6 2022, 9:55 AM · Restricted Project, Restricted Project

Apr 5 2022

asbirlea accepted D123162: [CaptureTracking] Ignore ephemeral values when determining pointer escapeness.
Apr 5 2022, 4:05 PM · Restricted Project, Restricted Project
asbirlea accepted D123126: [cmake] Remove LLVM_ENABLE_NEW_PASS_MANAGER cmake option.

Thank you!

Apr 5 2022, 1:11 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Mar 29 2022

asbirlea accepted D122601: Fix MemorySSAUpdater::insertDef for dead code.
Mar 29 2022, 10:44 AM · Restricted Project, Restricted Project
asbirlea added a comment to D122601: Fix MemorySSAUpdater::insertDef for dead code.

Looks good to me as well.

Mar 29 2022, 10:44 AM · Restricted Project, Restricted Project

Mar 28 2022

asbirlea added a reverting change for rG29fada4a3d3d: [EarlyCSE] Don't eagerly optimize MemoryUses: rGf7381a795ab2: Revert 29fada4a3d3db309f11f7fa7a0c61cd4021e9947.
Mar 28 2022, 4:19 PM · Restricted Project
asbirlea committed rGf7381a795ab2: Revert 29fada4a3d3db309f11f7fa7a0c61cd4021e9947 (authored by asbirlea).
Revert 29fada4a3d3db309f11f7fa7a0c61cd4021e9947
Mar 28 2022, 4:19 PM · Restricted Project, Restricted Project
asbirlea added a reverting change for D121987: [EarlyCSE] Don't eagerly optimize MemoryUses: rGf7381a795ab2: Revert 29fada4a3d3db309f11f7fa7a0c61cd4021e9947.
Mar 28 2022, 4:19 PM · Restricted Project, Restricted Project

Mar 24 2022

asbirlea accepted D122420: [opt] Remove -analyze option.

lgtm

Mar 24 2022, 11:35 AM · Restricted Project, Restricted Project

Mar 22 2022

asbirlea accepted D121987: [EarlyCSE] Don't eagerly optimize MemoryUses.

I'm seeing no meaningful difference in performance from this as is. Let's see if there are other reports and we can increase the limit if this is the case.

Mar 22 2022, 3:06 PM · Restricted Project, Restricted Project

Mar 17 2022

asbirlea accepted D121953: [NewPM] Don't skip SCCs not in current RefSCC.

As discussed offline, the potential issue with this change is whether we encounter a pathological case in the "opposite direction".

Mar 17 2022, 4:39 PM · Restricted Project, Restricted Project
asbirlea committed rG6c4931e7d085: [docs] Fix codeblock. (authored by asbirlea).
[docs] Fix codeblock.
Mar 17 2022, 3:55 PM · Restricted Project
asbirlea committed rG187a5f230f4b: [docs] Add details to MemorySSA docs. (authored by asbirlea).
[docs] Add details to MemorySSA docs.
Mar 17 2022, 3:26 PM · Restricted Project
asbirlea closed D121740: [docs] Add details to MemorySSA docs..
Mar 17 2022, 3:25 PM · Restricted Project, Restricted Project
asbirlea updated the diff for D121740: [docs] Add details to MemorySSA docs..

Update based on comments.

Mar 17 2022, 3:23 PM · Restricted Project, Restricted Project
asbirlea added a comment to D121740: [docs] Add details to MemorySSA docs..

Thanks for the suggestions, updated a couple more places.

Mar 17 2022, 3:22 PM · Restricted Project, Restricted Project
asbirlea accepted D121381: [MemorySSA] Support lazy use optimization.

LGTM with a couple nits.

Mar 17 2022, 3:02 PM · Restricted Project, Restricted Project

Mar 15 2022

asbirlea added a comment to D121381: [MemorySSA] Support lazy use optimization.

Sent out https://reviews.llvm.org/D121740.

Mar 15 2022, 2:24 PM · Restricted Project, Restricted Project
asbirlea requested review of D121740: [docs] Add details to MemorySSA docs..
Mar 15 2022, 2:22 PM · Restricted Project, Restricted Project
asbirlea added a comment to D121381: [MemorySSA] Support lazy use optimization.

LGTM for this patch on the impact side, testing looks good.

Mar 15 2022, 1:08 PM · Restricted Project, Restricted Project
asbirlea accepted D121695: [BasicAA] Account for wrapping when using abs(VarIndex) >= abs(Scale)..

LGTM.

Mar 15 2022, 11:34 AM · Restricted Project, Restricted Project

Mar 14 2022

asbirlea added a comment to D117926: [SLP] Optionally preserve MemorySSA.

Reading the details in your post and looking at the SLPVectorizer code I think there's the potential for the pass to benefit from using MemorySSA. However I do not understand enough of the SLPVectorizer mechanics to guess if this will work out or not.
In this context, I think it makes sense to have incremental changes while keeping building and updating MSSA off by default; as you mentioned in the post, all changes can (and should) be reverted if a later decision is to pursue something else.

Mar 14 2022, 9:44 AM · Restricted Project, Restricted Project

Mar 10 2022

asbirlea accepted D121167: [NewPM] Actually recompute GlobalsAA before module optimization pipeline.

Given the situation, I agree this is a reasonable way to handle it.

Mar 10 2022, 2:01 PM · Restricted Project, Restricted Project, Restricted Project
asbirlea added a comment to D121381: [MemorySSA] Support lazy use optimization.

Thank you for sending this out!

Mar 10 2022, 1:15 PM · Restricted Project, Restricted Project

Mar 3 2022

Herald added a project to D115160: [GVN] MemorySSA for GVN: use the incoming memory state in the value numbers: Restricted Project.
Mar 3 2022, 2:57 PM · Restricted Project, Restricted Project

Mar 1 2022

asbirlea added a comment to rG87753cebf5f8: [X86] combineX86ShufflesRecursively - don't both widening inputs before calling….

Affected code is close to or identical to the modules in: https://github.com/llvm/llvm-test-suite/tree/main/Bitcode/Regression/fft

OK - any in particular? And again, what cpu/avx-level is being targetted?

Mar 1 2022, 1:05 AM

Feb 25 2022

asbirlea added a comment to rG87753cebf5f8: [X86] combineX86ShufflesRecursively - don't both widening inputs before calling….

Affected code is close to or identical to the modules in: https://github.com/llvm/llvm-test-suite/tree/main/Bitcode/Regression/fft

Feb 25 2022, 4:27 PM
asbirlea added a comment to D118572: [NewGVN] Improve phi-of-ops fix to allow loads that loop invariant-ish.

Thank you for looking at this! Please see inline.

Feb 25 2022, 3:12 PM · Restricted Project

Feb 24 2022

asbirlea added a comment to D116825: [GVN] MemorySSA for GVN: use MemorySSA for redundant loads elimination.

Do we have any compile time measurements for this change with O3?

@chill mentioned he did some early testing on that, so he might have some numbers.
From my side, it's too early. I'm seeing a correctness issue in the early testing that needs resolving. I'm looking into getting @chill something actionable there.

I have couple of high level comments/clarifications:

  • Are all mini-transforms within GVN moving to MSSA in this case?

Is they needed MemDep before, yes.

  • Is the move to MemorySSA for performance of cases not caught through MDA?

It should address at least one or both. Primary goal is remove last user of MDA and remove the analysis from trunk. The implementation for the transition should be such that it either catches more cases or provides improved performance (or both). From previous experience with switching DSE, this is possible if implemented right.

@asbirlea, a question to clarify my understanding around enabling MSSA for a pass:

  • We optimistically build memorySSA for passes that unconditionally require it. This building and preservation can be costly from compile time perspective. Also, we may have possible invalidation by later function passes which do not preserve GVN, thereby having different implications for pipelines depending on where GVN is present. For example, if we end up with something like:
LPM {
 ...
 LICM <-- requires MSSA.

}
instcombine <-- invalidates MSSA
GVNPass() <-- build MSSA again
simplifyCFG <-- invalidates MSSA

is arguably much worse in compile time than:

LPM {
 ...
 LICM <-- requires MSSA.

}
GVNPass() <-- use preserved MSSA
instcombine <-- invalidates MSSA
simplifyCFG
Feb 24 2022, 11:55 AM · Restricted Project, Restricted Project

Feb 22 2022

asbirlea committed rG7fea963a4535: [Docs] Add self to credits (authored by asbirlea).
[Docs] Add self to credits
Feb 22 2022, 4:46 PM
asbirlea committed rGed69e3266ca5: [Docs]Add office hours. (authored by asbirlea).
[Docs]Add office hours.
Feb 22 2022, 4:41 PM

Feb 18 2022

asbirlea accepted D120036: Create office hours documentation..

Thank you for starting this!
From my side, LGTM.

Feb 18 2022, 4:17 PM · Restricted Project

Feb 17 2022

asbirlea accepted D119898: [MemorySSA] Clear VisitedBlocks per query.

lgtm

Feb 17 2022, 7:32 PM · Restricted Project, Restricted Project

Feb 16 2022

asbirlea committed rG21aaa1fb22db: [bazel] Add libc dependency. (authored by asbirlea).
[bazel] Add libc dependency.
Feb 16 2022, 5:17 PM

Feb 15 2022

asbirlea added a comment to D119898: [MemorySSA] Clear VisitedBlocks per query.

I think the clearing should be added at the beginning of insertDef and insertUse. A similar clearing is done on visited phis at the same two places. I didn't get a chance to test it yet though.

Feb 15 2022, 6:23 PM · Restricted Project, Restricted Project

Feb 10 2022

asbirlea accepted D119486: [docs] Replace `opt -analyze` with better alternatives..

lgtm.

Feb 10 2022, 3:37 PM · Restricted Project

Feb 9 2022

asbirlea committed rG49ab76009051: [DagCombine] Increase depth by number of operands to avoid a pathological… (authored by asbirlea).
[DagCombine] Increase depth by number of operands to avoid a pathological…
Feb 9 2022, 1:35 PM
asbirlea closed D118877: [DagCombine] Increase depth by number of operands to avoid a pathological compile time..
Feb 9 2022, 1:35 PM · Restricted Project

Feb 8 2022

asbirlea added a comment to D118255: [GVN] MemorySSA for GVN: add a switch to enable MemorySSA for GVN.

Just a couple of quick comments:

  • Why take out the option to DisableGVNLoadPRE?
  • Can the MemDep and MSSA be mutually exclusive options? (i.e. enabling one disables the other)
Feb 8 2022, 11:06 AM · Restricted Project, Restricted Project

Feb 4 2022

asbirlea added a comment to D118877: [DagCombine] Increase depth by number of operands to avoid a pathological compile time..

AFAICT, this patch doesn't make any difference for https://github.com/llvm/llvm-project/issues/52858.

Feb 4 2022, 11:42 AM · Restricted Project

Feb 2 2022

asbirlea added a comment to D118877: [DagCombine] Increase depth by number of operands to avoid a pathological compile time..

Reproducer with llc -O2: https://reviews.llvm.org/P8277

Feb 2 2022, 11:48 PM · Restricted Project
asbirlea requested review of D118877: [DagCombine] Increase depth by number of operands to avoid a pathological compile time..
Feb 2 2022, 11:46 PM · Restricted Project

Jan 25 2022

asbirlea accepted D117999: [NewGVN] FIx phi-of-ops in the presence of memory read operations.

Thank you for sending this fix!
It feels like a pretty big hammer, but some of the tests being modified here are exact examples of the miscompile I was seeing. I'm good with it at this point.

Jan 25 2022, 5:13 PM · Restricted Project
asbirlea added a comment to D117926: [SLP] Optionally preserve MemorySSA.

Any chance you could profile where most time is spent?

Jan 25 2022, 5:03 PM · Restricted Project, Restricted Project
asbirlea added a comment to D109958: [LoopFlatten] Enable it by default.

Quick update: I'm not seeing any major run time regressions, but there are a few unrelated issues that may make the results I'm seeing incomplete.
I'm seeing some compile-time regressions, I'd be curious to see what the updated compiler-tracker results look like.

Jan 25 2022, 4:33 PM · Restricted Project
asbirlea accepted D117907: [NewGVN] do phi(undef, x) -> x only if x is not poison.

Thank you!

Jan 25 2022, 4:30 PM · Restricted Project

Jan 20 2022

asbirlea added a comment to D115160: [GVN] MemorySSA for GVN: use the incoming memory state in the value numbers.

A very broad comment for a smooth transition

  • the patches should be split into changes to GVN independent of the switch, e.g. a) factoring out components/methods where the differences between MSSA and MD occur and b) things that need to be added for the transition
  • we should keep the MD implementation and have a flag for switching between the current one and the MSSA one being added
  • once the flag flip sticks, then we'd remove the MD implementation
Jan 20 2022, 12:34 PM · Restricted Project, Restricted Project

Jan 19 2022

asbirlea added a comment to D109958: [LoopFlatten] Enable it by default.

Thank you for resolving all the issues raised! Could you rebase this patch on ToT? (to include the MSSA update and the move to LPM1?)

Jan 19 2022, 11:19 AM · Restricted Project

Jan 18 2022

asbirlea accepted D116660: [LoopFlatten] Update MemorySSA state.

Changes LGTM.
Nit: clang-format.
A simpler way to check all verification is now to pass VerificationLevel::Full to verifyMemorySSA(); I'm mentioning this for your local testing, in case your build doesn't have EXPENSIVE_CHECKS on already, not to include in this patch.

Jan 18 2022, 2:20 PM · Restricted Project

Dec 13 2021

asbirlea committed rG46fb81095507: [NewGVN] Use PredicateInfo info when previously used for the same ssa.copy… (authored by asbirlea).
[NewGVN] Use PredicateInfo info when previously used for the same ssa.copy…
Dec 13 2021, 4:54 PM
asbirlea committed rGac994f831cb7: [MemorySSA] Document details regarding MemorySSA's precision. (authored by asbirlea).
[MemorySSA] Document details regarding MemorySSA's precision.
Dec 13 2021, 4:54 PM
asbirlea closed D110907: [NewGVN] Use PredicateInfo info when previously used for the same ssa.coy intrinsic.
Dec 13 2021, 4:54 PM · Restricted Project

Nov 16 2021

asbirlea accepted D113947: [NewPM] Add option to prevent rerunning function pipeline on functions in CGSCC adaptor.

Thank you for this work!

Nov 16 2021, 2:57 PM · Restricted Project

Nov 11 2021

asbirlea accepted D113712: [DSE] Allow DSE to optimize MemorySSA by default..
Nov 11 2021, 5:19 PM · Restricted Project
asbirlea added inline comments to D112315: [DSE] Use optimized access if available for redundant store elimination..
Nov 11 2021, 5:19 PM · Restricted Project
asbirlea added a comment to D112313: [DSE] Optimize defining access of defs while walking upwards..

Allowing passes to do this is a slippery slope...
We already have the issue that it is sometimes hard to reproduce an issue with a single pass, due to the cached state in MemorySSA. This cached state is already dependent on what other passes do, a mix between queries and transforms, which may leave MemorySSA either "over-optimized" (has stored info beyond what it could deduce if built from scratch) or "under-optimized" (could deduce more). Having DSE optimize more is not that far fetched considering this.
I'm inclined to let DSE do this, only because 1) the traversals and inferences are beyond what MSSA alone does now and 2) they're "free" (i.e. DSE does them anyway).
However, I would not be ok with any pass being able to set optimized accesses.

That policy sounds good to me, thanks for sharing! Should we add something like that to the MemorySSA docs?

Nov 11 2021, 5:18 PM · Restricted Project
asbirlea added inline comments to D112315: [DSE] Use optimized access if available for redundant store elimination..
Nov 11 2021, 10:48 AM · Restricted Project
asbirlea added a comment to D112313: [DSE] Optimize defining access of defs while walking upwards..

Allowing passes to do this is a slippery slope...
We already have the issue that it is sometimes hard to reproduce an issue with a single pass, due to the cached state in MemorySSA. This cached state is already dependent on what other passes do, a mix between queries and transforms, which may leave MemorySSA either "over-optimized" (has stored info beyond what it could deduce if built from scratch) or "under-optimized" (could deduce more). Having DSE optimize more is not that far fetched considering this.
I'm inclined to let DSE do this, only because 1) the traversals and inferences are beyond what MSSA alone does now and 2) they're "free" (i.e. DSE does them anyway).
However, I would not be ok with any pass being able to set optimized accesses.

Nov 11 2021, 10:41 AM · Restricted Project
asbirlea added a comment to D104268: [ptr_provenance] Introduce optional ptr_provenance operand to load/store.

Thank you for all the clarifications. I'm convinced this is the right way to move forward at this point.

Nov 11 2021, 9:58 AM · Restricted Project, Restricted Project

Nov 9 2021

asbirlea accepted D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses.

LGTM, but please wait to see if @nikic has additional comments.

Nov 9 2021, 2:42 PM · Restricted Project, Restricted Project

Nov 8 2021

asbirlea accepted D112935: [NFC] Rename GVN -> GVNPass and SROA -> SROAPass.
Nov 8 2021, 5:01 PM · Restricted Project, Restricted Project
asbirlea added a comment to D110822: [GVN] Simple GVN hoist - loads and stores.

The request changes is for the use of getID() in MemorySSA(). Other optimizations should never rely on the value of the IDs assigned by MemorySSA.

The getID() was already a public method in derived classes and the ID is used according to its description "Guaranteed unique among MemoryAccesses".

How about value numbering the BasicBlock that corresponds to a MemoryPhi ? It's a little bit of overhead and not strictly necessary if we have an ID readily available, but, fair enough, I don't mind doing it this way.

Nov 8 2021, 4:36 PM · Restricted Project
asbirlea added a comment to D113304: [NewPM] Only invalidate modified functions' analyses in CGSCC passes + turn on eagerly invalidate analyses.

Thank you for the additional clarifications in the comments and descriptions!

Nov 8 2021, 2:54 PM · Restricted Project, Restricted Project
asbirlea requested changes to D110822: [GVN] Simple GVN hoist - loads and stores.

The request changes is for the use of getID() in MemorySSA(). Other optimizations should never rely on the value of the IDs assigned by MemorySSA.

Nov 8 2021, 2:07 PM · Restricted Project
asbirlea accepted D113303: [NFC][FuncAttrs] Keep track of modified functions.
Nov 8 2021, 11:26 AM · Restricted Project

Nov 4 2021

asbirlea accepted D113210: [NewPM] Use the default AA pipeline by default.
Nov 4 2021, 12:22 PM · Restricted Project, Restricted Project
asbirlea added a comment to D113210: [NewPM] Use the default AA pipeline by default.

Can you also update lib/LTO/LTOBackend.cpp to something like:

// Parse a custom AA pipeline if asked to, otherwise the default will be used.
if (!Conf.AAPipeline.empty()) {
  AAManager AA;
  if (auto Err = PB.parseAAPipeline(AA, Conf.AAPipeline)) {
    report_fatal_error(Twine("unable to parse AA pipeline description '") +
                       Conf.AAPipeline + "': " + toString(std::move(Err)));
  }
  // Register the AA manager first so that our version is the one used.
  FAM.registerPass([&] { return std::move(AA); });
}
Nov 4 2021, 12:08 PM · Restricted Project, Restricted Project
asbirlea added a comment to D113196: [NewPM] Make eager analysis invalidation per-adaptor.

This change's numbers look better overall. The compile-time regressions are smaller across the board and the memory gains are still there, in some cases reduced and in some cases improved. I don't think the level of noise is that high.
Overall, this change is the right approach both conceptually (only invalidate in these adaptors) and given the practical results.

Nov 4 2021, 10:57 AM · Restricted Project

Nov 3 2021

asbirlea accepted D111161: [UnknownProvenance] Add bitcode support..
Nov 3 2021, 4:52 PM · Restricted Project, Restricted Project
asbirlea accepted D111163: [UnknownProvenance] add support in ValueMapper.
Nov 3 2021, 4:51 PM · Restricted Project, Restricted Project
asbirlea added a comment to D104268: [ptr_provenance] Introduce optional ptr_provenance operand to load/store.

Adding a few items/questions discussed in the open meeting:

Nov 3 2021, 4:51 PM · Restricted Project, Restricted Project
asbirlea accepted D113032: [ArgPromo] Preserve FunctionAnalysisManagerCGSCCProxy.
Nov 3 2021, 2:54 PM · Restricted Project

Nov 1 2021

asbirlea added a comment to D109762: [NewPM][SimpleLoopUnswitch] Add DivergenceInfo.

I talked to some people and we've decided that the best thing to do would be to refactor out the nontrivial unswitching part into a function pass. Nontrivial unswitching is fairly special in the kinds of transforms it does.

Will that always work as expected? The real dependency is that the divergence analysis is not incrementally updated. So even if this is a function pass, we may want to rerun it on the whole function every time it manages to unswitch a loop.

With this patch, we have to rerun DivergenceAnalysis every time we run the pass on every loop. If we change nontrivial unswitching into a function pass, we can upgrade that to only having to be rerun every time we actually unswitch something.
To be more specific, with the function pass, we'd have to invalidate most analyses anyway after every successful unswitch.

for every loop
  DA = FAM.getAnalysis<DivergenceAnalysis>(); // this will not rerun the analysis if we haven't invalidated below because we didn't successfully unswitch
  if successful unswitch
    auto PA = PreservedAnalyases::none();
    PA.preserve<SCEV>();
    // and all the other existing analyses we preserved
    FAM.invalidate(PA);
Nov 1 2021, 4:11 PM · Restricted Project

Oct 25 2021

asbirlea committed rG3850cba7cc97: [bazel build] (manually) port da47ec3ca076 (authored by asbirlea).
[bazel build] (manually) port da47ec3ca076
Oct 25 2021, 3:12 PM
asbirlea added a comment to rGda47ec3ca076: Basic: Stop using expectedToOptional() in FileManagerTest, NFC.

This broke upstream bazel: https://buildkite.com/llvm-project/upstream-bazel-rbe/builds/10376#98340937-c4c7-4bd7-997e-dd56691da9d8
Was the header file missed?

Oct 25 2021, 2:34 PM