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

[Sergio Afonso via llvm-branch-commits]

================
@@ -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:

> I'm wary about making this kind of change in a widely shared file. Maybe we could just handle this in OmpOpGen.cpp? Specifically, infer this information in there based on the type of the attribute?

Yes, this is something I tried to avoid as well. The problem is that the only existing attribute we could potentially use to get the element type information is the `returnType` inherited from `Attr`. We could potentially remove the "::llvm::ArrayRef<>" part of that string in the case of `DenseArrayAttrBase` and derived types, which doesn't seem like a very clean solution but it would work (as long as these subclasses/definitions don't override that property). For other subclasses of `ElementsAttrBase` we would have to accept having to use array-style attributes (e.g. `::mlir::DenseIntElementsAttr`) instead of lists of elements.

I'd like to avoid hardcoding as many type names as possible in the new tablegen backend, since people could just create new general or OpenMP-specific attribute types and then it would have to be updated. I think it makes sense to specialize it for as few and as generic cases as we can get away with and just make sure they already contain the information we need. In this case, we're just missing the element type and rank of array attributes, which seems like something that could be of general use eventually.

Having said that, this is just the approach that works that made the most sense to me, but I'm very much interested in discussing potentially better alternatives.

> This may need wider support, specifically we may need to generate an accessor function in .h.inc/.cpp.inc.

Good point, I'll delay making this change until we decide whether we want to keep these new properties or not.

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