[Mlir-commits] [mlir] [mlir][OpenMP] inscan reduction modifier and scan op mlir support (PR #114737)

[Sergio Afonso]

================
@@ -517,14 +523,25 @@ static ParseResult parseClauseWithRegionArgs(
     SmallVectorImpl<OpAsmParser::UnresolvedOperand> &operands,
     SmallVectorImpl<Type> &types,
     SmallVectorImpl<OpAsmParser::Argument> &regionPrivateArgs,
-    ArrayAttr *symbols = nullptr, DenseBoolArrayAttr *byref = nullptr) {
+    ArrayAttr *symbols = nullptr, DenseBoolArrayAttr *byref = nullptr,
+    ReductionModifierAttr *reductionMod = nullptr) {
   SmallVector<SymbolRefAttr> symbolVec;
   SmallVector<bool> isByRefVec;
   unsigned regionArgOffset = regionPrivateArgs.size();
 
   if (parser.parseLParen())
     return failure();
 
+  StringRef enumStr;
+  if (succeeded(parser.parseOptionalKeyword("Id"))) {
+    if (parser.parseColon() || parser.parseKeyword(&enumStr) ||
+        parser.parseComma())
+      return failure();
+    std::optional<ReductionModifier> enumValue =
+        symbolizeReductionModifier(enumStr);
+    *reductionMod = ReductionModifierAttr::get(parser.getContext(), *enumValue);
+  }
----------------
skatrak wrote:

For consistency, I believe we should optionally parse the keyword directly:
```mlir
omp.parallel reduction(inscan, byref @add_f32 %0 -> %prv.0, @add_f32 %1 -> %prv.1 : !llvm.ptr, !llvm.ptr) { ... }
omp.parallel reduction(byref @add_f32 %0 -> %prv.0, @add_f32 %1 -> %prv.1 : !llvm.ptr, !llvm.ptr) { ... }
```
I guess here what we should be careful of is not parsing a potential "byref" attached to the first reduction variable as the modifier. If that's too problematic to implement, I guess `reduction(mod: inscan, ...)` would work.

Also, only try parsing it if `reductionMod != nullptr`, since that allows us to use this same function for clauses that don't accept this modifier, like it's already done for `byref` and `symbols`. With the current implementation, doing e.g. `private(Id:InScan, @privatizer %x -> %x.priv : !llvm.ptr)` likely crashes the compiler due to not checking that.

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