This is an archive of the discontinued LLVM Phabricator instance.

[mlir][bufferize][NFC] Merge AnalysisState and BufferizationAliasInfo
ClosedPublic

Authored by springerm on Feb 4 2023, 2:36 AM.

Details

Summary

There is no longer a need to keep the two separate. This is in preparation of reusing the same AnalysisState for tensor.empty elimination and One-Shot Bufferize (to address performance bottlenecks).

Diff Detail

Event Timeline

springerm created this revision.Feb 4 2023, 2:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 4 2023, 2:36 AM
springerm requested review of this revision.Feb 4 2023, 2:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 4 2023, 2:36 AM
scotttodd accepted this revision.Feb 7 2023, 12:46 PM
scotttodd added inline comments.
mlir/include/mlir/Dialect/Bufferization/Transforms/OneShotAnalysis.h
207–242

nit: "uses uses" typo

mlir/lib/Dialect/Bufferization/Transforms/OneShotAnalysis.cpp
180–182

How does this interact with reusing the state? Maybe this comment should be updated in https://reviews.llvm.org/D143404 ?

This is in preparation of reusing the same AnalysisState for tensor.empty elimination and One-Shot Bufferize (to address performance bottlenecks).

(I'm also wondering how much of a sharp edge this is and if there are other ways to make the analysis more robust to mutations / usage patterns)

This revision is now accepted and ready to land.Feb 7 2023, 12:46 PM
This revision was landed with ongoing or failed builds.Feb 8 2023, 12:16 AM
This revision was automatically updated to reflect the committed changes.
springerm marked an inline comment as done.
springerm added inline comments.Feb 8 2023, 12:20 AM
mlir/lib/Dialect/Bufferization/Transforms/OneShotAnalysis.cpp
180–182

I added a comment to the class, clarifying this: Modifying the IR invalidates the analysis in general, but there are a few things that are safe. (E.g., adding new ops or changing uses of tensor.empty as long as we re-analyze these ops.)