Page MenuHomePhabricator

[MLIR] Break cyclic dependencies with MLIRAnalysis
ClosedPublic

Authored by stephenneuendorffer on Jan 29 2020, 11:37 AM.

Details

Summary

MLIRAnalysis depended on MLIRVectorOps
MLIRVectorOps depended on MLIRAnalysis for Loop information.

Both of these can be solved by factoring out libraries related to loop
analysis into their own library. The new MLIRLoopAnalysis might be
better off with the Loop Dialect in the future.

Diff Detail

Event Timeline

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

mehdi_amini accepted this revision.Jan 29 2020, 9:56 PM

LGTM, thanks!

But I'd like to have River's review here first.

mlir/lib/Analysis/CMakeLists.txt
41

I have a local patch prototyping this as well, but I split this in two libraries because it seemed a bit strange to me that using "Dominance" would require me to ask for linking to libVerifier (I'm not opposed this though).

I think we can break the dependency by removing all of the "test" passes that are currently in the Analysis lib. They predate the Test dialect, and should really be moved out.

I think we can break the dependency by removing all of the "test" passes that are currently in the Analysis lib. They predate the Test dialect, and should really be moved out.

OpStats.cpp and MemRefBoundCheck.cpp also depend on the MLIRPassLibrary. What do you want to do with these?

I think we can break the dependency by removing all of the "test" passes that are currently in the Analysis lib. They predate the Test dialect, and should really be moved out.

OpStats.cpp and MemRefBoundCheck.cpp also depend on the MLIRPassLibrary. What do you want to do with these?

OpStats.cpp can be moved to Transforms for now, and MemRefBoundCheck is just a test pass so it can go into the test/lib/Transforms.

stephenneuendorffer retitled this revision from [MLIR] Break cyclic dependency between MLIRPass and MLIRAnalysis to [MLIR] Break cyclic dependencies with MLIRAnalysis.
stephenneuendorffer edited the summary of this revision. (Show Details)

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

mehdi_amini accepted this revision.Feb 4 2020, 8:29 PM
mehdi_amini added inline comments.
mlir/lib/Analysis/CMakeLists.txt
51

I suspect a bunch of this will end-up moving to the Affine dialect itself.

stephenneuendorffer edited the summary of this revision. (Show Details)

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: console-log.txt

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

This revision was not accepted when it landed; it landed in state Needs Review.Feb 5 2020, 11:33 AM
This revision was automatically updated to reflect the committed changes.