This is an archive of the discontinued LLVM Phabricator instance.

[mlir][spirv] Move type checks from dialect class to type hierarchy
ClosedPublic

Authored by antiagainst on Mar 16 2020, 11:52 AM.

Details

Summary

Types should be checked with the type hierarchy. This should result in
better responsibility division and API surface.

Depends On D76242

Diff Detail

Event Timeline

antiagainst created this revision.Mar 16 2020, 11:52 AM
mravishankar accepted this revision.Mar 16 2020, 12:30 PM

This is so much nicer! Thanks

mlir/lib/Dialect/SPIRV/SPIRVLowering.cpp
166

You could just make this a dyn_cast<spirv::SPIRVType>()

This revision is now accepted and ready to land.Mar 16 2020, 12:30 PM
antiagainst marked 2 inline comments as done.Mar 17 2020, 6:58 AM
antiagainst added inline comments.
mlir/lib/Dialect/SPIRV/SPIRVLowering.cpp
166

I think it's different. If this is not a SPIR-V type, then dyn_cast<spirv::SPIRVType>() gives us a Type() instead of llvm::None. The former will let the framework to try another conversion rule but the later will directly fail. (Although here it is the last conversion rule so probably doesn't matter that much.) Anyway, this is completely changed in the next commit (https://reviews.llvm.org/D76244). :)

antiagainst marked an inline comment as done.

Rebased

This revision was automatically updated to reflect the committed changes.