[Mlir-commits] [mlir] [mlir] Translating task_reduction clause for pass-by-value vars to LLVMIR (PR #125218)

[Sergio Afonso]

================
@@ -1388,6 +1388,14 @@ class OpenMP_TaskReductionClauseSkip<
     OptionalAttr<SymbolRefArrayAttr>:$task_reduction_syms
   );
 
+  let extraClassDeclaration = [{
+    /// Returns the number of reduction variables.
+    unsigned getNumReductionVars() { return getTaskReductionVars().size(); }
+
+    /// Returns the reduction symbols
+    auto getReductionSyms() { return getTaskReductionSyms(); }
----------------
skatrak wrote:

Is there a reason using these method names is required? They seem intended to exploit duck typing in order to treat `task_reduction` as if it was a `reduction` clause later, but that is a different clause.

I think we should avoid doing these kinds of things, as they would cause conflicts when an operation contains both clauses (not the case here, but it might be copied in the future) or generally make code somewhat misleading. In this case, specifically, we could rename `getNumReductionVars()` to `getNumTaskReductionVars()` and remove `getReductionSyms()`, so we use `getTaskReductionSyms()` instead.

I'm not against making an exception if there's a reason for it, as long as it's properly documented, but I think it's not something we generally would want to do otherwise.

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