[PATCH] D75610: [mlir][ods] Improve integer signedness modelling

Lei Zhang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 4 11:15:44 PST 2020


antiagainst added a subscriber: ftynse.
antiagainst added inline comments.


================
Comment at: mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td:569
+                         SingleBlockImplicitTerminator<"ReturnOp">, Symbol]> {
+  let arguments = (ins
+    TypeAttr:$type,
----------------
rriddle wrote:
> IMO: we should cleanup all of these definitions to not use `Arguments`. It is much much cleaner.
I agree. This one is a bit extreme given we have many fields already.  But I'll leave that to @ftynse to decide.


================
Comment at: mlir/include/mlir/Dialect/SPIRV/SPIRVBase.td:66
   let predicate = And<[
-    IntegerAttrBase<I32, "">.predicate,
+    I32Attr.predicate,
     SPV_IsKnownEnumCaseFor<name>,
----------------
mravishankar wrote:
> Do we want to used unsigned attribute here, i.e UI32Attr.
Core EnumAttr and EnumAttrCase is still based on signless integers; this is to be consistent there given we are just simplifying the verifications for the known case comparison. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75610





More information about the llvm-commits mailing list