[Mlir-commits] [mlir] [mlir][SPIR-V] Refine OpTypeImage capability inference (PR #195060)

[Igor Wodiany]

================
@@ -423,13 +425,102 @@ ImageSamplerUseInfo ImageType::getSamplerUseInfo() const {
 
 ImageFormat ImageType::getImageFormat() const { return getImpl()->format; }
 
+void TypeExtensionVisitor::addConcrete(ImageType type) {
+  // OpTypeImage with a 64-bit integer Sampled Type requires the
+  // SPV_EXT_shader_image_int64 extension (companion to Int64ImageEXT).
+  if (auto intTy = dyn_cast<IntegerType>(type.getElementType());
+      intTy && intTy.getWidth() == 64) {
+    static constexpr auto ext = Extension::SPV_EXT_shader_image_int64;
+    extensions.push_back(ext);
+  }
+  add(type.getElementType());
+}
+
 void TypeCapabilityVisitor::addConcrete(ImageType type) {
-  if (auto dimCaps = spirv::getCapabilities(type.getDim()))
-    capabilities.push_back(*dimCaps);
+  // Capability requirements for OpTypeImage are determined jointly by Dim,
+  // Sampled, MS, and Arrayed - see the SPIR-V spec's "Capabilities" column on
+  // OpTypeImage.
+  Dim dim = type.getDim();
+  bool isMultisampled =
+      type.getSamplingInfo() == ImageSamplingInfo::MultiSampled;
+  bool isArrayed = type.getArrayedInfo() == ImageArrayedInfo::Arrayed;
+  ImageSamplerUseInfo sampler = type.getSamplerUseInfo();
+  bool noSampler = sampler == ImageSamplerUseInfo::NoSampler;
+  bool needSampler = sampler == ImageSamplerUseInfo::NeedSampler;
+
+  switch (dim) {
+  case Dim::Dim1D: {
+    if (needSampler) {
+      static constexpr auto cap = Capability::Sampled1D;
+      capabilities.push_back(cap);
----------------
IgWod wrote:

I wonder whether it'd make sense to replace repeated:

```c++
static constexpr auto cap = Capability::Sampled1D;
capabilities.push_back(cap);
```

with a lambda that takes `Capability::X` as an argument. At the moment the code feels very verbose. Or can we pass `Capability::X` directly to `push_back`? That will allow to remove some braces and maybe the code will be a bit easier to follow?

Otherwise LGTM, but it'd be nice if the structure can be made so it's a bit easier to follow.

https://github.com/llvm/llvm-project/pull/195060