[PATCH] D72533: [mlir] Add a signedness semantics bit to IntegerType

[Lei Zhang via Phabricator via llvm-commits]

antiagainst added inline comments.


================
Comment at: mlir/lib/IR/Attributes.cpp:283
+int64_t IntegerAttr::getInt() const {
+  assert(!getImpl()->getType().isUnsignedInteger() &&
+         "integer signedness unsupported");
----------------
rriddle wrote:
> This is a slightly concerning. Is your plan to have getSInt/getUInt? Or remove this all together?
I don't plan to add getSInt/getUInt at the moment because there is a TODO for removing this API entirely. I'm not sure whether that's still planned? @jpienaar 


================
Comment at: mlir/lib/IR/Attributes.cpp:696
+  // Make sure signedness semantics is consistent.
+  switch (intType.getSignedness()) {
+  case IntegerType::Signed:
----------------
rriddle wrote:
> Can you tighten this up a bit?
> 
> if (intType.isSignless())
>   return true;
> return intType.isSigned() ? isSigned : !isSigned;
I think that's effectively the same as the `switch`? Anyway, changed. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72533