This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Refactor ModuleState into AsmState and expose it to users.
ClosedPublic

Authored by rriddle on Jan 6 2020, 10:38 AM.

Details

Summary

This allows for users to cache printer state, which can be costly to recompute. Each of the IR print methods gain a new overload taking this new state class.

Depends On D72293

Diff Detail

Event Timeline

rriddle created this revision.Jan 6 2020, 10:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 6 2020, 10:38 AM

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

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

rriddle updated this revision to Diff 236854.Jan 8 2020, 10:10 AM

Tidy comment in AsmState.

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

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

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

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

Looks good on first walk through

mlir/include/mlir/IR/AsmState.h
8

File description?

27

What does higher level mean here? It is a bit overloaded term and I think you have a very explicit meaning in mind.

42

So the lifetime of the AsmStateImpl is linked to this object, and unique_ptr is used to enable only forward declaring it above?

mlir/lib/IR/AsmPrinter.cpp
2205

How is guaranteed that getImpl doesn't return null here? (e.g., verify that AsmState has been initialized)

rriddle updated this revision to Diff 237731.Jan 13 2020, 10:49 AM
rriddle marked 6 inline comments as done.

Address comments.

mlir/include/mlir/IR/AsmState.h
27

Switched to 'parent'.

42

Yes, this is the pImpl idiom.

mlir/lib/IR/AsmPrinter.cpp
2205

Ooops, meant to delete the default constructor. Thanks for the catch.

Unit tests: pass. 61744 tests passed, 0 failed and 780 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

jpienaar accepted this revision.Jan 14 2020, 8:21 AM

Looks good thanks

This revision is now accepted and ready to land.Jan 14 2020, 8:21 AM
This revision was automatically updated to reflect the committed changes.

Unit tests: pass. 61804 tests passed, 0 failed and 781 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml