[PATCH] D76242: [mlir][spirv] Plumbing target environment into type converter

[Mahesh Ravishankar via Phabricator via llvm-commits]

mravishankar requested changes to this revision.
mravishankar added inline comments.
This revision now requires changes to proceed.


================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVLowering.h:77
   /// Creates a SPIR-V conversion target for the given target environment.
-  static std::unique_ptr<SPIRVConversionTarget> get(TargetEnvAttr targetEnv,
-                                                    MLIRContext *context);
+  static std::unique_ptr<SPIRVConversionTarget> get(TargetEnvAttr targetAttr);
 
----------------
Added a comment on the previous CL in this stack, but just mirroring here. I think its would be good to have a `get` method which will set the default target env. Its seems more ergonomic to me.


================
Comment at: mlir/include/mlir/Dialect/SPIRV/TargetAndABI.h:54
+  TargetEnvAttr targetAttr;
+  llvm::SmallSet<Extension, 4> givenExtensions;    /// Allowed extensions
+  llvm::SmallSet<Capability, 8> givenCapabilities; /// Allowed capabilities
----------------
These might cause subtle issues later  on. Can we make this a SetVector?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76242