Page MenuHomePhabricator

asbirlea (Alina Sbirlea)
Google

Projects

User does not belong to any projects.

User Details

User Since
Feb 10 2016, 9:51 AM (234 w, 3 d)

Recent Activity

Thu, Aug 6

asbirlea requested changes to D85109: [NewPM] Clear abandoned analyses set when preserveSet is called.
Thu, Aug 6, 4:48 PM · Restricted Project
asbirlea added a comment to D84609: [MemDepAnalysis] Cut-off threshold reshuffling .

I wasn't really asking about NewGVN story, i know it's stagnant somewhat.
I was only asking, would it be better to instead look into porting
GVN::processNonLocalLoad() to be MemorySSA-driven.

If we're building two analyses (MSSA & MemDepAnalysis) instead of one, I expect we'll see a spike in compile-time. Additionally, MemDepAnalysis is a piece of technical debt that would be nice to replace altogether.
IMO it makes sense to make the transition for the whole pass, or for the pipeline of 3 passes in one go.

I recently ran some numbers for GVN compile-time impact: http://llvm-compile-time-tracker.com/index.php?branch=nikic/perf/new-gvn-3 From the bottom to the top, the first commit disables LoadPRE, the second disables the entirety of processNonLocalLoad and the last one enables NewGVN (with the corresponding MemorySSA run).

I think the key takeaways here is that the non-local load analysis in GVN is really, really expensive. Per-block MemDep analysis is fairly cheap, but the non-local one is not, and I think GVN is the only MemDep-based pass that uses it.

So, if it is feasible to replace the non-local load analysis (and load PRE) in GVN with something MemorySSA based, I think it's plausible that it will be a compile-time improvement despite the need to construct MemorySSA. And it would become free for a following MemCpyOpt/DSE :)

Of course, this is on the assumption that MemorySSA can actually perform a similar degree of optimization with better compile-time. As these optimizations require walking past MemoryPhis, it's not going to be free, but I would still expect it to be cheaper and have better cutoff control.

Thu, Aug 6, 1:42 PM · Restricted Project
asbirlea added a comment to D85472: [DomTree] Extend update API to allow a post CFG view..

I'm inclined to change the API to pass a second vector of updates instead of the GraphDiff directly (for the PostView), since during updates the GD gets modified and it makes sense to keep it as an internal component. Thoughts?

Thu, Aug 6, 1:33 PM · Restricted Project
asbirlea requested review of D85472: [DomTree] Extend update API to allow a post CFG view..
Thu, Aug 6, 1:30 PM · Restricted Project
asbirlea accepted D67687: [NewPM][CodeGen] Introduce machine pass and machine pass manager.
Thu, Aug 6, 1:09 PM · Restricted Project
asbirlea accepted D85394: [NewPM][GuardWidening] Fix loop guard widening tests under NPM.
Thu, Aug 6, 1:06 PM · Restricted Project
asbirlea added inline comments to D85394: [NewPM][GuardWidening] Fix loop guard widening tests under NPM.
Thu, Aug 6, 1:05 PM · Restricted Project
asbirlea added inline comments to D85394: [NewPM][GuardWidening] Fix loop guard widening tests under NPM.
Thu, Aug 6, 10:50 AM · Restricted Project

Wed, Aug 5

asbirlea committed rGbeb9993d960b: [MSSA] Update test with more detailed and resilient checks. [NFC] (authored by asbirlea).
[MSSA] Update test with more detailed and resilient checks. [NFC]
Wed, Aug 5, 4:47 PM
asbirlea accepted D85333: [MSSA][NewPM] Handle tests with -print-memoryssa.
Wed, Aug 5, 11:43 AM · Restricted Project

Mon, Aug 3

asbirlea added inline comments to D67687: [NewPM][CodeGen] Introduce machine pass and machine pass manager.
Mon, Aug 3, 5:01 PM · Restricted Project
asbirlea committed rG1ce82015f6d0: [MemorySSA] Restrict optimizations after a PhiTranslation. (authored by asbirlea).
[MemorySSA] Restrict optimizations after a PhiTranslation.
Mon, Aug 3, 2:50 PM
asbirlea closed D84905: [MemorySSA] Restrict optimizations after a PhiTranslation..
Mon, Aug 3, 2:49 PM · Restricted Project
asbirlea updated the diff for D84905: [MemorySSA] Restrict optimizations after a PhiTranslation..

Address comments.

Mon, Aug 3, 2:48 PM · Restricted Project
asbirlea accepted D85001: [SimpleLoopUnswitch][NFC] Add option to always drop make.implicit metadata in non-trivial unswitching and save compile time.
Mon, Aug 3, 11:09 AM · Restricted Project
asbirlea added inline comments to D85109: [NewPM] Clear abandoned analyses set when preserveSet is called.
Mon, Aug 3, 11:07 AM · Restricted Project

Sat, Aug 1

asbirlea added a comment to D84959: [NewPM][LVI] Abandon LVI after CVP.

This is ok to check in to resolve the regression.
I'll go back and check if that invalidate is overly conservative, but don't block in it.

Sat, Aug 1, 11:37 AM · Restricted Project, Restricted Project

Thu, Jul 30

asbirlea updated the diff for D84905: [MemorySSA] Restrict optimizations after a PhiTranslation..

Add comments.

Thu, Jul 30, 7:42 PM · Restricted Project
asbirlea added inline comments to D84905: [MemorySSA] Restrict optimizations after a PhiTranslation..
Thu, Jul 30, 7:32 PM · Restricted Project
asbirlea added a comment to D84959: [NewPM][LVI] Abandon LVI after CVP.

The additional debug print with "Invalidating all non-preserved analyses for" seems too noisy and not informative.
+1 on rebasing this without the no-lvi-abandon flag on D84981.

Thu, Jul 30, 5:53 PM · Restricted Project, Restricted Project
asbirlea accepted D84981: [NewPM] Don't print 'Invalidating all non-preserved analyses'.

Thanks for removing the noise.

Thu, Jul 30, 5:51 PM · Restricted Project
asbirlea accepted D84977: [NewPM] Only verify loop for nonskipped user loop pass.

lg.

Thu, Jul 30, 5:28 PM · Restricted Project
asbirlea added a reviewer for D81554: [ADT] Allow IsSizeLessThanThresholdT for incomplete types. NFC: dblaikie.
Thu, Jul 30, 8:18 AM · Restricted Project
asbirlea accepted D84925: [SimpleLoopUnswitch] Preserve make.implicit in non-trivial unswitch if legal.

lgtm

Thu, Jul 30, 8:14 AM · Restricted Project
asbirlea accepted D84916: [SimpleLoopUnswitch] Drop make.implicit metadata in case of non-trivial unswitching.

lgtm.

Thu, Jul 30, 8:07 AM · Restricted Project

Wed, Jul 29

asbirlea added a comment to D84713: [DominatorTree] Simplify ChildrenGetter..

I'm looking into it. If needed this can be reverted as it's not blocking the work for DomTree updates with a CFGView.

Wed, Jul 29, 10:25 PM · Restricted Project, Restricted Project
asbirlea requested review of D84905: [MemorySSA] Restrict optimizations after a PhiTranslation..
Wed, Jul 29, 7:05 PM · Restricted Project
asbirlea accepted D84872: [NewPM][opt] Revert to legacy PM when any codegen passes are specified.

lgtm

Wed, Jul 29, 1:50 PM · Restricted Project
asbirlea added inline comments to D84872: [NewPM][opt] Revert to legacy PM when any codegen passes are specified.
Wed, Jul 29, 12:38 PM · Restricted Project
asbirlea added a comment to D84873: AMDGPU: In determining load clobbering in AnnotateUniform, don't scan if there are too many blocks..

Does MemorySSA have the same problem? Could we just switch this to use MemorySSA?

Wed, Jul 29, 12:30 PM · Restricted Project

Tue, Jul 28

asbirlea committed rGe22de4e46d1d: [DominatorTree] Simplify ChildrenGetter. (authored by asbirlea).
[DominatorTree] Simplify ChildrenGetter.
Tue, Jul 28, 3:45 PM
asbirlea closed D84713: [DominatorTree] Simplify ChildrenGetter..
Tue, Jul 28, 3:45 PM · Restricted Project, Restricted Project
asbirlea updated the diff for D84713: [DominatorTree] Simplify ChildrenGetter..

Renamed ChildrenGet to getChildren. The same name only exists in GraphDiff, it's ok to keep a consistent naming.

Tue, Jul 28, 3:33 PM · Restricted Project, Restricted Project
asbirlea added a comment to D84609: [MemDepAnalysis] Cut-off threshold reshuffling .

I wasn't really asking about NewGVN story, i know it's stagnant somewhat.
I was only asking, would it be better to instead look into porting
GVN::processNonLocalLoad() to be MemorySSA-driven.

Tue, Jul 28, 3:08 PM · Restricted Project
asbirlea added a comment to D84609: [MemDepAnalysis] Cut-off threshold reshuffling .

Some general comments:

  • The MemDepAnalysis has been known to be problematic for compile-time, so reducing the 100k implicit threshold seems reasonable.
  • It's natural the compiler tracker will be happy, but can we consider runtime implications due to potential missed optimizations?
  • Could we evaluate which optimizations are missed, in which passes using MemDepAnalysis along with their run-time impact? (in particular for the benchmarks where we see the large compile-time benefits)

AFAIK, the main passes using MemDepAnalysis are DSE, MemCpyOpt and GVN and there is active work in porting these to MemorySSA. The same analysis of compile-time vs run-time benefits is needed for that switch, so having data from reducing thresholds here will be very valuable for the short term (current patch) on deciding how much to reduce it to, and for the long-term switch to MemorySSA.

Background: the motivation for this patch is the fact that D84108 significantly
regresses compile-time of lencod, especially context_ini.c (+79.97%).
It has been traced to GVN spending most of the time in GVN::processNonLocalLoad(),
in MemoryDependenceResults::getNonLocalPointerDependency().

Since a compile-, and run- time performance assessment will be needed both here,
and in MemorySSA switch, would it be more productive to directly proceed to the latter?

Tue, Jul 28, 2:19 PM · Restricted Project
asbirlea accepted D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations.

I'm ok if this is added in either this or a follow-up patch.
I think it's useful to have it in the codebase now, with one test to showcase it (e.g. do another run line in new-pass-manager.ll, with check-prefixes).

Tue, Jul 28, 11:14 AM · Restricted Project, Restricted Project
asbirlea added inline comments to D84772: [NewPM][PassInstrument] Add a new kind of before-pass callback that only get called if the pass is not skipped.
Tue, Jul 28, 11:07 AM · Restricted Project
asbirlea added a comment to D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations.

Does it make sense to have the option to enable seeing where the adaptors are run?

Tue, Jul 28, 11:04 AM · Restricted Project, Restricted Project
asbirlea accepted D84773: [NewPM][PassInstrument] Make PrintIR and TimePasses to use before-pass-run callback.

lgtm

Tue, Jul 28, 10:49 AM · Restricted Project
asbirlea added inline comments to D84772: [NewPM][PassInstrument] Add a new kind of before-pass callback that only get called if the pass is not skipped.
Tue, Jul 28, 10:48 AM · Restricted Project
asbirlea added a comment to D84609: [MemDepAnalysis] Cut-off threshold reshuffling .

Some general comments:

  • The MemDepAnalysis has been known to be problematic for compile-time, so reducing the 100k implicit threshold seems reasonable.
  • It's natural the compiler tracker will be happy, but can we consider runtime implications due to potential missed optimizations?
  • Could we evaluate which optimizations are missed, in which passes using MemDepAnalysis along with their run-time impact? (in particular for the benchmarks where we see the large compile-time benefits)

AFAIK, the main passes using MemDepAnalysis are DSE, MemCpyOpt and GVN and there is active work in porting these to MemorySSA. The same analysis of compile-time vs run-time benefits is needed for that switch, so having data from reducing thresholds here will be very valuable for the short term (current patch) on deciding how much to reduce it to, and for the long-term switch to MemorySSA.

Tue, Jul 28, 10:40 AM · Restricted Project
asbirlea updated asbirlea.
Tue, Jul 28, 10:30 AM

Mon, Jul 27

asbirlea committed rGfbca317694bf: [CFGDiff] Refactor Succ/Pred maps. (authored by asbirlea).
[CFGDiff] Refactor Succ/Pred maps.
Mon, Jul 27, 6:04 PM
asbirlea committed rG18c725e735b1: [DomTree] Remove dead code.[NFC] (authored by asbirlea).
[DomTree] Remove dead code.[NFC]
Mon, Jul 27, 6:03 PM
asbirlea closed D84567: [CFGDiff] Refactor Succ/Pred maps..
Mon, Jul 27, 6:03 PM · Restricted Project
Herald added projects to D84713: [DominatorTree] Simplify ChildrenGetter.: Restricted Project, Restricted Project.
Mon, Jul 27, 4:43 PM · Restricted Project, Restricted Project
asbirlea updated the diff for D84567: [CFGDiff] Refactor Succ/Pred maps..

Address comments.

Mon, Jul 27, 4:26 PM · Restricted Project
asbirlea committed rGf1d4db4f0cdc: [GraphDiff] Use class method getChildren instead of GraphTraits. (authored by asbirlea).
[GraphDiff] Use class method getChildren instead of GraphTraits.
Mon, Jul 27, 4:13 PM
asbirlea closed D84562: [GraphDiff] Use class method getChildren instead of GraphTraits..
Mon, Jul 27, 4:13 PM · Restricted Project

Fri, Jul 24

Herald added a project to D84567: [CFGDiff] Refactor Succ/Pred maps.: Restricted Project.
Fri, Jul 24, 4:33 PM · Restricted Project
Herald added a project to D84562: [GraphDiff] Use class method getChildren instead of GraphTraits.: Restricted Project.
Fri, Jul 24, 3:30 PM · Restricted Project
asbirlea committed rG8bf4c1f4fb25: Reapply "[DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff." (authored by asbirlea).
Reapply "[DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff."
Fri, Jul 24, 2:42 PM
asbirlea closed D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff..
Fri, Jul 24, 2:42 PM · Restricted Project, Restricted Project
asbirlea added a comment to D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff..

Great, thank you for confirming :-)

Fri, Jul 24, 2:41 PM · Restricted Project, Restricted Project
asbirlea added a comment to D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff..

Yes. The difference in perf in instructions and cycles count is due to reversing the update order.
I'm still not seeing meaningful changes in wall-time even with the update order reversed, but reliable wall-time testing is hard.

Fri, Jul 24, 2:14 PM · Restricted Project, Restricted Project
asbirlea accepted D84542: Rename scoped-noalias -> scoped-noalias-aa.

Thank you!

Fri, Jul 24, 12:11 PM · Restricted Project

Thu, Jul 23

asbirlea updated the diff for D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff..

The increase in number of instructions and cycles was caused by reversing the order in which the updates are applied by GraphDiff.
I'll look into re-adding some of the original cleanups in a follow-up patch.

Thu, Jul 23, 10:12 PM · Restricted Project, Restricted Project
asbirlea added a comment to D67687: [NewPM][CodeGen] Introduce machine pass and machine pass manager.

IIRC the MIR pipeline is essentially flat (vs the IR one), so at this point it's probably fine to not have the adaptor complexity. However, if this changes to become more than a one time use, then perhaps the Module-Function hierarchy makes sense.
Let me ask this differently, if the CodegenNPM introduced the ModuleToFunctionPassAdaptor now, and it was only used during codegen set-up (assuming negligible overhead for this complexity), is it possible it will be useful long-term in case there are changes in the current restrictions?

TBH, I don't think the adaptor class would be useful for cases other than codegen setup, because a partial MIR pipeline seems not very useful to be run as part of an IR pipeline. The only other place, I could think of, the adaptor could be useful is to concatenate the pre-codengen IR pipeline with the MIR pipeline. But similar to the possibility of combining opt-pipeline with codegen pipeline, I'm not sure it brings actual benefits other than aesthetics.

Thu, Jul 23, 4:40 PM · Restricted Project
asbirlea added a comment to D83219: [ARM] Add MVE_TwoOpPattern. NFC.

Thank you very much for the quick patch! Confirming Halide is fixed too.

Thu, Jul 23, 12:53 PM · Restricted Project
asbirlea added a comment to D67687: [NewPM][CodeGen] Introduce machine pass and machine pass manager.

IIRC the MIR pipeline is essentially flat (vs the IR one), so at this point it's probably fine to not have the adaptor complexity. However, if this changes to become more than a one time use, then perhaps the Module-Function hierarchy makes sense.
Let me ask this differently, if the CodegenNPM introduced the ModuleToFunctionPassAdaptor now, and it was only used during codegen set-up (assuming negligible overhead for this complexity), is it possible it will be useful long-term in case there are changes in the current restrictions?

Thu, Jul 23, 12:52 PM · Restricted Project

Wed, Jul 22

asbirlea added a comment to D83219: [ARM] Add MVE_TwoOpPattern. NFC.

We're also seeing runtime failures in Halide (SIGILL). Either reverting or having a quick forward fix would be very much appreciated.

Wed, Jul 22, 11:49 AM · Restricted Project

Tue, Jul 21

asbirlea accepted D84259: [NFC][NewPM] Add clarification on analysis manager proxies.

lgtm.

Tue, Jul 21, 12:22 PM · Restricted Project

Thu, Jul 16

asbirlea accepted D82698: [NewPM] make parsePassPipeline parse adaptor-wrapped user passes.

Thanks, this is good.

Thu, Jul 16, 7:00 PM · Restricted Project

Wed, Jul 15

asbirlea accepted D83798: [SCEV] Fix ScalarEvolution tests under NPM.

Thank you!

Wed, Jul 15, 10:22 PM · Restricted Project
asbirlea accepted D83869: [Loop Simplify] Resolve an issue where metadata is not applied to a loop latch..

Thank you.

Wed, Jul 15, 11:30 AM · Restricted Project
asbirlea accepted D83187: [LoopUnroll] Update branch weight for remainder loop.

lgtm with 2 nits to clarify/fix.

Wed, Jul 15, 10:55 AM · Restricted Project

Tue, Jul 14

asbirlea added a comment to D82344: [NewPM] Allow passes to never be skipped.

I'm inclined to favor having the isRequired() method optional with the SFINAE in this patch.
It's up to you two which patch to actually keep, but I'd say update this patch, with renaming to isRequired() (and flipping the bool value), adding the default to true in isRequired() for all adaptors and if, as you mentioned the diff can be simplified, great, but if not, this one makes sense.

Tue, Jul 14, 3:59 PM · Restricted Project
asbirlea added a comment to D70376: [LVI] Restructure caching.

FWIW, the patch to abandon in PreserveAnalysis is correct and the right approach IMO.

Tue, Jul 14, 12:37 PM · Restricted Project
asbirlea added inline comments to D51718: Update MemorySSA in LoopRotate..
Tue, Jul 14, 10:00 AM · Restricted Project

Mon, Jul 13

asbirlea accepted D83633: [NewPM][opt] Translate -foo-analysis to require<foo-analysis>.

lgtm.

Mon, Jul 13, 10:18 AM · Restricted Project

Fri, Jul 10

asbirlea added a comment to D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff..

Thank you for the testing. Could you help with with instructions on how to run the tracker myself?
My local testing showed a negligible regression for mafft and a negligible improvement on other benchmarks, so it looked like noise on average.

Fri, Jul 10, 12:43 PM · Restricted Project, Restricted Project

Jul 9 2020

asbirlea updated the diff for D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff..

Nit: re-add consts

Jul 9 2020, 6:29 PM · Restricted Project, Restricted Project
asbirlea updated the summary of D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff..
Jul 9 2020, 6:26 PM · Restricted Project, Restricted Project
asbirlea updated the diff for D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff..

Updated to include the part of the patch that's moving the Updates to a CFGDiff object. Splitting off from the clean-up work merging the two branches when BUI is null.
This patch does not exhibit the compile-time regression which caused it to be reverted previously.

Jul 9 2020, 6:23 PM · Restricted Project, Restricted Project
asbirlea accepted D83498: [NFC] Derive from PassInfoMixin for no-op/printing passes.
Jul 9 2020, 4:27 PM · Restricted Project
asbirlea added a comment to D83421: [RFC] MemorySSAUpdater: Simplify applyUpdates.

Okay, thank you for that context, that makes sense. I'm going to drop this change then obviously.

FYI, I'm considering to at least try out making dominator tree construction based on the CfgInterfaceImpl from D83088, so that GenericDomTreeConstruction is no longer all a giant ball of templates. Do you think this would help you with your changes? Obviously there's the question of how it impacts compile time, but we won't really know for sure until we try it, which is one of my motivations here.

Jul 9 2020, 9:58 AM · Restricted Project

Jul 8 2020

asbirlea accepted D83430: [AliasSetTracker] More precise AAInfo intersection check.

LGTM, thank you!

Jul 8 2020, 5:30 PM · Restricted Project
asbirlea added a comment to D83421: [RFC] MemorySSAUpdater: Simplify applyUpdates.

This is an issue I am aware of and the main motivation for the DominatorTree refactoring work. We do need an updated DT here, and currently we're not computing the correct one. This isn't causing issues at the moment because of the scenarios where the updater is being used, but using the current DT in general is not correct.

Jul 8 2020, 2:33 PM · Restricted Project

Jul 7 2020

asbirlea accepted D72410: [DSE] Eliminate stores by terminators (free,lifetime.end)..

Thanks you, LGTM.

Jul 7 2020, 4:31 PM · Restricted Project
asbirlea added inline comments to D72410: [DSE] Eliminate stores by terminators (free,lifetime.end)..
Jul 7 2020, 1:03 PM · Restricted Project

Jul 6 2020

asbirlea added a reviewer for D83093: DomTree: Define GraphTraits for GenericDomTreeBase: kuhar.
Jul 6 2020, 12:02 PM · Restricted Project
asbirlea added a comment to D83013: [LPM] Port CGProfilePass from NPM to LPM.

Adding Chandler and Alina here as well.

In general, I don't think that this is such a great idea. Being able to have this sort of thing work more reliably is one of the reasons for the new pass manager. I think I'd like to see this split out into an old versus new pass manager pass to avoid the difficulty of cleaning this up after we finish migrating llvm to the new pass manager. This also seems to add some technical debt around options and other enablement which is also less than ideal. Is this compelling to add right now versus finishing work migrating llvm completely to the new pass manager and removing the old one? From speaking with Alina I think that work should be done in a short while.

Thanks.

-eric

I don't think we're that close yet, probably at least a couple months out, there are lots of loose ends to be tied up. I'll make a post soon in llvm-dev (maybe first we can sync up again) about what I think needs to be done before the NPM switch.

Jul 6 2020, 11:08 AM · Restricted Project, Restricted Project

Jun 29 2020

asbirlea added a comment to D82698: [NewPM] make parsePassPipeline parse adaptor-wrapped user passes.

I think it's great to have a way to specify the exact nesting of managers we want.
Please extend the changes to all tests.

Jun 29 2020, 1:36 PM · Restricted Project
asbirlea added a comment to D82566: [CodeMoverUtils] Make specific analysis dependent checks optional.

Seems like this needs to be rebased on top of D82290?

Jun 29 2020, 12:28 PM · Restricted Project
asbirlea accepted D82290: [CodeMoverUtils][WIP] Isolate checks strictly related to the code motion candidate instruction.

Looks like a reasonable cleanup making safety checks reusable.
Please run check-all to check the statistics you're changing are not checked for in unit tests.

Jun 29 2020, 12:28 PM · Restricted Project
asbirlea accepted D82293: [CodeMoverUtils][WIP] Move code motion related checks from LICM to CodeMoverUtils.

This looks only dependent on D82290, not D82566.

Jun 29 2020, 12:28 PM · Restricted Project
asbirlea accepted D81835: [SimplifyCFG] Fix inconsistency in block size assessment for threading.

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

Jun 29 2020, 11:55 AM · Restricted Project
asbirlea added a comment to D82293: [CodeMoverUtils][WIP] Move code motion related checks from LICM to CodeMoverUtils.

Yes, the diff look good. Could you clarify what isSafeToMove does? I don't see it in the included file, only isSafeToMoveBefore. Is there a dependent patch not linked?

Jun 29 2020, 8:03 AM · Restricted Project

Jun 26 2020

asbirlea requested changes to D82293: [CodeMoverUtils][WIP] Move code motion related checks from LICM to CodeMoverUtils.

The issue with this change is that there is a lot of unnecessary work done now. Before, we could exit early to decide an instruction could never be hoisted/sunk. Now, we're removing all those early checks and only when actually trying to move get the "actually, you can't". I expect this to get a big compile-time penalty.
Could you do some testing and include results in the patch summary?

Jun 26 2020, 4:19 PM · Restricted Project
asbirlea accepted D82689: [NewPM][BasicAA] basicaa -> basic-aa in Transforms/DeadStoreElimination.

Thank you!

Jun 26 2020, 4:13 PM · Restricted Project
asbirlea accepted D82688: [NewPM][BasicAA] basicaa -> basic-aa in Transforms/{New,}GVN.

Thanks you!

Jun 26 2020, 4:13 PM · Restricted Project

Jun 25 2020

asbirlea accepted D82618: [MemorySSA] Update comment in PassBuilder.

It's not impossible. It's hard and likely not worth it.
Preserving MemorySSA for this pass means creating MemoryAccesses for all blocks cloned when unrolling. Creating an access and then updating so many times is likely more expensive than recomputing MemorySSA from scratch.

Jun 25 2020, 11:59 PM · Restricted Project

Jun 24 2020

asbirlea accepted D82511: [NewPM] Move debugging log printing after PassInstrumentation before-pass-callbacks.

Thank you!

Jun 24 2020, 10:45 PM · Restricted Project
asbirlea accepted D82488: [NewPM] Separate out alias analysis passes in opt.

Thank you!

Jun 24 2020, 8:05 PM · Restricted Project
asbirlea accepted D82511: [NewPM] Move debugging log printing after PassInstrumentation before-pass-callbacks.

LGTM.

Jun 24 2020, 8:05 PM · Restricted Project
asbirlea accepted D82512: [NewPM][opt] Assert PassPipeline and Passes don't both contain passes.

LGTM, thanks!

Jun 24 2020, 5:56 PM · Restricted Project
asbirlea added inline comments to D82320: [NewPM] Attempt to run opt passes specified via -foo-pass under NPM.
Jun 24 2020, 3:47 PM · Restricted Project
asbirlea added a comment to D82488: [NewPM] Separate out alias analysis passes in opt.

A general comment on the series of patches: it's great to have this means of testing the NPM through the legacy way of specifying passes, but I believe the long term goal is to only rely on specifying via passes=.... Most tests will need to be updated to get there, so having this mass testing available now is very useful!
Just note that when the transition is made to only use passes=... to opt, all additions made solely for matching the legacy -foo-pass arguments will need to be cleaned up (e.g. the API added here in PassBuilder.h)

Jun 24 2020, 3:47 PM · Restricted Project

Jun 23 2020

asbirlea accepted D72631: [DSE] Eliminate stores at the end of the function..

Thank you for the patch!

Jun 23 2020, 12:54 PM · Restricted Project
asbirlea accepted D82204: [DSE,MSSA] Treat `store 0` after calloc as noop stores..

lgtm

Jun 23 2020, 10:43 AM · Restricted Project