[flang-commits] [flang] [mlir] [MLIR][OpenMP] Automate operand structure definition (PR #99508)

Sergio Afonso via flang-commits flang-commits at lists.llvm.org
Mon Aug 26 03:37:42 PDT 2024


================
@@ -408,17 +408,26 @@ class ElementsAttrBase<Pred condition, string summary> :
   let storageType = [{ ::mlir::ElementsAttr }];
   let returnType = [{ ::mlir::ElementsAttr }];
   let convertFromStorage = "$_self";
+
+  // The underlying C++ value type of each element.
+  string elementReturnType = ?;
----------------
skatrak wrote:

Thank you for the discussion.

> Since the information has to be specified manually for ODS definitions to be used with OMP, would it be much worse to have a centralized mapping of Attr -> C++ representation in the OMP backend?

I will look into creating such a mapping, though my concern about that approach is that it's unlikely to be very general. That is, I think it might be easy to break it by just using some new attribute definition based on `ElementsAttrBase`. I think the addition of a field to hold the element type would be more resilient in this regard, since it would always be present regardless of levels of inheritance, etc (except if explicitly overridden to prevent it).

> There is a kind of layering inversion here: only the OMP tablegen backend makes use of this information, yet the onus is on the main tablegen definitions to provide this information. No other dialects will realistically ever have to provide these definitions, and it's not clear to them why these are useful if they aren't using OMP.
>
> I don't think there's a way forward for adding this kind of metadata to ODS proper if there is no conceivable use outside of OMP.

I share that concern, but at the same time I feel like what I'm proposing of adding is a property to hold a subset of data that is already defined by `DenseArrayAttrBase` (the `cppType` template argument). It's adding some flexibility to query that information that initially would only be used by the OpenMP dialect but which is not really specific to it. We don't have any uses for it on other dialects at this point, but I think there is no reason why it couldn't have other use cases later on.

Also, apart from subclasses/definitions based directly on `ElementsAttrBase` (of which I can currently see none outside of CommonAttrConstraints.td), no other attribute types defined in a dialect would actually have to initialize that new property, as this is done for all direct subclasses/definitions in this PR. So, in that sense, it would be a transparent addition.

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


More information about the flang-commits mailing list