[PATCH] D75875: [mlir][spirv] Support querying type extension/capability requirements

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 12 15:45:23 PDT 2020


rriddle added inline comments.


================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVTypes.h:291
+
+    /// Allow StructType class access to cconstructors.
+    friend class ElementTypeRange;
----------------
typo: cconstructors 


================
Comment at: mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp:58
+
+      emitError(op->getLoc(), "'")
+          << op->getName() << "' requires at least one extension in ["
----------------
nit: Return the error directly.


================
Comment at: mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp:60
+          << op->getName() << "' requires at least one extension in ["
+          << llvm::join(extStrings, ", ")
+          << "] but none allowed in target environment";
----------------
nit: I don't think the join is actually necessary.


================
Comment at: mlir/lib/Dialect/SPIRV/Transforms/UpdateVCEPass.cpp:92
+
+      emitError(op->getLoc(), "'")
+          << op->getName() << "' requires at least one capability in ["
----------------
Same here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75875





More information about the llvm-commits mailing list