[Mlir-commits] [mlir] Reland "[mlir][spirv] Fix UpdateVCEPass to deduce the correct set of capabilities" (PR #151502)

[Igor Wodiany]

================
@@ -95,6 +95,14 @@ static LogicalResult checkAndUpdateCapabilityRequirements(
   return success();
 }
 
+static void addAllImpliedCapabilities(SetVector<spirv::Capability> &caps) {
+  for (unsigned i = 0; i < caps.size(); ++i) {
+    spirv::Capability cap = caps[i];
+    ArrayRef<spirv::Capability> impliedCaps = getDirectImpliedCapabilities(cap);
+    caps.insert_range(impliedCaps);
+  }
----------------
IgWod-IMG wrote:

What if we just put `allCaps` into `caps` at the end of the function and return void? Then we can put comment on the function saying that it updates `caps` "in-place". Destruction becomes irrelevant and only one temporary vector is needed. I think from API perspective this is the cleanest solution. I think it’s confusing to have a function that returns something new but also destroys the input – updating the input to contain the result is fine. And I think this is what @kuhar suggests:

> Can we collect all the new caps in a new set vector and update caps at the very end after the loop?

But before making any changes let them confirm, what's their preferred solution. This is my interpretation of their comment :)

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