[Mlir-commits] [mlir] [mlir] `ResourceAttrInterface` to abstract AsmResourceBlob from resource handle. (PR #101780)

Pavel Prokofyev llvmlistbot at llvm.org
Fri Aug 9 09:09:12 PDT 2024


integralpro wrote:

> > DenseElementsAttr has the following two-
> > ```
> >   /// Return the raw storage data held by this attribute. Users should generally
> >   /// not use this directly, as the internal storage format is not always in the
> >   /// form the user might expect.
> >   ArrayRef<char> getRawData() const;
> > 
> >   /// Return the raw StringRef data held by this attribute.
> >   ArrayRef<StringRef> getRawStringData() const;
> > ```
> > 
> > 
> >     
> >       
> >     
> > 
> >       
> >     
> > 
> >     
> >   
> > Another way is to have them in ElementsAttr interface, then DenseResourceElementsAttr and similar may provide implementation specific to the `resource`.
> 
> I think that probably makes sense, and I believe it's what River is suggesting.
> 
> > One thing that I'm worrying that it will open up for more uses of structured attributes (like DenseIntOrFPElementsAttr) with opaque storage to be accessed by their `getRawData()` in addition to uses DenseResourceElementsAttr where storage is explicit.
> 
> Agreed - but it's already noted that this is a 'use-at-your-own-risk' API so most users shouldn't be using it. DenseIntOrFPElementsAttr could use an empty implementation, or return an error, or something else.

nice! Thank you for the feedback! I'll make a change that follows `ElementsAttrInterface::getRawData()` path.

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


More information about the Mlir-commits mailing list