This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Parser] Don't hardcode the use of ModuleOp in the parser
ClosedPublic

Authored by rriddle on Dec 3 2020, 1:45 PM.

Details

Summary

This was important when ModuleOp was the only top level operation, but that isn't necessarily the case anymore. This is one of the last remaining aspects of the infrastructure that is hardcoded to ModuleOp.

Depends On D92450

Diff Detail

Event Timeline

rriddle created this revision.Dec 3 2020, 1:45 PM
rriddle requested review of this revision.Dec 3 2020, 1:45 PM
mehdi_amini accepted this revision.Dec 3 2020, 2:14 PM

Sweet!

mlir/include/mlir/Dialect/SPIRV/SPIRVModule.h
25

Could this be using OwningSPIRVModuleRef = OwningOpRef<spirv::ModuleOp>; ? (same for OwningModuleRef)

mlir/lib/Parser/Parser.cpp
2035

Does this assume that topLevelBlock isn't empty?

This revision is now accepted and ready to land.Dec 3 2020, 2:14 PM
rriddle marked 2 inline comments as done.Dec 3 2020, 3:22 PM
rriddle added inline comments.
mlir/include/mlir/Dialect/SPIRV/SPIRVModule.h
25

It can and should, but it involves updating each of the usages that forward declare the class. I'd rather keep that as a simple NFC followup to keep this revision as focused as possible.

mlir/lib/Parser/Parser.cpp
2035

Technically it's fine given that iplist is cyclic(previous node of the sentinel is the sentinel if empty), but switched to explicit check for empty anyways.

This revision was landed with ongoing or failed builds.Dec 3 2020, 3:54 PM
This revision was automatically updated to reflect the committed changes.
rriddle marked 2 inline comments as done.