This is an archive of the discontinued LLVM Phabricator instance.

[MLIR] Add support for defining and using Op specific analysis
ClosedPublic

Authored by jurahul on Jul 29 2020, 5:22 PM.

Details

Summary
  • Add variants of getAnalysis() and friends that operate on a specific derived operation types.
  • Add OpPassManager::getAnalysis() to always call the base getAnalysis() with OpT.
  • With this, an OperationPass can call getAnalysis<> using an analysis type that is generic (works on Operation *) or specific to the OpT for the pass. Anything else will fail to compile.
  • Extend AnalysisManager unit test to test this, and add a new PassManager unit test to test this functionality in the context of an OperationPass.

Diff Detail

Event Timeline

jurahul created this revision.Jul 29 2020, 5:22 PM
Herald added a project: Restricted Project. · View Herald Transcript
jurahul requested review of this revision.Jul 29 2020, 5:22 PM
jurahul updated this revision to Diff 283031.Aug 4 2020, 2:14 PM

Address clang-tidy warnings

Thanks Rahul.

mlir/include/mlir/Pass/AnalysisManager.h
132

Why is this guarded by a template?

136–142

typo: assuming

its -> it's

137

nit: Use triple tick comments here.

172

Why is the argument list variadic?

253

This method is unnecessary, please remove it. *Op methods already implicitly convert to Operation*, there is nothing gained here by providing the derived operation type.

253

Please fix all of the comments to be triple tick.

275

.getOperation() shouldn't be necessary.

290

This method is unnecessary, please remove it.

mlir/include/mlir/Pass/Pass.h
206

Same here, this method should be removed.

221

Same here, this method should be removed.

jurahul updated this revision to Diff 284539.Aug 10 2020, 5:53 PM

Address review comments

jurahul marked 10 inline comments as done.Aug 10 2020, 5:54 PM
jurahul added inline comments.
mlir/include/mlir/Pass/AnalysisManager.h
132

Removed.

rriddle added inline comments.Aug 13 2020, 11:52 AM
mlir/include/mlir/Pass/AnalysisManager.h
171–193

nit: Remove the double private:

mlir/unittests/Pass/PassManagerTest.cpp
67

I'm not sure that any of this is necessary to test, if the op specific analysis doesn't provide a proper constructor none of this should compile in the first place.

rriddle accepted this revision.Aug 13 2020, 12:53 PM
This revision is now accepted and ready to land.Aug 13 2020, 12:53 PM
jurahul marked an inline comment as done.Aug 13 2020, 8:42 PM
jurahul added inline comments.
mlir/unittests/Pass/PassManagerTest.cpp
67

Beyond just being able to compile, I think it would be good to test basic functionality. For instance, the correct op is passed into the constructor etc.

jurahul updated this revision to Diff 285552.Aug 13 2020, 8:44 PM

Remove double private

jurahul marked an inline comment as done.Aug 13 2020, 8:44 PM