[PATCH] D75876: [mlir][spirv] Let SPIRVConversionTarget consider type availability

[Lei Zhang via Phabricator via llvm-commits]

antiagainst marked 3 inline comments as done.
antiagainst added inline comments.


================
Comment at: mlir/lib/Dialect/SPIRV/SPIRVLowering.cpp:412
 std::unique_ptr<spirv::SPIRVConversionTarget>
 spirv::SPIRVConversionTarget::get(spirv::TargetEnvAttr targetEnv,
                                   MLIRContext *context) {
----------------
mravishankar wrote:
> mravishankar wrote:
> > Could we add a method that sets some targetEnv by default (the one that are used in the tests here for example). 
> Just to add to this. Then we can remove the "lookupTargetEnvOrDefault" method right? 
I'd like to keep the responsibility separate. We already have three helper functions for getting the target env: `getDefaultTargetEnv`, `lookupTargetEnv`, and `lookupTargetEnvOrDefault`. This class should be focusing on conversion target. If one want to just use the default target env, one can use `getDefaultTargetEnv` to create a `SPIRVConversionTarget`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75876/new/

https://reviews.llvm.org/D75876