[mlir] Add support for some unsigned integers to DenseArrayAttr

[Mogball]

Summary

This patch adds support for UI8, UI16, UI32, and UI64 to DenseArrayAttr,
which correspond to the C++ uint8_t, uint16_t, uint32_t, and
uint64_t types.

Depends on D130957

[jpienaar]

mlir/include/mlir/IR/BuiltinAttributes.td

184

We are using signless and unsigned here now, should this be signed and unsigned instead?

mlir/lib/AsmParser/AttributeParser.cpp

881

I almost wonder if it wouldn't read easier with if unsigned check on outside (if (unsigned) { ... return } ), this just feels a bit dense. (Using templated helper could also help)

[mehdi_amini]

The motivation for this isn't clear to me right now: can you elaborate? Why wouldn't we always use signed int for these? (considering their intended purpose).

[Mogball]

The element types of dense integer arrays are currently always signless integers. As Jacques mentioned, we could switch them to be explicitly signed (and unsigned) instead of signless.

Always signed might make sense for the original purpose of the attribute, but having a dense array of primitives is also very convenient for a lot of lower-level uses beyond "an array of indices". E.g. ArrayRef<T> can be written directly to a binary format and mapped back into memory, and for the same reasons as its original introduction, a dense array can be much more ergonomic than a dense elements attribute (not worrying about shape and splats, and a much more efficient storage).

[jpienaar]

In D130958#3693890, @Mogball wrote:

The element types of dense integer arrays are currently always signless integers. As Jacques mentioned, we could switch them to be explicitly signed (and unsigned) instead of signless.

Always signed might make sense for the original purpose of the attribute, but having a dense array of primitives is also very convenient for a lot of lower-level uses beyond "an array of indices". E.g. ArrayRef<T> can be written directly to a binary format and mapped back into memory, and for the same reasons as its original introduction, a dense array can be much more ergonomic than a dense elements attribute (not worrying about shape and splats, and a much more efficient storage).

Another option is (and I think this may be closer to what Mehdi intended to ask) is that this is signless and up to user/op to query correct type, this is just storage. Choice between signed and unsigned here semi arbitrary then (and unsigned may even be better as less interpretation of bits), and accessor has a signed and unsigned variant. Is that what you were thinking Mehdi?

[lattner]

You're right that signedness doesn't affect the semantics of the bits in memory, but some clients care about signed semantics in their type system. I think both approaches are principled, but we should look to make MLIR consistent across the builtin storage attributes. For example, if IntegerAttr supports/maintains the i32/si32/ui32 distinction then it would make sense for DenseArrayAttr to support them as well. Alternatively, it would also be principled for nothing to support signs in the storage types: both designs work but we shouldn't do half and half :-)

Incidentally, I don't think it makes sense to support just i32 and ui32. If DenseArrayAttr should support signedness, it should support all of i64, ui64, si64 rather than overloading si64 onto i64.

[mehdi_amini]

It depends on what is the intent behind DenseArrayAtt : I originally thought it not to work for replacing DenseElementsAttr for large constants, but instead of all the many small array of dimensions like my.transpose %tensor [0, 2, 1] where we have small integer arrays. The only reason I added a type was 1) Type was a mandatory part of attributes until very recently 2) some early feedback from River was going along the line of making it implement the right set of interface (because why not...).

From this point of view, there is no really need for consistency / completeness with respect to IntegerAttr: it's just a different use-case.

[Mogball]

I do think there's value is using DenseArrayAttr for "medium"-sized constants (i.e. small enough to not warrant an external resource) in a way that still supports signedness semantics like IntegerAttr, if not only to make it conform better to the ElementsAttr interface (e.g. attr.tryGetValues<int64_t>() on an i32 array crashes instead of gracefully failing). For example, DenseArrayAttr has an efficient storage format without a lot of the complexity associated with DenseElementsAttr. I'm probably going to abandon this patch, because I think the better approach is to make it like DenseIntOrFPElementsAttr. Thougths?

[jpienaar]

In D130958#3706919, @Mogball wrote:

I do think there's value is using DenseArrayAttr for "medium"-sized constants (i.e. small enough to not warrant an external resource) in a way that still supports signedness semantics like IntegerAttr, if not only to make it conform better to the ElementsAttr interface (e.g. attr.tryGetValues<int64_t>() on an i32 array crashes instead of gracefully failing). For example, DenseArrayAttr has an efficient storage format without a lot of the complexity associated with DenseElementsAttr. I'm probably going to abandon this patch, because I think the better approach is to make it like DenseIntOrFPElementsAttr. Thougths?

And by that you mean having a combined base and derived instances with differently typed accessors?

[Mogball]

And by that you mean having a combined base and derived instances with differently typed accessors?

Yes.

[mehdi_amini]

In D130958#3706919, @Mogball wrote:

I do think there's value is using DenseArrayAttr for "medium"-sized constants (i.e. small enough to not warrant an external resource)

What are these? Do you have an example of op that has this kind of constants and that by design they will always be small? Also, if such dialect exists: wouldn't such an op be so esoteric that they could define their own attribute instead of having to add this to builtin?

[Mogball]

Do you have an example of op that has this kind of constants and that by design they will always be small?

One example might be:

// Create a new vector using the elements at the given indices
%0 = foo.splice %arg0[0, 2, 3, 6, 7] : vector<32xf32> -> vector<5xf32>

The operation might be implemented directly as kernel instead of being lowered. The list of indices can get quite "long" but would never be so long that it would not longer make sense to use a dense array. The operation could have signedness semantics on the indices.

I originally thought it not to work for replacing DenseElementsAttr for large constants, but instead of all the many small array of dimensions like my.transpose %tensor [0, 2, 1] where we have small integer arrays

Then why does it support floating point elements?

DenseElementsAttr has a lot of awkwardness and overhead when you just want a flat list of primitive elements, which is why DenseArrayAttr was created in the first place (for a list of indices). I also think it's well-positioned to just be a lighter version of DenseElementsAttr for 1D data.

[mehdi_amini]

In D130958#3707480, @Mogball wrote:

Do you have an example of op that has this kind of constants and that by design they will always be small?

One example might be:

// Create a new vector using the elements at the given indices
%0 = foo.splice %arg0[0, 2, 3, 6, 7] : vector<32xf32> -> vector<5xf32>

The operation might be implemented directly as kernel instead of being lowered. The list of indices can get quite "long" but would never be so long that it would not longer make sense to use a dense array. The operation could have signedness semantics on the indices.

I don't see why we need to define a MLIR type for the content of such attributes? This is exactly what I had in mind for this attribute initially and I was intended to support ArrayRef<int64_t> without a type, because why would we do differently here?

Then why does it support floating point elements?

Here is the original revision https://reviews.llvm.org/D116391
I only added float because because River asked me to support a bit more than int64_t and that when I wrote the template it was trivial to instantiate for floating points.

DenseElementsAttr has a lot of awkwardness and overhead when you just want a flat list of primitive elements, which is why DenseArrayAttr was created in the first place (for a list of indices). I also think it's well-positioned to just be a lighter version of DenseElementsAttr for 1D data.

Yes but I would want to see the use-cases for this lighter version that support having more element type first, the one you proposed above isn't convincing to me.

[Mogball]

I don't see why we need to define a MLIR type for the content of such attributes?

But DenseArrayAttr does have a type. If it's just there to conform with the ElementsAttr interface, then that begs the question of why this attribute needs to implement that interface. ArrayAttr doesn't implement ElementsAttr.

I only added float because because River asked me to support a bit more than int64_t

If it can support more than just ArrayRef<int64_t>, the original motivating use-case of the attribute, then wouldn't that mean it's also suitable for other use-cases? If the only purpose of the attribute is to be an array of indices, why have floats and integers of other sizes? Why implement ElementsAttr?

I would want to see the use-cases for this lighter version that support having more element type first

For one, it's a much more natural way to expression 1D vector constants %0 = vector.constant [:ui8 2, 5, 7] : vector<3xui8>. DenseElementsAttr makes this annoying in particular due to its splat handling.

On a side note, what of the discussion wrt its syntax? I believe there was an agreement to transition the syntax towards something like array<...> or vector<...>?

[mehdi_amini]

In D130958#3707749, @Mogball wrote:

I don't see why we need to define a MLIR type for the content of such attributes?

But DenseArrayAttr does have a type.

Sure, but your example does not motivate adding new types, which is what I'm asking here: a motivating example.

If it can support more than just ArrayRef<int64_t>, the original motivating use-case of the attribute, then wouldn't that mean it's also suitable for other use-cases? If the only purpose of the attribute is to be an array of indices, why have floats and integers of other sizes? Why implement ElementsAttr?

These are good questions, but they aren't relevant to justify this patch IMO.

I would want to see the use-cases for this lighter version that support having more element type first

For one, it's a much more natural way to expression 1D vector constants %0 = vector.constant [:ui8 2, 5, 7] : vector<3xui8>. DenseElementsAttr makes this annoying in particular due to its splat handling.

Vectors are multi-dimensional though.

On a side note, what of the discussion wrt its syntax? I believe there was an agreement to transition the syntax towards something like array<...> or vector<...>?

Yes: this needs to be implemented, I didn't get to it but if someone wants to beat me to it :)

[Mogball]

These are good questions, but they aren't relevant to justify this patch IMO.

Oh, I'm not too worried about this specific patch. I'm more talking about the design of the attribute in general: what it's intended to be and what it's intended to support. If we decide that this patch isn't the right way to go about realizing that design, I'm happy to explore other approaches (e.g., introducing an actual DenseArrayAttr class, or switching to signed and unsigned integers instead of signless, as Jacques suggested).

Sure, but your example does not motivate adding new types, which is what I'm asking here: a motivating example.

If we're talking about this specific patch (adding uint_32t etc. C++ data types to dense arrays), besides unsigned 1D vectors, an operation using this attribute to represent a list of indices/sizes might prefer unsigned integers.

[lattner]

Responding to a couple of questions, paraphrasing:

On "why/how does this only exist for mid-sized things": well one example would be for tensor constants, where you store large constants as resources, and use a cost model to have "small enough ones" inline to make constant folding or other transformations have direct access to them. This may or may not be a good idea, but it is a reasonable thing for someone in the ecosystem to want to do.

Your example of GEP indices is the same thing.

On "why do we carry an element type": my understanding is that you wanted to use this for GEP and shapes and other things that require /interpretation/ of the data, it isn't just a "blob of bytes". Given this you need to know the element size, which then creeps into whether something is int or float, and then immediately turns into "actually I wanted typed accessors in C++". C++ has signs, and it is extremely reasonable IMO for those typed accessors to work with C++ type system.

On "integers can be different than DenseArrayAttr": I agree of course, we can make a choice here. I just don't understand why it would be /better/ for it to be different. Being consistent has good ecosystem reasons, and while (e.g.) the LLVM dialect enjoys signless integers, frontend-y dialects enjoy signed integers. I don't see why that would be any different for individual integers/floats than for a quarternion implemented with 4 of them in a DenseArrayAttr.

-Chris

[Mogball]

an operation using this attribute to represent a list of indices/sizes might prefer unsigned integers.

I'm refactoring the LLVM dialect right now, and the indices of ExtractValueOp and InsertValueOp are unsigned, so they would prefer ArrayRef<uint32_t> to ArrayRef<int32_t>, for example.

[mehdi_amini]

On "why do we carry an element type": my understanding is that you wanted to use this for GEP and shapes and other things that require /interpretation/ of the data, it isn't just a "blob of bytes".

Sure: but all these cases don't need a "generic introspectable way to look into the data". That is the client of your GEP operation are gonna use the ODS-generated class to do: gepOp.getIndices() which returns an ArrayRef<int64_t> and clients know how to use this.
Nowhere in this use-case you actually expose or need gepOp.getIndiceAttr().getType().cast<ShapedType>().getElementType().cast<IntegerType>().isSignless() or similar.

[lattner]

Right, I think that use case can work without this, with less safety/checking that having this checked and enforced in ODS. Looking beyond the LLVM dialect, are you arguing that properly typed accessors aren't useful for any client?

Alternatively, are you worried there is no use-case for this information?

What I'm looking for is a better way to model "I have an ArrayAttr of 4 homogenous IntegerAttrs" (e.g. a frontend with an integer quaternion constant). IntegerAttr carries signedness, but DenseArrayAttr does not, so it isn't a direct replacement. It would be convenient for DenseArrayAttr to maintain the sign just like IntegerAttr does, making it a drop in replacement for this. It is entirely possible to put the signedness/type somewhere else (e.g. have a new attribute that is a DenseArrayAttr+Type), but such a design is inconsistent with the IntegerAttr approach.

I also don't see a downside to maintaining the information. Are you opposed to it, or just unclear why it is valuable?

[mehdi_amini]

Are you opposed to it, or just unclear why it is valuable?

I'm opposed to it, because it is unclear why it is valuable.
That is: it adds design complexity where someone in ODS now has to choose between multiple options without obvious discriminator. That leads to fragmentation and inconsistencies (the kind of complexity that contributed to plague DenseElementsAttr users).
I see the case of a "gep" operation or the slice @Mogball mentioned above as intrinsically different than a constant op. The constant op is just materializing an SSA value for the attribute, which justifies supporting MLIR type associated with the attribute somehow, but for the others it does not. It isn't clear to me why it is a good thing to unify the attribute for both cases, even they both store "an array" ultimately.

So what are the use cases beyond constant? Why does this warrant a builtin attribute instead of a dialect specific one (in particular since we have the ElementsAttr interface to unify introspecting the data?).

Also, the main reason I actually wanted a builtin attribute here was because it offers a textual syntax that can't be reached by a dialect attribute (without writing C++ printer/parser), that is printing %0 = foo.splice %arg0 [0, 2, 3, 6, 7] instead of `%0 = foo.splice %arg0 <[0, 2, 3, 6, 7]>.

[lattner]

I'm opposed to it, because it is unclear why it is valuable.
That is: it adds design complexity where someone in ODS now has to choose between multiple options without obvious discriminator. That leads to fragmentation and inconsistencies (the kind of complexity that contributed to plague DenseElementsAttr users).
I see the case of a "gep" operation or the slice @Mogball mentioned above as intrinsically different than a constant op. The constant op is just materializing an SSA value for the attribute, which justifies supporting MLIR type associated with the attribute somehow, but for the others it does not. It isn't clear to me why it is a good thing to unify the attribute for both cases, even they both store "an array" ultimately.

I think there may be some disconnect or misunderstanding here, it is entirely possible it is mine!

Zoom into something like the LLVM dialect extract/insertvalue operations. These are defined like this:

def LLVM_ExtractValueOp : LLVM_Op<"extractvalue", [NoSideEffect]> {
  let summary = "Extract a value from an LLVM struct.";

  let arguments = (ins LLVM_AnyAggregate:$container, DenseI64ArrayAttr:$position);
  let results = (outs LLVM_Type:$res);

  let builders = [
    OpBuilder<(ins "Value":$container, "ArrayRef<int64_t>":$position)>
  ];

"position" is currently ODS'd into these helpers:

::mlir::DenseI64ArrayAttr getPositionAttr();
::llvm::ArrayRef<int64_t> getPosition();

Note that we (arbitrarily) project the elements of position as a signed integer - this just made the leap from "DenseArrayAttr is untyped storage" to "operators that use DenseArrayAttr should use signed integers for their accessors". The former doesn't lead to the later: many operators will have opinionated ideas of what their API should be for a million different reasons, including things as mundane as "I use ArrayRef<uint8_t> pervasively in my codebase and I don't want to always be casting from ArrayRef<int8_t> to ArrayRef<uint8_t> every time I use an accessor. C++ is a typed language as you know, and it is important for ODS to generate /good/ APIs for operators implicitly.

Furthermore, beyond the simple case here, there are use cases beyond constant in pervasively signful operators (e.g. things like tensorflow ops) that take "attributes" where the sign is meaningful when generating C++ bindings (or for a million other reasons). If the attribute does not carry a sign natively, then you need to use a wrapper attribute that carries the sign plus the DenseArrayAttr. This is possible, but is awkward and is inconsistent with IntegerAttr as I mentioned upthread.

Furthermore, the entire point of DenseArrayAttr is to be typed: it is not an opaque blob of bytes -- it already carries the size of the elements. There is no downside I'm aware of to allowing dialect authors to store signedness, and (as I've mentioned before) this already is proven useful for the adjacent concept of IntegerAttr.

Why does this warrant a builtin attribute instead of a dialect specific one (in particular since we have the ElementsAttr interface to unify introspecting the data?).
Also, the main reason I actually wanted a builtin attribute here was because it offers a textual syntax that can't be reached by a dialect attribute (without writing C++ printer/parser), that is printing %0 = foo.splice %arg0 [0, 2, 3, 6, 7] instead of `%0 = foo.splice %arg0 <[0, 2, 3, 6, 7]>.

I believe the "main reason" you point out is true regardless of whether the elements are signed or not. LLVM_ExtractValueOp is a bit of a silly example of this given that insanely large indices aren't a thing, but imagine something like the LLVM 1.0 version of GEP that only allowed indices up to 256 and so store them as an array of bytes:

def PseudoLLVM1_ExtractValueOp : LLVM_Op<"extractvalue", [NoSideEffect]> {
  let arguments = (ins LLVM_AnyAggregate:$container, DenseI8ArrayAttr:$position);

I really wouldn't want to have index #128 printed as "-128" in the MLIR dump! These are unsigned values! I want to declare this as:

def PseudoLLVM1_ExtractValueOp : LLVM_Op<"extractvalue", [NoSideEffect]> {
  let arguments = (ins LLVM_AnyAggregate:$container, DenseUI8ArrayAttr:$position);

and have the printer/parser and accessors all be properly uint8_t instead of int8_t.

-Chris

[Mogball]

Sorry, I should've posted an update here. Mehdi and I spoke offline and we agreed to move forward on adding a proper, fully-functional DenseArrayAttr base class with the existing specializations while maintaining the convenience of the subclasses. The base class will be fully-functional and will not be a discriminated union. Afterwards, I will add support for signedness semantics and querying with unsigned integers.

[lattner]

Ok - are you planning to go with something like the current patch where you just add unsigend integers (I8 and UI8), or are you also adding explicitly signed things as well (I8, UI8 and SI8)?

[Mogball]

The goal is to have full support for signless, signed, and unsigned integers (as well as floats and doubles).

If it's just unsigned integers, then this patch will do fine, but we also talked about the incomplete implementation of ElementsAttr. ElementsAttr should allow generic iteration of the underlying data types, but the implementation of the attribute right now does not. So either we don't implement the interface because it isn't a good fit for the attribute or we add a fully-function base class that can dynamically dispatch iterators. We agreed to go with the latter.

[lattner]

awesome! thanks :)

[mehdi_amini]

In D130958#3715032, @lattner wrote:

"position" is currently ODS'd into these helpers:

::mlir::DenseI64ArrayAttr getPositionAttr();
::llvm::ArrayRef<int64_t> getPosition();

Note that we (arbitrarily) project the elements of position as a signed integer - this just made the leap from "DenseArrayAttr is untyped storage" to "operators that use DenseArrayAttr should use signed integers for their accessors".

Something should be explicit here: you're talking about "C++ signed integer" here while the whole discussion has been as I understood it about MLIR Type system.
Using ArrayRef<int64_t> for an array of MLIR signless i64 seems fine to me for example.

including things as mundane as "I use ArrayRef<uint8_t> pervasively in my codebase and I don't want to always be casting from ArrayRef<int8_t> to ArrayRef<uint8_t> every time I use an accessor. C++ is a typed language as you know, and it is important for ODS to generate /good/ APIs for operators implicitly.

Right, and I believe in consistency in MLIR, so I rather not see one dialect using ArrayRef<uint8_t> while other use ArrayRef<int8_t> without good reason.
I don't believe builtin / core should just offer all possible ways to do something "because we can". If a users wants to diverge, they can: we offer an extensible system and they can built their own attribute.

Furthermore, beyond the simple case here, there are use cases beyond constant in pervasively signful operators (e.g. things like tensorflow ops) that take "attributes" where the sign is meaningful when generating C++ bindings (or for a million other reasons)> If the attribute does not carry a sign natively, then you need to use a wrapper attribute that carries the sign plus the DenseArrayAttr. This is possible, but is awkward and is inconsistent with IntegerAttr as I mentioned upthread.

I am confused: it seems you're mixing again the C++ type we're exposing with whether there is an MLIR type?
As I wrote before, I haven't seen a case beyond constant where an MLIR type is relevant.

Furthermore, the entire point of DenseArrayAttr is to be typed: it is not an opaque blob of bytes

From a generic MLIR introspection, I don't see why it can't be opaque? (other than constant...).

it already carries the size of the elements. There is no downside I'm aware of to allowing dialect authors to store signedness, and (as I've mentioned before) this already is proven useful for the adjacent concept of IntegerAttr.

I'm not convinced that IntegerAttr is a good design for the kind of things we're talking about here: that is an attribute specific to an operation like dma.wait channel = 42 ; that is the channel id does not need an MLIR type either here. This is an opaque blob that only the Op can know about.

Why does this warrant a builtin attribute instead of a dialect specific one (in particular since we have the ElementsAttr interface to unify introspecting the data?).
Also, the main reason I actually wanted a builtin attribute here was because it offers a textual syntax that can't be reached by a dialect attribute (without writing C++ printer/parser), that is printing %0 = foo.splice %arg0 [0, 2, 3, 6, 7] instead of `%0 = foo.splice %arg0 <[0, 2, 3, 6, 7]>.

I believe the "main reason" you point out is true regardless of whether the elements are signed or not. LLVM_ExtractValueOp is a bit of a silly example of this given that insanely large indices aren't a thing, but imagine something like the LLVM 1.0 version of GEP that only allowed indices up to 256 and so store them as an array of bytes:

def PseudoLLVM1_ExtractValueOp : LLVM_Op<"extractvalue", [NoSideEffect]> {
  let arguments = (ins LLVM_AnyAggregate:$container, DenseI8ArrayAttr:$position);

I really wouldn't want to have index #128 printed as "-128" in the MLIR dump! These are unsigned values! I want to declare this as:

Sure, but you can use a DenseArrayAttribute with i16, or write your own attribute: again we don't have to provide a builtin support for all the possible edge cases, as long as there are convenient solution for these (we use to not have much extensibility originally, but we're beyond this).

The way I see it: we wil have a much better "product" if it is consistent and limits the number of ways to do the same things, encouraging to avoid fragmentation in the ecosystem for the 99% common case, while allowing special cases to be supported through extension (like Dialect attribute).

We may have a way forward with @Mogball, but I am waiting to see how it'll pan out: because I really don't want users to write DenseUI8ArrayAttr for the next splice op (so for example I strongly object to expose any of this in ODS even if we add C++ support for this).

[lattner]

Ok, I'm happy to see what Jeff has in mind.

Let me know if you want to chat about this sometime though. I think we have fairly different views on how MLIR can best serve authors of domain-specific compilers.

My viewpoint is that it is useful to allow dialect authors to use the builtin machinery in a flexible way (again, trivial example is integerattr supporting signful use cases), but it seems that you prefer that the builtin support is minimal, forcing dialect authors to define their own things instead of using the builtin features.

There are pro's and con's to both approaches for sure, it could be good to talk through the tradeoffs in person or over zoom or something.

[Mogball]

This has been succeeded by https://reviews.llvm.org/D132758

mlir/include/mlir/IR/BuiltinAttributes.h

public:
 
   /// Print the short form `[42, 100, -1]` without any type prefix.
   void print(AsmPrinter &printer) const;
   void print(raw_ostream &os) const;
   /// Print the short form `42, 100, -1` without any braces or type prefix.
   void printWithoutBraces(raw_ostream &os) const;
 
   /// Parse the short form `[42, 100, -1]` without any type prefix.
-  static Attribute parse(AsmParser &parser, Type odsType);
+  static Attribute parse(AsmParser &parser, Type type);
 
   /// Parse the short form `42, 100, -1` without any type prefix or braces.
-  static Attribute parseWithoutBraces(AsmParser &parser, Type odsType);
+  static Attribute parseWithoutBraces(AsmParser &parser, Type type);
 
   /// Support for isa<>/cast<>.
   static bool classof(Attribute attr);
 };
 template <>
 void DenseArrayAttr<int8_t>::printWithoutBraces(raw_ostream &os) const;
+template <>
+void DenseArrayAttr<uint8_t>::printWithoutBraces(raw_ostream &os) const;
 
 extern template class DenseArrayAttr<bool>;
 extern template class DenseArrayAttr<int8_t>;
 extern template class DenseArrayAttr<int16_t>;
 extern template class DenseArrayAttr<int32_t>;
 extern template class DenseArrayAttr<int64_t>;
+extern template class DenseArrayAttr<uint8_t>;
+extern template class DenseArrayAttr<uint16_t>;
+extern template class DenseArrayAttr<uint32_t>;
+extern template class DenseArrayAttr<uint64_t>;
 extern template class DenseArrayAttr<float>;
 extern template class DenseArrayAttr<double>;
 } // namespace detail
 
 // Public name for all the supported DenseArrayAttr
 using DenseI1ArrayAttr = detail::DenseArrayAttr<bool>;
 using DenseI8ArrayAttr = detail::DenseArrayAttr<int8_t>;
 using DenseI16ArrayAttr = detail::DenseArrayAttr<int16_t>;
 using DenseI32ArrayAttr = detail::DenseArrayAttr<int32_t>;
 using DenseI64ArrayAttr = detail::DenseArrayAttr<int64_t>;
+using DenseUI8ArrayAttr = detail::DenseArrayAttr<uint8_t>;
+using DenseUI16ArrayAttr = detail::DenseArrayAttr<uint16_t>;
+using DenseUI32ArrayAttr = detail::DenseArrayAttr<uint32_t>;
+using DenseUI64ArrayAttr = detail::DenseArrayAttr<uint64_t>;
 using DenseF32ArrayAttr = detail::DenseArrayAttr<float>;
 using DenseF64ArrayAttr = detail::DenseArrayAttr<double>;
 
 //===----------------------------------------------------------------------===//
 // DenseResourceElementsAttr
 
 namespace detail {
 /// Base class for DenseResourceElementsAttr that is instantiated and

mlir/include/mlir/IR/BuiltinAttributes.td

let description = [{
     [1, 2, 3]
     ```
   }];
   let parameters = (ins AttributeSelfTypeParameter<"", "ShapedType">:$type,
                         "DenseArrayBaseAttr::EltType":$eltType,
                         ArrayRefParameter<"char">:$elements);
   let extraClassDeclaration = [{
     // All possible supported element type.
-    enum class EltType { I1, I8, I16, I32, I64, F32, F64 };
+    enum class EltType {
+      I1, I8, I16, I32, I64, UI8, UI16, UI32, UI64, F32, F64
+    };
 
     /// Allow implicit conversion to ElementsAttr.
     operator ElementsAttr() const {
       return *this ? cast<ElementsAttr>() : nullptr;
     }
 
     /// ElementsAttr implementation.
     using ContiguousIterableTypesT =
-        std::tuple<bool, int8_t, int16_t, int32_t, int64_t, float, double>;
+        std::tuple<bool, int8_t, int16_t, int32_t, int64_t, uint8_t, uint16_t,
+                   uint32_t, uint64_t, float, double>;
     const bool *value_begin_impl(OverloadToken<bool>) const;
     const int8_t *value_begin_impl(OverloadToken<int8_t>) const;
     const int16_t *value_begin_impl(OverloadToken<int16_t>) const;
     const int32_t *value_begin_impl(OverloadToken<int32_t>) const;
     const int64_t *value_begin_impl(OverloadToken<int64_t>) const;
+    const uint8_t *value_begin_impl(OverloadToken<uint8_t>) const;
+    const uint16_t *value_begin_impl(OverloadToken<uint16_t>) const;
+    const uint32_t *value_begin_impl(OverloadToken<uint32_t>) const;
+    const uint64_t *value_begin_impl(OverloadToken<uint64_t>) const;
     const float *value_begin_impl(OverloadToken<float>) const;
     const double *value_begin_impl(OverloadToken<double>) const;
 
     /// Returns the shaped type, containing the number of elements in the array
     /// and the array element type.
     ShapedType getType() const;
     /// Returns the element type.
     EltType getElementType() const;

mlir/lib/AsmParser/AttributeParser.cpp

if (auto intType = type.dyn_cast<IntegerType>()) {
     switch (type.getIntOrFloatBitWidth()) {
     case 1:
       if (isEmptyList)
         result = DenseI1ArrayAttr::get(parser.getContext(), {});
       else
         result = DenseI1ArrayAttr::parseWithoutBraces(parser, Type{});
       break;
     case 8:
+      if (type.isUnsignedInteger()) {
+        if (isEmptyList)
+          result = DenseUI8ArrayAttr::get(parser.getContext(), {});
+        else
+          result = DenseUI8ArrayAttr::parseWithoutBraces(parser, Type{});
+      } else {
         if (isEmptyList)
           result = DenseI8ArrayAttr::get(parser.getContext(), {});
         else
           result = DenseI8ArrayAttr::parseWithoutBraces(parser, Type{});
+      }
       break;
     case 16:
+      if (type.isUnsignedInteger()) {
+        if (isEmptyList)
+          result = DenseUI16ArrayAttr::get(parser.getContext(), {});
+        else
+          result = DenseUI16ArrayAttr::parseWithoutBraces(parser, Type{});
+      } else {
         if (isEmptyList)
           result = DenseI16ArrayAttr::get(parser.getContext(), {});
         else
           result = DenseI16ArrayAttr::parseWithoutBraces(parser, Type{});
+      }
       break;
     case 32:
+      if (type.isUnsignedInteger()) {
+        if (isEmptyList)
+          result = DenseUI32ArrayAttr::get(parser.getContext(), {});
+        else
+          result = DenseUI32ArrayAttr::parseWithoutBraces(parser, Type{});
+      } else {
         if (isEmptyList)
           result = DenseI32ArrayAttr::get(parser.getContext(), {});
         else
           result = DenseI32ArrayAttr::parseWithoutBraces(parser, Type{});
+      }
       break;
     case 64:
+      if (type.isUnsignedInteger()) {
+        if (isEmptyList)
+          result = DenseUI64ArrayAttr::get(parser.getContext(), {});
+        else
+          result = DenseUI64ArrayAttr::parseWithoutBraces(parser, Type{});
+      } else {
         if (isEmptyList)
           result = DenseI64ArrayAttr::get(parser.getContext(), {});
         else
           result = DenseI64ArrayAttr::parseWithoutBraces(parser, Type{});
+      }
       break;
     default:
       emitError(typeLoc, "expected i1, i8, i16, i32, or i64 but got: ") << type;
       return {};
     }
   } else if (auto floatType = type.dyn_cast<FloatType>()) {
     switch (type.getIntOrFloatBitWidth()) {
     case 32:

mlir/lib/IR/BuiltinAttributes.cpp

DenseArrayBaseAttr::EltType DenseArrayBaseAttr::getElementType() const {
   return getImpl()->eltType;
 }
 
 ShapedType DenseArrayBaseAttr::getType() const { return getImpl()->type; }
 
 const bool *DenseArrayBaseAttr::value_begin_impl(OverloadToken<bool>) const {
   return cast<DenseI1ArrayAttr>().asArrayRef().begin();
 }
-
 const int8_t *
 DenseArrayBaseAttr::value_begin_impl(OverloadToken<int8_t>) const {
   return cast<DenseI8ArrayAttr>().asArrayRef().begin();
 }
 const int16_t *
 DenseArrayBaseAttr::value_begin_impl(OverloadToken<int16_t>) const {
   return cast<DenseI16ArrayAttr>().asArrayRef().begin();
 }
 const int32_t *
 DenseArrayBaseAttr::value_begin_impl(OverloadToken<int32_t>) const {
   return cast<DenseI32ArrayAttr>().asArrayRef().begin();
 }
 const int64_t *
 DenseArrayBaseAttr::value_begin_impl(OverloadToken<int64_t>) const {
   return cast<DenseI64ArrayAttr>().asArrayRef().begin();
 }
+const uint8_t *
+DenseArrayBaseAttr::value_begin_impl(OverloadToken<uint8_t>) const {
+  return cast<DenseUI8ArrayAttr>().asArrayRef().begin();
+}
+const uint16_t *
+DenseArrayBaseAttr::value_begin_impl(OverloadToken<uint16_t>) const {
+  return cast<DenseUI16ArrayAttr>().asArrayRef().begin();
+}
+const uint32_t *
+DenseArrayBaseAttr::value_begin_impl(OverloadToken<uint32_t>) const {
+  return cast<DenseUI32ArrayAttr>().asArrayRef().begin();
+}
+const uint64_t *
+DenseArrayBaseAttr::value_begin_impl(OverloadToken<uint64_t>) const {
+  return cast<DenseUI64ArrayAttr>().asArrayRef().begin();
+}
 const float *DenseArrayBaseAttr::value_begin_impl(OverloadToken<float>) const {
   return cast<DenseF32ArrayAttr>().asArrayRef().begin();
 }
 const double *
 DenseArrayBaseAttr::value_begin_impl(OverloadToken<double>) const {
   return cast<DenseF64ArrayAttr>().asArrayRef().begin();
 }
 
case DenseArrayBaseAttr::EltType::I16:
     this->cast<DenseI16ArrayAttr>().printWithoutBraces(os);
     return;
   case DenseArrayBaseAttr::EltType::I32:
     this->cast<DenseI32ArrayAttr>().printWithoutBraces(os);
     return;
   case DenseArrayBaseAttr::EltType::I64:
     this->cast<DenseI64ArrayAttr>().printWithoutBraces(os);
     return;
+  case DenseArrayBaseAttr::EltType::UI8:
+    this->cast<DenseUI8ArrayAttr>().printWithoutBraces(os);
+    return;
+  case DenseArrayBaseAttr::EltType::UI16:
+    this->cast<DenseUI16ArrayAttr>().printWithoutBraces(os);
+    return;
+  case DenseArrayBaseAttr::EltType::UI32:
+    this->cast<DenseUI32ArrayAttr>().printWithoutBraces(os);
+    return;
+  case DenseArrayBaseAttr::EltType::UI64:
+    this->cast<DenseUI64ArrayAttr>().printWithoutBraces(os);
+    return;
   case DenseArrayBaseAttr::EltType::F32:
     this->cast<DenseF32ArrayAttr>().printWithoutBraces(os);
     return;
   case DenseArrayBaseAttr::EltType::F64:
     this->cast<DenseF64ArrayAttr>().printWithoutBraces(os);
     return;
   }
   llvm_unreachable("<unknown DenseArrayBaseAttr>");

 
 /// Specialization for int8_t for forcing printing as number instead of chars.
 template <>
 void DenseArrayAttr<int8_t>::printWithoutBraces(raw_ostream &os) const {
   ArrayRef<int8_t> values{*this};
   llvm::interleaveComma(values, os, [&](int64_t v) { os << v; });
 }
 
+/// Specialization for uint8_t for forcing printing as number instead of chars.
+template <>
+void DenseArrayAttr<uint8_t>::printWithoutBraces(raw_ostream &os) const {
+  ArrayRef<uint8_t> values{*this};
+  llvm::interleaveComma(values, os, [&](int64_t v) { os << v; });
+}
+
 template <typename T>
 void DenseArrayAttr<T>::print(raw_ostream &os) const {
   os << "[";
   printWithoutBraces(os);
   os << "]";
 }
 
 /// Parse a single element: generic template for int types, specialized for

 
 template <>
 ParseResult parseDenseArrayAttrElt<double>(AsmParser &parser, double &value) {
   return parser.parseFloat(value);
 }
 
 /// Parse a DenseArrayAttr without the braces: `1, 2, 3`
 template <typename T>
-Attribute DenseArrayAttr<T>::parseWithoutBraces(AsmParser &parser,
-                                                Type odsType) {
+Attribute DenseArrayAttr<T>::parseWithoutBraces(AsmParser &parser, Type type) {
   SmallVector<T> data;
   if (failed(parser.parseCommaSeparatedList([&]() {
         T value;
         if (parseDenseArrayAttrElt(parser, value))
           return failure();
         data.push_back(value);
         return success();
       })))
     return {};
   return get(parser.getContext(), data);
 }
 
 /// Parse a DenseArrayAttr: `[ 1, 2, 3 ]`
 template <typename T>
-Attribute DenseArrayAttr<T>::parse(AsmParser &parser, Type odsType) {
+Attribute DenseArrayAttr<T>::parse(AsmParser &parser, Type type) {
   if (parser.parseLSquare())
     return {};
   // Handle empty list case.
   if (succeeded(parser.parseOptionalRSquare()))
     return get(parser.getContext(), {});
-  Attribute result = parseWithoutBraces(parser, odsType);
+  Attribute result = parseWithoutBraces(parser, type);
   if (parser.parseRSquare())
     return {};
   return result;
 }
 
 /// Conversion from DenseArrayAttr<T> to ArrayRef<T>.
 template <typename T>
 DenseArrayAttr<T>::operator ArrayRef<T>() const {

 struct denseArrayAttrEltTypeBuilder<int64_t> {
   constexpr static auto eltType = DenseArrayBaseAttr::EltType::I64;
   static ShapedType getShapedType(MLIRContext *context,
                                   ArrayRef<int64_t> shape) {
     return RankedTensorType::get(shape, IntegerType::get(context, 64));
   }
 };
 template <>
+struct denseArrayAttrEltTypeBuilder<uint8_t> {
+  constexpr static auto eltType = DenseArrayBaseAttr::EltType::UI8;
+  static ShapedType getShapedType(MLIRContext *context,
+                                  ArrayRef<int64_t> shape) {
+    return RankedTensorType::get(
+        shape, IntegerType::get(context, 8,
+                                IntegerType::SignednessSemantics::Unsigned));
+  }
+};
+template <>
+struct denseArrayAttrEltTypeBuilder<uint16_t> {
+  constexpr static auto eltType = DenseArrayBaseAttr::EltType::UI16;
+  static ShapedType getShapedType(MLIRContext *context,
+                                  ArrayRef<int64_t> shape) {
+    return RankedTensorType::get(
+        shape, IntegerType::get(context, 16,
+                                IntegerType::SignednessSemantics::Unsigned));
+  }
+};
+template <>
+struct denseArrayAttrEltTypeBuilder<uint32_t> {
+  constexpr static auto eltType = DenseArrayBaseAttr::EltType::UI32;
+  static ShapedType getShapedType(MLIRContext *context,
+                                  ArrayRef<int64_t> shape) {
+    return RankedTensorType::get(
+        shape, IntegerType::get(context, 32,
+                                IntegerType::SignednessSemantics::Unsigned));
+  }
+};
+template <>
+struct denseArrayAttrEltTypeBuilder<uint64_t> {
+  constexpr static auto eltType = DenseArrayBaseAttr::EltType::UI64;
+  static ShapedType getShapedType(MLIRContext *context,
+                                  ArrayRef<int64_t> shape) {
+    return RankedTensorType::get(
+        shape, IntegerType::get(context, 64,
+                                IntegerType::SignednessSemantics::Unsigned));
+  }
+};
+template <>
 struct denseArrayAttrEltTypeBuilder<float> {
   constexpr static auto eltType = DenseArrayBaseAttr::EltType::F32;
   static ShapedType getShapedType(MLIRContext *context,
                                   ArrayRef<int64_t> shape) {
     return RankedTensorType::get(shape, Float32Type::get(context));
   }
 };
 template <>

 namespace mlir {
 namespace detail {
 // Explicit instantiation for all the supported DenseArrayAttr.
 template class DenseArrayAttr<bool>;
 template class DenseArrayAttr<int8_t>;
 template class DenseArrayAttr<int16_t>;
 template class DenseArrayAttr<int32_t>;
 template class DenseArrayAttr<int64_t>;
+template class DenseArrayAttr<uint8_t>;
+template class DenseArrayAttr<uint16_t>;
+template class DenseArrayAttr<uint32_t>;
+template class DenseArrayAttr<uint64_t>;
 template class DenseArrayAttr<float>;
 template class DenseArrayAttr<double>;
 } // namespace detail
 } // namespace mlir
 
 //===----------------------------------------------------------------------===//
 // DenseElementsAttr
 //===----------------------------------------------------------------------===//

mlir/test/IR/elements-attr-interface.mlir

 // expected-error@below {{Test iterating `int8_t`: 10, 11, -12, 13, 14}}
 arith.constant [:i8 10, 11, -12, 13, 14]
 // expected-error@below {{Test iterating `int16_t`: 10, 11, -12, 13, 14}}
 arith.constant [:i16 10, 11, -12, 13, 14]
 // expected-error@below {{Test iterating `int32_t`: 10, 11, -12, 13, 14}}
 arith.constant [:i32 10, 11, -12, 13, 14]
 // expected-error@below {{Test iterating `int64_t`: 10, 11, -12, 13, 14}}
 arith.constant [:i64 10, 11, -12, 13, 14]
+// expected-error@below {{Test iterating `uint8_t`: 10, 11, 244, 13, 14}}
+arith.constant [:ui8 10, 11, -12, 13, 14]
+// expected-error@below {{Test iterating `uint16_t`: 10, 11, 65524, 13, 14}}
+arith.constant [:ui16 10, 11, -12, 13, 14]
+// expected-error@below {{Test iterating `uint32_t`: 10, 11, 4294967284, 13, 14}}
+arith.constant [:ui32 10, 11, -12, 13, 14]
+// expected-error@below {{Test iterating `uint64_t`: 10, 11, 18446744073709551604, 13, 14}}
+arith.constant [:ui64 10, 11, -12, 13, 14]
 // expected-error@below {{Test iterating `float`: 10.00, 11.00, -12.00, 13.00, 14.00}}
 arith.constant [:f32 10., 11., -12., 13., 14.]
 // expected-error@below {{Test iterating `double`: 10.00, 11.00, -12.00, 13.00, 14.00}}
 arith.constant [:f64 10., 11., -12., 13., 14.]
 
 // Check that we handle an external constant parsed from the config.
 // expected-error@below {{Test iterating `int64_t`: unable to iterate type}}
 // expected-error@below {{Test iterating `uint64_t`: 1, 2, 3}}

mlir/test/lib/IR/TestBuiltinAttributeInterfaces.cpp

getOperation().walk([&](Operation *op) {
             testElementsAttrIteration<int16_t>(op, elementsAttr, "int16_t");
             break;
           case DenseArrayBaseAttr::EltType::I32:
             testElementsAttrIteration<int32_t>(op, elementsAttr, "int32_t");
             break;
           case DenseArrayBaseAttr::EltType::I64:
             testElementsAttrIteration<int64_t>(op, elementsAttr, "int64_t");
             break;
+          case DenseArrayBaseAttr::EltType::UI8:
+            testElementsAttrIteration<uint8_t>(op, elementsAttr, "uint8_t");
+            break;
+          case DenseArrayBaseAttr::EltType::UI16:
+            testElementsAttrIteration<uint16_t>(op, elementsAttr, "uint16_t");
+            break;
+          case DenseArrayBaseAttr::EltType::UI32:
+            testElementsAttrIteration<uint32_t>(op, elementsAttr, "uint32_t");
+            break;
+          case DenseArrayBaseAttr::EltType::UI64:
+            testElementsAttrIteration<uint64_t>(op, elementsAttr, "uint64_t");
+            break;
           case DenseArrayBaseAttr::EltType::F32:
             testElementsAttrIteration<float>(op, elementsAttr, "float");
             break;
           case DenseArrayBaseAttr::EltType::F64:
             testElementsAttrIteration<double>(op, elementsAttr, "double");
             break;
           }
           continue;