[Mlir-commits] [mlir] [mlir][spirv] Add mgpu* wrappers for Vulkan runtime, migrate some tests (PR #123114)

[Jakub Kuderski]

================
@@ -26,6 +28,37 @@
 
 namespace {
 
+class VulkanModule;
+
+// Class to be a thing that can be returned from `mgpuModuleGetFunction`.
+struct VulkanFunction {
+  VulkanModule *module;
+  std::string name;
+
+  VulkanFunction(VulkanModule *module, const char *name)
+      : module(module), name(name) {}
+};
+
+// Class to own a copy of the SPIR-V provided to `mgpuModuleLoad` and to manage
+// allocation of pointers returned from `mgpuModuleGetFunction`.
+class VulkanModule {
+public:
+  VulkanModule(const char *ptr, size_t size) : blob(ptr, ptr + size) {}
+  ~VulkanModule() = default;
+
+  VulkanFunction *getFunction(const char *name) {
+    return functions.emplace_back(std::make_unique<VulkanFunction>(this, name))
+        .get();
+  }
+
+  uint8_t *blobData() { return reinterpret_cast<uint8_t *>(blob.data()); }
+  uint32_t blobSize() { return static_cast<uint32_t>(blob.size()); }
----------------
kuhar wrote:

> what the code actually does and the types it uses are a stronger signal than names or comments

I agree with this but this is not an all or nothing approach in absence of a dedicated unit type. Having worked with similar code in the past across multiple projects, I've learned to be extra cautious around how we represent SPIR-V modules. Also note that it's quite common to see guidelines around time units in many projects, e.g.:
* https://users.ece.cmu.edu/~eno/coding/CppCodingStandard.html#units
* https://ruudvanasseldonk.com/2022/03/20/please-put-units-in-names
* https://github.com/search?q=repo%3Airee-org%2Firee+inbytes+&type=code.

At google, we had an internal lint that would complain about any variables of primitive types named something like `duration` or `time` without an appropriate suffix.

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