[PATCH] D72765: [mlir][spirv] Support implied extensions and capabilities

[River Riddle via Phabricator via llvm-commits]

rriddle added inline comments.


================
Comment at: mlir/lib/Dialect/SPIRV/SPIRVTypes.cpp:89
+    for (Capability c : getDirectImpliedCapabilities(allCaps[i]))
+      if (!llvm::is_contained(allCaps, c))
+        allCaps.push_back(c);
----------------
I would imagine this would be much simpler/efficient if you used a SetVector instead.


================
Comment at: mlir/tools/mlir-tblgen/SPIRVUtilsGen.cpp:1306
+
+    SmallVector<std::string, 1> impliedCaps;
+    std::vector<Record *> impliedCapsDefs = def.getValueAsListOfDefs("implies");
----------------
This seems unnecessarily inefficient. Can you just split the format below and use interleaveComma over the defs instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72765