[Mlir-commits] [mlir] [mlir][OpenMP] inscan reduction modifier and scan op mlir support (PR #114737)
Anchu Rajendran S
llvmlistbot at llvm.org
Mon Dec 9 11:29:46 PST 2024
https://github.com/anchuraj updated https://github.com/llvm/llvm-project/pull/114737
>From d8c450537b4c7dbaeae1059ad407831a9fb77363 Mon Sep 17 00:00:00 2001
From: Anchu Rajendran <asudhaku at amd.com>
Date: Sat, 2 Nov 2024 21:57:20 -0500
Subject: [PATCH 1/3] Changes for inscan reduction and scan op
---
.../mlir/Dialect/OpenMP/OpenMPClauses.td | 71 ++++++
.../mlir/Dialect/OpenMP/OpenMPEnums.td | 21 ++
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 40 +++-
.../Conversion/SCFToOpenMP/SCFToOpenMP.cpp | 1 +
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 224 +++++++++++++-----
mlir/test/Dialect/OpenMP/invalid.mlir | 79 ++++++
mlir/test/Dialect/OpenMP/ops.mlir | 23 ++
7 files changed, 384 insertions(+), 75 deletions(-)
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td
index 855deab94b2f16..826c5bccaf434c 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td
@@ -308,6 +308,40 @@ class OpenMP_DoacrossClauseSkip<
def OpenMP_DoacrossClause : OpenMP_DoacrossClauseSkip<>;
+//===----------------------------------------------------------------------===//
+// V5.2: [5.4.7] `exclusive` clause
+//===----------------------------------------------------------------------===//
+
+class OpenMP_ExclusiveClauseSkip<
+ bit traits = false, bit arguments = false, bit assemblyFormat = false,
+ bit description = false, bit extraClassDeclaration = false
+ > : OpenMP_Clause<traits, arguments, assemblyFormat, description,
+ extraClassDeclaration> {
+ let arguments = (ins
+ Variadic<AnyType>:$exclusive_vars
+ );
+
+ let optAssemblyFormat = [{
+ `exclusive` `(` $exclusive_vars `:` type($exclusive_vars) `)`
+ }];
+
+ let extraClassDeclaration = [{
+ bool hasExclusiveVars() {
+ return getExclusiveVars().size()>0;
+ }
+ }];
+
+ let description = [{
+ The exclusive clause is used on a separating directive that separates a
+ structured block into two structured block sequences. If it
+ is specified, the input phase excludes the preceding structured block
+ sequence and instead includes the following structured block sequence,
+ while the scan phase includes the preceding structured block sequence.
+ }];
+}
+
+def OpenMP_ExclusiveClause : OpenMP_ExclusiveClauseSkip<>;
+
//===----------------------------------------------------------------------===//
// V5.2: [10.5.1] `filter` clause
//===----------------------------------------------------------------------===//
@@ -418,6 +452,40 @@ class OpenMP_HasDeviceAddrClauseSkip<
def OpenMP_HasDeviceAddrClause : OpenMP_HasDeviceAddrClauseSkip<>;
+//===----------------------------------------------------------------------===//
+// V5.2: [5.4.7] `inclusive` clause
+//===----------------------------------------------------------------------===//
+
+class OpenMP_InclusiveClauseSkip<
+ bit traits = false, bit arguments = false, bit assemblyFormat = false,
+ bit description = false, bit extraClassDeclaration = false
+ > : OpenMP_Clause<traits, arguments, assemblyFormat, description,
+ extraClassDeclaration> {
+ let arguments = (ins
+ Variadic<AnyType>:$inclusive_vars
+ );
+
+ let optAssemblyFormat = [{
+ `inclusive` `(` $inclusive_vars `:` type($inclusive_vars) `)`
+ }];
+
+ let extraClassDeclaration = [{
+ bool hasInclusiveVars() {
+ return getInclusiveVars().size()>0;
+ }
+ }];
+
+ let description = [{
+ The inclusive clause is used on a separating directive that separates a
+ structured block into two structured block sequences. If it is specified,
+ the input phase includes the preceding structured block sequence and the
+ scan phase includes the following structured block sequence.
+ }];
+}
+
+def OpenMP_InclusiveClause : OpenMP_InclusiveClauseSkip<>;
+
+
//===----------------------------------------------------------------------===//
// V5.2: [15.1.2] `hint` clause
//===----------------------------------------------------------------------===//
@@ -480,6 +548,7 @@ class OpenMP_InReductionClauseSkip<
];
let arguments = (ins
+ OptionalAttr<ReductionModifierAttr>:$in_reduction_mod,
Variadic<OpenMP_PointerLikeType>:$in_reduction_vars,
OptionalAttr<DenseBoolArrayAttr>:$in_reduction_byref,
OptionalAttr<SymbolRefArrayAttr>:$in_reduction_syms
@@ -1008,6 +1077,7 @@ class OpenMP_ReductionClauseSkip<
];
let arguments = (ins
+ OptionalAttr<ReductionModifierAttr>:$reduction_mod,
Variadic<OpenMP_PointerLikeType>:$reduction_vars,
OptionalAttr<DenseBoolArrayAttr>:$reduction_byref,
OptionalAttr<SymbolRefArrayAttr>:$reduction_syms
@@ -1138,6 +1208,7 @@ class OpenMP_TaskReductionClauseSkip<
];
let arguments = (ins
+ OptionalAttr<ReductionModifierAttr>:$task_reduction_mod,
Variadic<OpenMP_PointerLikeType>:$task_reduction_vars,
OptionalAttr<DenseBoolArrayAttr>:$task_reduction_byref,
OptionalAttr<SymbolRefArrayAttr>:$task_reduction_syms
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
index b1a9e3330522b2..23086556bbb2f5 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
@@ -178,6 +178,27 @@ def OrderModifier
def OrderModifierAttr : EnumAttr<OpenMP_Dialect, OrderModifier,
"order_mod">;
+//===----------------------------------------------------------------------===//
+// reduction_modifier enum.
+//===----------------------------------------------------------------------===//
+
+def ReductionModifierInScan : I32EnumAttrCase<"InScan", 0>;
+def ReductionModifierTask : I32EnumAttrCase<"Task", 1>;
+def ReductionModifierDefault : I32EnumAttrCase<"Default", 2>;
+
+def ReductionModifier : OpenMP_I32EnumAttr<
+ "ReductionModifier",
+ "reduction modifier", [
+ ReductionModifierInScan,
+ ReductionModifierTask,
+ ReductionModifierDefault
+ ]>;
+
+def ReductionModifierAttr : OpenMP_EnumAttr<ReductionModifier,
+ "reduction_modifier"> {
+ let assemblyFormat = "`(` $value `)`";
+}
+
//===----------------------------------------------------------------------===//
// sched_mod enum.
//===----------------------------------------------------------------------===//
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 156e6eb371b85d..f37f26c50eff58 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -170,7 +170,7 @@ def ParallelOp : OpenMP_Op<"parallel", traits = [
let assemblyFormat = clausesAssemblyFormat # [{
custom<PrivateReductionRegion>($region, $private_vars, type($private_vars),
- $private_syms, $reduction_vars, type($reduction_vars), $reduction_byref,
+ $private_syms, $reduction_mod, $reduction_vars, type($reduction_vars), $reduction_byref,
$reduction_syms) attr-dict
}];
@@ -215,7 +215,7 @@ def TeamsOp : OpenMP_Op<"teams", traits = [
let assemblyFormat = clausesAssemblyFormat # [{
custom<PrivateReductionRegion>($region, $private_vars, type($private_vars),
- $private_syms, $reduction_vars, type($reduction_vars), $reduction_byref,
+ $private_syms, $reduction_mod, $reduction_vars, type($reduction_vars), $reduction_byref,
$reduction_syms) attr-dict
}];
@@ -274,7 +274,7 @@ def SectionsOp : OpenMP_Op<"sections", traits = [
let assemblyFormat = clausesAssemblyFormat # [{
custom<PrivateReductionRegion>($region, $private_vars, type($private_vars),
- $private_syms, $reduction_vars, type($reduction_vars), $reduction_byref,
+ $private_syms, $reduction_mod, $reduction_vars, type($reduction_vars), $reduction_byref,
$reduction_syms) attr-dict
}];
@@ -513,7 +513,7 @@ def WsloopOp : OpenMP_Op<"wsloop", traits = [
let assemblyFormat = clausesAssemblyFormat # [{
custom<PrivateReductionRegion>($region, $private_vars, type($private_vars),
- $private_syms, $reduction_vars, type($reduction_vars), $reduction_byref,
+ $private_syms, $reduction_mod, $reduction_vars, type($reduction_vars), $reduction_byref,
$reduction_syms) attr-dict
}];
@@ -567,7 +567,7 @@ def SimdOp : OpenMP_Op<"simd", traits = [
let assemblyFormat = clausesAssemblyFormat # [{
custom<PrivateReductionRegion>($region, $private_vars, type($private_vars),
- $private_syms, $reduction_vars, type($reduction_vars), $reduction_byref,
+ $private_syms, $reduction_mod, $reduction_vars, type($reduction_vars), $reduction_byref,
$reduction_syms) attr-dict
}];
@@ -691,7 +691,7 @@ def TaskOp : OpenMP_Op<"task", traits = [
let assemblyFormat = clausesAssemblyFormat # [{
custom<InReductionPrivateRegion>(
- $region, $in_reduction_vars, type($in_reduction_vars),
+ $region, $in_reduction_mod, $in_reduction_vars, type($in_reduction_vars),
$in_reduction_byref, $in_reduction_syms, $private_vars,
type($private_vars), $private_syms) attr-dict
}];
@@ -769,9 +769,9 @@ def TaskloopOp : OpenMP_Op<"taskloop", traits = [
let assemblyFormat = clausesAssemblyFormat # [{
custom<InReductionPrivateReductionRegion>(
- $region, $in_reduction_vars, type($in_reduction_vars),
+ $region, $in_reduction_mod, $in_reduction_vars, type($in_reduction_vars),
$in_reduction_byref, $in_reduction_syms, $private_vars,
- type($private_vars), $private_syms, $reduction_vars,
+ type($private_vars), $private_syms, $reduction_mod, $reduction_vars,
type($reduction_vars), $reduction_byref, $reduction_syms) attr-dict
}];
@@ -816,7 +816,7 @@ def TaskgroupOp : OpenMP_Op<"taskgroup", traits = [
let assemblyFormat = clausesAssemblyFormat # [{
custom<TaskReductionRegion>(
- $region, $task_reduction_vars, type($task_reduction_vars),
+ $region, $task_reduction_mod, $task_reduction_vars, type($task_reduction_vars),
$task_reduction_byref, $task_reduction_syms) attr-dict
}];
@@ -1237,7 +1237,7 @@ def TargetOp : OpenMP_Op<"target", traits = [
let assemblyFormat = clausesAssemblyFormat # [{
custom<InReductionMapPrivateRegion>(
- $region, $in_reduction_vars, type($in_reduction_vars),
+ $region, $in_reduction_mod, $in_reduction_vars, type($in_reduction_vars),
$in_reduction_byref, $in_reduction_syms, $map_vars, type($map_vars),
$private_vars, type($private_vars), $private_syms) attr-dict
}];
@@ -1652,6 +1652,26 @@ def CancellationPointOp : OpenMP_Op<"cancellation_point", clauses = [
let hasVerifier = 1;
}
+def ScanOp : OpenMP_Op<"scan", [
+ AttrSizedOperandSegments, RecipeInterface, IsolatedFromAbove
+ ], clauses = [
+ OpenMP_InclusiveClause, OpenMP_ExclusiveClause]> {
+ let summary = "scan directive";
+ let description = [{
+ The scan directive allows to specify scan reduction. Scan directive
+ should be enclosed with in a parent directive along with which , a
+ reduction clause with `InScan` modifier must be specified. Scan directive
+ allows to separate code blocks to input phase and scan phase in the region
+ enclosed by the parent.
+ }] # clausesDescription;
+
+ let builders = [
+ OpBuilder<(ins CArg<"const ScanOperands &">:$clauses)>
+ ];
+
+ let hasVerifier = 1;
+}
+
//===----------------------------------------------------------------------===//
// 2.19.5.7 declare reduction Directive
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Conversion/SCFToOpenMP/SCFToOpenMP.cpp b/mlir/lib/Conversion/SCFToOpenMP/SCFToOpenMP.cpp
index aa241b91d758ca..233739e1d6d917 100644
--- a/mlir/lib/Conversion/SCFToOpenMP/SCFToOpenMP.cpp
+++ b/mlir/lib/Conversion/SCFToOpenMP/SCFToOpenMP.cpp
@@ -451,6 +451,7 @@ struct ParallelOpLowering : public OpRewritePattern<scf::ParallelOp> {
/* private_vars = */ ValueRange(),
/* private_syms = */ nullptr,
/* proc_bind_kind = */ omp::ClauseProcBindKindAttr{},
+ /* reduction_mod = */ nullptr,
/* reduction_vars = */ llvm::SmallVector<Value>{},
/* reduction_byref = */ DenseBoolArrayAttr{},
/* reduction_syms = */ ArrayAttr{});
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 94e71e089d4b18..c46724463b593f 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -491,16 +491,22 @@ struct PrivateParseArgs {
SmallVectorImpl<Type> &types, ArrayAttr &syms)
: vars(vars), types(types), syms(syms) {}
};
+
struct ReductionParseArgs {
+ ReductionModifierAttr &reductionMod;
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &vars;
SmallVectorImpl<Type> &types;
DenseBoolArrayAttr &byref;
ArrayAttr &syms;
- ReductionParseArgs(SmallVectorImpl<OpAsmParser::UnresolvedOperand> &vars,
+ ReductionParseArgs(ReductionModifierAttr &redMod,
+ SmallVectorImpl<OpAsmParser::UnresolvedOperand> &vars,
SmallVectorImpl<Type> &types, DenseBoolArrayAttr &byref,
ArrayAttr &syms)
- : vars(vars), types(types), byref(byref), syms(syms) {}
+ : reductionMod(redMod), vars(vars), types(types), byref(byref),
+ syms(syms) {}
};
+
+// specifies the arguments needs for `reduction` clause
struct AllRegionParseArgs {
std::optional<ReductionParseArgs> inReductionArgs;
std::optional<MapParseArgs> mapArgs;
@@ -517,7 +523,8 @@ static ParseResult parseClauseWithRegionArgs(
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &operands,
SmallVectorImpl<Type> &types,
SmallVectorImpl<OpAsmParser::Argument> ®ionPrivateArgs,
- 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();
@@ -525,6 +532,16 @@ static ParseResult parseClauseWithRegionArgs(
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);
+ }
+
if (parser.parseCommaSeparatedList([&]() {
if (byref)
isByRefVec.push_back(
@@ -615,15 +632,14 @@ static ParseResult parseBlockArgClause(
if (succeeded(parser.parseOptionalKeyword(keyword))) {
if (!reductionArgs)
return failure();
-
if (failed(parseClauseWithRegionArgs(
parser, reductionArgs->vars, reductionArgs->types, entryBlockArgs,
- &reductionArgs->syms, &reductionArgs->byref)))
+ &reductionArgs->syms, &reductionArgs->byref,
+ &(reductionArgs->reductionMod))))
return failure();
}
return success();
}
-
static ParseResult parseBlockArgRegion(OpAsmParser &parser, Region ®ion,
AllRegionParseArgs args) {
llvm::SmallVector<OpAsmParser::Argument> entryBlockArgs;
@@ -667,7 +683,7 @@ static ParseResult parseBlockArgRegion(OpAsmParser &parser, Region ®ion,
}
static ParseResult parseInReductionMapPrivateRegion(
- OpAsmParser &parser, Region ®ion,
+ OpAsmParser &parser, Region ®ion, ReductionModifierAttr &inReductionMod,
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &inReductionVars,
SmallVectorImpl<Type> &inReductionTypes,
DenseBoolArrayAttr &inReductionByref, ArrayAttr &inReductionSyms,
@@ -676,43 +692,47 @@ static ParseResult parseInReductionMapPrivateRegion(
llvm::SmallVectorImpl<OpAsmParser::UnresolvedOperand> &privateVars,
llvm::SmallVectorImpl<Type> &privateTypes, ArrayAttr &privateSyms) {
AllRegionParseArgs args;
- args.inReductionArgs.emplace(inReductionVars, inReductionTypes,
- inReductionByref, inReductionSyms);
+ args.inReductionArgs.emplace(inReductionMod, inReductionVars,
+ inReductionTypes, inReductionByref,
+ inReductionSyms);
args.mapArgs.emplace(mapVars, mapTypes);
args.privateArgs.emplace(privateVars, privateTypes, privateSyms);
return parseBlockArgRegion(parser, region, args);
}
static ParseResult parseInReductionPrivateRegion(
- OpAsmParser &parser, Region ®ion,
+ OpAsmParser &parser, Region ®ion, ReductionModifierAttr &inReductionMod,
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &inReductionVars,
SmallVectorImpl<Type> &inReductionTypes,
DenseBoolArrayAttr &inReductionByref, ArrayAttr &inReductionSyms,
llvm::SmallVectorImpl<OpAsmParser::UnresolvedOperand> &privateVars,
llvm::SmallVectorImpl<Type> &privateTypes, ArrayAttr &privateSyms) {
AllRegionParseArgs args;
- args.inReductionArgs.emplace(inReductionVars, inReductionTypes,
- inReductionByref, inReductionSyms);
+ args.inReductionArgs.emplace(inReductionMod, inReductionVars,
+ inReductionTypes, inReductionByref,
+ inReductionSyms);
args.privateArgs.emplace(privateVars, privateTypes, privateSyms);
return parseBlockArgRegion(parser, region, args);
}
static ParseResult parseInReductionPrivateReductionRegion(
- OpAsmParser &parser, Region ®ion,
+ OpAsmParser &parser, Region ®ion, ReductionModifierAttr &inReductionMod,
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &inReductionVars,
SmallVectorImpl<Type> &inReductionTypes,
DenseBoolArrayAttr &inReductionByref, ArrayAttr &inReductionSyms,
llvm::SmallVectorImpl<OpAsmParser::UnresolvedOperand> &privateVars,
llvm::SmallVectorImpl<Type> &privateTypes, ArrayAttr &privateSyms,
+ ReductionModifierAttr &reductionMod,
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &reductionVars,
SmallVectorImpl<Type> &reductionTypes, DenseBoolArrayAttr &reductionByref,
ArrayAttr &reductionSyms) {
AllRegionParseArgs args;
- args.inReductionArgs.emplace(inReductionVars, inReductionTypes,
- inReductionByref, inReductionSyms);
+ args.inReductionArgs.emplace(inReductionMod, inReductionVars,
+ inReductionTypes, inReductionByref,
+ inReductionSyms);
args.privateArgs.emplace(privateVars, privateTypes, privateSyms);
- args.reductionArgs.emplace(reductionVars, reductionTypes, reductionByref,
- reductionSyms);
+ args.reductionArgs.emplace(reductionMod, reductionVars, reductionTypes,
+ reductionByref, reductionSyms);
return parseBlockArgRegion(parser, region, args);
}
@@ -729,24 +749,27 @@ static ParseResult parsePrivateReductionRegion(
OpAsmParser &parser, Region ®ion,
llvm::SmallVectorImpl<OpAsmParser::UnresolvedOperand> &privateVars,
llvm::SmallVectorImpl<Type> &privateTypes, ArrayAttr &privateSyms,
+ ReductionModifierAttr &reductionMod,
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &reductionVars,
SmallVectorImpl<Type> &reductionTypes, DenseBoolArrayAttr &reductionByref,
ArrayAttr &reductionSyms) {
AllRegionParseArgs args;
args.privateArgs.emplace(privateVars, privateTypes, privateSyms);
- args.reductionArgs.emplace(reductionVars, reductionTypes, reductionByref,
- reductionSyms);
+ args.reductionArgs.emplace(reductionMod, reductionVars, reductionTypes,
+ reductionByref, reductionSyms);
return parseBlockArgRegion(parser, region, args);
}
static ParseResult parseTaskReductionRegion(
OpAsmParser &parser, Region ®ion,
+ ReductionModifierAttr &taskReductionMod,
SmallVectorImpl<OpAsmParser::UnresolvedOperand> &taskReductionVars,
SmallVectorImpl<Type> &taskReductionTypes,
DenseBoolArrayAttr &taskReductionByref, ArrayAttr &taskReductionSyms) {
AllRegionParseArgs args;
- args.taskReductionArgs.emplace(taskReductionVars, taskReductionTypes,
- taskReductionByref, taskReductionSyms);
+ args.taskReductionArgs.emplace(taskReductionMod, taskReductionVars,
+ taskReductionTypes, taskReductionByref,
+ taskReductionSyms);
return parseBlockArgRegion(parser, region, args);
}
@@ -780,13 +803,15 @@ struct PrivatePrintArgs {
: vars(vars), types(types), syms(syms) {}
};
struct ReductionPrintArgs {
+ ReductionModifierAttr reductionMod;
ValueRange vars;
TypeRange types;
DenseBoolArrayAttr byref;
ArrayAttr syms;
- ReductionPrintArgs(ValueRange vars, TypeRange types, DenseBoolArrayAttr byref,
- ArrayAttr syms)
- : vars(vars), types(types), byref(byref), syms(syms) {}
+ ReductionPrintArgs(ReductionModifierAttr reductionMod, ValueRange vars,
+ TypeRange types, DenseBoolArrayAttr byref, ArrayAttr syms)
+ : reductionMod(reductionMod), vars(vars), types(types), byref(byref),
+ syms(syms) {}
};
struct AllRegionPrintArgs {
std::optional<ReductionPrintArgs> inReductionArgs;
@@ -799,17 +824,20 @@ struct AllRegionPrintArgs {
};
} // namespace
-static void printClauseWithRegionArgs(OpAsmPrinter &p, MLIRContext *ctx,
- StringRef clauseName,
- ValueRange argsSubrange,
- ValueRange operands, TypeRange types,
- ArrayAttr symbols = nullptr,
- DenseBoolArrayAttr byref = nullptr) {
+static void printClauseWithRegionArgs(
+ OpAsmPrinter &p, MLIRContext *ctx, StringRef clauseName,
+ ValueRange argsSubrange, ValueRange operands, TypeRange types,
+ ArrayAttr symbols = nullptr, DenseBoolArrayAttr byref = nullptr,
+ ReductionModifierAttr reductionMod = nullptr) {
if (argsSubrange.empty())
return;
p << clauseName << "(";
+ if (reductionMod) {
+ p << "Id: " << stringifyReductionModifier(reductionMod.getValue()) << ", ";
+ }
+
if (!symbols) {
llvm::SmallVector<Attribute> values(operands.size(), nullptr);
symbols = ArrayAttr::get(ctx, values);
@@ -859,7 +887,8 @@ printBlockArgClause(OpAsmPrinter &p, MLIRContext *ctx, StringRef clauseName,
if (reductionArgs)
printClauseWithRegionArgs(p, ctx, clauseName, argsSubrange,
reductionArgs->vars, reductionArgs->types,
- reductionArgs->syms, reductionArgs->byref);
+ reductionArgs->syms, reductionArgs->byref,
+ reductionArgs->reductionMod);
}
static void printBlockArgRegion(OpAsmPrinter &p, Operation *op, Region ®ion,
@@ -888,42 +917,49 @@ static void printBlockArgRegion(OpAsmPrinter &p, Operation *op, Region ®ion,
}
static void printInReductionMapPrivateRegion(
- OpAsmPrinter &p, Operation *op, Region ®ion, ValueRange inReductionVars,
+ OpAsmPrinter &p, Operation *op, Region ®ion,
+ ReductionModifierAttr inReductionMod, ValueRange inReductionVars,
TypeRange inReductionTypes, DenseBoolArrayAttr inReductionByref,
ArrayAttr inReductionSyms, ValueRange mapVars, TypeRange mapTypes,
ValueRange privateVars, TypeRange privateTypes, ArrayAttr privateSyms) {
AllRegionPrintArgs args;
- args.inReductionArgs.emplace(inReductionVars, inReductionTypes,
- inReductionByref, inReductionSyms);
+ args.inReductionArgs.emplace(inReductionMod, inReductionVars,
+ inReductionTypes, inReductionByref,
+ inReductionSyms);
args.mapArgs.emplace(mapVars, mapTypes);
args.privateArgs.emplace(privateVars, privateTypes, privateSyms);
printBlockArgRegion(p, op, region, args);
}
static void printInReductionPrivateRegion(
- OpAsmPrinter &p, Operation *op, Region ®ion, ValueRange inReductionVars,
+ OpAsmPrinter &p, Operation *op, Region ®ion,
+ ReductionModifierAttr inReductionMod, ValueRange inReductionVars,
TypeRange inReductionTypes, DenseBoolArrayAttr inReductionByref,
ArrayAttr inReductionSyms, ValueRange privateVars, TypeRange privateTypes,
ArrayAttr privateSyms) {
AllRegionPrintArgs args;
- args.inReductionArgs.emplace(inReductionVars, inReductionTypes,
- inReductionByref, inReductionSyms);
+ args.inReductionArgs.emplace(inReductionMod, inReductionVars,
+ inReductionTypes, inReductionByref,
+ inReductionSyms);
args.privateArgs.emplace(privateVars, privateTypes, privateSyms);
printBlockArgRegion(p, op, region, args);
}
static void printInReductionPrivateReductionRegion(
- OpAsmPrinter &p, Operation *op, Region ®ion, ValueRange inReductionVars,
+ OpAsmPrinter &p, Operation *op, Region ®ion,
+ ReductionModifierAttr inReductionMod, ValueRange inReductionVars,
TypeRange inReductionTypes, DenseBoolArrayAttr inReductionByref,
ArrayAttr inReductionSyms, ValueRange privateVars, TypeRange privateTypes,
- ArrayAttr privateSyms, ValueRange reductionVars, TypeRange reductionTypes,
+ ArrayAttr privateSyms, ReductionModifierAttr reductionMod,
+ ValueRange reductionVars, TypeRange reductionTypes,
DenseBoolArrayAttr reductionByref, ArrayAttr reductionSyms) {
AllRegionPrintArgs args;
- args.inReductionArgs.emplace(inReductionVars, inReductionTypes,
- inReductionByref, inReductionSyms);
+ args.inReductionArgs.emplace(inReductionMod, inReductionVars,
+ inReductionTypes, inReductionByref,
+ inReductionSyms);
args.privateArgs.emplace(privateVars, privateTypes, privateSyms);
- args.reductionArgs.emplace(reductionVars, reductionTypes, reductionByref,
- reductionSyms);
+ args.reductionArgs.emplace(reductionMod, reductionVars, reductionTypes,
+ reductionByref, reductionSyms);
printBlockArgRegion(p, op, region, args);
}
@@ -937,25 +973,28 @@ static void printPrivateRegion(OpAsmPrinter &p, Operation *op, Region ®ion,
static void printPrivateReductionRegion(
OpAsmPrinter &p, Operation *op, Region ®ion, ValueRange privateVars,
- TypeRange privateTypes, ArrayAttr privateSyms, ValueRange reductionVars,
+ TypeRange privateTypes, ArrayAttr privateSyms,
+ ReductionModifierAttr reductionMod, ValueRange reductionVars,
TypeRange reductionTypes, DenseBoolArrayAttr reductionByref,
ArrayAttr reductionSyms) {
AllRegionPrintArgs args;
args.privateArgs.emplace(privateVars, privateTypes, privateSyms);
- args.reductionArgs.emplace(reductionVars, reductionTypes, reductionByref,
- reductionSyms);
+ args.reductionArgs.emplace(reductionMod, reductionVars, reductionTypes,
+ reductionByref, reductionSyms);
printBlockArgRegion(p, op, region, args);
}
static void printTaskReductionRegion(OpAsmPrinter &p, Operation *op,
Region ®ion,
+ ReductionModifierAttr taskReductionMod,
ValueRange taskReductionVars,
TypeRange taskReductionTypes,
DenseBoolArrayAttr taskReductionByref,
ArrayAttr taskReductionSyms) {
AllRegionPrintArgs args;
- args.taskReductionArgs.emplace(taskReductionVars, taskReductionTypes,
- taskReductionByref, taskReductionSyms);
+ args.taskReductionArgs.emplace(taskReductionMod, taskReductionVars,
+ taskReductionTypes, taskReductionByref,
+ taskReductionSyms);
printBlockArgRegion(p, op, region, args);
}
@@ -1653,7 +1692,8 @@ void TargetOp::build(OpBuilder &builder, OperationState &state,
TargetOp::build(builder, state, /*allocate_vars=*/{}, /*allocator_vars=*/{},
makeArrayAttr(ctx, clauses.dependKinds), clauses.dependVars,
clauses.device, clauses.hasDeviceAddrVars, clauses.ifExpr,
- /*in_reduction_vars=*/{}, /*in_reduction_byref=*/nullptr,
+ /*in_reduction_mod=*/nullptr, /*in_reduction_vars=*/{},
+ /*in_reduction_byref=*/nullptr,
/*in_reduction_syms=*/nullptr, clauses.isDevicePtrVars,
clauses.mapVars, clauses.nowait, clauses.privateVars,
makeArrayAttr(ctx, clauses.privateSyms), clauses.threadLimit);
@@ -1676,7 +1716,7 @@ void ParallelOp::build(OpBuilder &builder, OperationState &state,
/*allocator_vars=*/ValueRange(), /*if_expr=*/nullptr,
/*num_threads=*/nullptr, /*private_vars=*/ValueRange(),
/*private_syms=*/nullptr, /*proc_bind_kind=*/nullptr,
- /*reduction_vars=*/ValueRange(),
+ /*reduction_mod =*/nullptr, /*reduction_vars=*/ValueRange(),
/*reduction_byref=*/nullptr, /*reduction_syms=*/nullptr);
state.addAttributes(attributes);
}
@@ -1687,7 +1727,8 @@ void ParallelOp::build(OpBuilder &builder, OperationState &state,
ParallelOp::build(builder, state, clauses.allocateVars, clauses.allocatorVars,
clauses.ifExpr, clauses.numThreads, clauses.privateVars,
makeArrayAttr(ctx, clauses.privateSyms),
- clauses.procBindKind, clauses.reductionVars,
+ clauses.procBindKind, clauses.reductionMod,
+ clauses.reductionVars,
makeDenseBoolArrayAttr(ctx, clauses.reductionByref),
makeArrayAttr(ctx, clauses.reductionSyms));
}
@@ -1786,12 +1827,13 @@ void TeamsOp::build(OpBuilder &builder, OperationState &state,
const TeamsOperands &clauses) {
MLIRContext *ctx = builder.getContext();
// TODO Store clauses in op: privateVars, privateSyms.
- TeamsOp::build(
- builder, state, clauses.allocateVars, clauses.allocatorVars,
- clauses.ifExpr, clauses.numTeamsLower, clauses.numTeamsUpper,
- /*private_vars=*/{}, /*private_syms=*/nullptr, clauses.reductionVars,
- makeDenseBoolArrayAttr(ctx, clauses.reductionByref),
- makeArrayAttr(ctx, clauses.reductionSyms), clauses.threadLimit);
+ TeamsOp::build(builder, state, clauses.allocateVars, clauses.allocatorVars,
+ clauses.ifExpr, clauses.numTeamsLower, clauses.numTeamsUpper,
+ /*private_vars=*/{}, /*private_syms=*/nullptr,
+ clauses.reductionMod, clauses.reductionVars,
+ makeDenseBoolArrayAttr(ctx, clauses.reductionByref),
+ makeArrayAttr(ctx, clauses.reductionSyms),
+ clauses.threadLimit);
}
LogicalResult TeamsOp::verify() {
@@ -1848,7 +1890,8 @@ void SectionsOp::build(OpBuilder &builder, OperationState &state,
// TODO Store clauses in op: privateVars, privateSyms.
SectionsOp::build(builder, state, clauses.allocateVars, clauses.allocatorVars,
clauses.nowait, /*private_vars=*/{},
- /*private_syms=*/nullptr, clauses.reductionVars,
+ /*private_syms=*/nullptr, clauses.reductionMod,
+ clauses.reductionVars,
makeDenseBoolArrayAttr(ctx, clauses.reductionByref),
makeArrayAttr(ctx, clauses.reductionSyms));
}
@@ -1983,7 +2026,7 @@ void WsloopOp::build(OpBuilder &builder, OperationState &state,
/*linear_vars=*/ValueRange(), /*linear_step_vars=*/ValueRange(),
/*nowait=*/false, /*order=*/nullptr, /*order_mod=*/nullptr,
/*ordered=*/nullptr, /*private_vars=*/{}, /*private_syms=*/nullptr,
- /*reduction_vars=*/ValueRange(), /*reduction_byref=*/nullptr,
+ nullptr, /*reduction_vars=*/ValueRange(), /*reduction_byref=*/nullptr,
/*reduction_syms=*/nullptr, /*schedule_kind=*/nullptr,
/*schedule_chunk=*/nullptr, /*schedule_mod=*/nullptr,
/*schedule_simd=*/false);
@@ -2000,7 +2043,7 @@ void WsloopOp::build(OpBuilder &builder, OperationState &state,
/*allocate_vars=*/{}, /*allocator_vars=*/{}, clauses.linearVars,
clauses.linearStepVars, clauses.nowait, clauses.order, clauses.orderMod,
clauses.ordered, /*private_vars=*/{}, /*private_syms=*/nullptr,
- clauses.reductionVars,
+ clauses.reductionMod, clauses.reductionVars,
makeDenseBoolArrayAttr(ctx, clauses.reductionByref),
makeArrayAttr(ctx, clauses.reductionSyms), clauses.scheduleKind,
clauses.scheduleChunk, clauses.scheduleMod, clauses.scheduleSimd);
@@ -2050,7 +2093,7 @@ void SimdOp::build(OpBuilder &builder, OperationState &state,
/*linear_vars=*/{}, /*linear_step_vars=*/{},
clauses.nontemporalVars, clauses.order, clauses.orderMod,
/*private_vars=*/{}, /*private_syms=*/nullptr,
- clauses.reductionVars,
+ clauses.reductionMod, clauses.reductionVars,
makeDenseBoolArrayAttr(ctx, clauses.reductionByref),
makeArrayAttr(ctx, clauses.reductionSyms), clauses.safelen,
clauses.simdlen);
@@ -2231,7 +2274,8 @@ void TaskOp::build(OpBuilder &builder, OperationState &state,
MLIRContext *ctx = builder.getContext();
TaskOp::build(builder, state, clauses.allocateVars, clauses.allocatorVars,
makeArrayAttr(ctx, clauses.dependKinds), clauses.dependVars,
- clauses.final, clauses.ifExpr, clauses.inReductionVars,
+ clauses.final, clauses.ifExpr, clauses.inReductionMod,
+ clauses.inReductionVars,
makeDenseBoolArrayAttr(ctx, clauses.inReductionByref),
makeArrayAttr(ctx, clauses.inReductionSyms), clauses.mergeable,
clauses.priority, /*private_vars=*/clauses.privateVars,
@@ -2257,7 +2301,8 @@ void TaskgroupOp::build(OpBuilder &builder, OperationState &state,
const TaskgroupOperands &clauses) {
MLIRContext *ctx = builder.getContext();
TaskgroupOp::build(builder, state, clauses.allocateVars,
- clauses.allocatorVars, clauses.taskReductionVars,
+ clauses.allocatorVars, clauses.taskReductionMod,
+ clauses.taskReductionVars,
makeDenseBoolArrayAttr(ctx, clauses.taskReductionByref),
makeArrayAttr(ctx, clauses.taskReductionSyms));
}
@@ -2278,11 +2323,12 @@ void TaskloopOp::build(OpBuilder &builder, OperationState &state,
// TODO Store clauses in op: privateVars, privateSyms.
TaskloopOp::build(
builder, state, clauses.allocateVars, clauses.allocatorVars,
- clauses.final, clauses.grainsize, clauses.ifExpr, clauses.inReductionVars,
+ clauses.final, clauses.grainsize, clauses.ifExpr, clauses.inReductionMod,
+ clauses.inReductionVars,
makeDenseBoolArrayAttr(ctx, clauses.inReductionByref),
makeArrayAttr(ctx, clauses.inReductionSyms), clauses.mergeable,
clauses.nogroup, clauses.numTasks, clauses.priority, /*private_vars=*/{},
- /*private_syms=*/nullptr, clauses.reductionVars,
+ /*private_syms=*/nullptr, clauses.reductionMod, clauses.reductionVars,
makeDenseBoolArrayAttr(ctx, clauses.reductionByref),
makeArrayAttr(ctx, clauses.reductionSyms), clauses.untied);
}
@@ -2859,6 +2905,54 @@ void MaskedOp::build(OpBuilder &builder, OperationState &state,
MaskedOp::build(builder, state, clauses.filteredThreadId);
}
+//===----------------------------------------------------------------------===//
+// Spec 5.2: Scan construct (5.6)
+//===----------------------------------------------------------------------===//
+
+void ScanOp::build(OpBuilder &builder, OperationState &state,
+ const ScanOperands &clauses) {
+ ScanOp::build(builder, state, clauses.inclusiveVars, clauses.exclusiveVars);
+}
+
+LogicalResult ScanOp::verify() {
+ if (hasExclusiveVars() && hasInclusiveVars()) {
+ return emitError(
+ "Exactly one of EXCLUSIVE or INCLUSIVE clause is expected");
+ }
+ const OperandRange &scanVars =
+ hasExclusiveVars() ? getExclusiveVars() : getInclusiveVars();
+ auto verifyScanVarsInReduction = [&scanVars](OperandRange reductionVars) {
+ for (const auto &it : scanVars)
+ if (!llvm::is_contained(reductionVars, it))
+ return false;
+ return true;
+ };
+ if (mlir::omp::WsloopOp parentOp =
+ (*this)->getParentOfType<mlir::omp::WsloopOp>()) {
+ if (parentOp.getReductionModAttr() &&
+ parentOp.getReductionModAttr().getValue() ==
+ mlir::omp::ReductionModifier::InScan) {
+ if (!verifyScanVarsInReduction(parentOp.getReductionVars())) {
+ return emitError(
+ "List item should appear in REDUCTION clause of the parent");
+ }
+ return success();
+ }
+ } else if (mlir::omp::SimdOp parentOp =
+ (*this)->getParentOfType<mlir::omp::SimdOp>()) {
+ if (parentOp.getReductionModAttr().getValue() ==
+ mlir::omp::ReductionModifier::InScan) {
+ if (!verifyScanVarsInReduction(parentOp.getReductionVars())) {
+ return emitError(
+ "List item should appear in REDUCTION clause of the parent");
+ }
+ return success();
+ }
+ }
+ return emitError("Scan Operation should be enclosed within a parent "
+ "WORSKSHARING LOOP or SIMD with INSCAN reduction modifier");
+}
+
#define GET_ATTRDEF_CLASSES
#include "mlir/Dialect/OpenMP/OpenMPOpsAttributes.cpp.inc"
diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir
index 2a19e4837f5504..d833cdba32821b 100644
--- a/mlir/test/Dialect/OpenMP/invalid.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid.mlir
@@ -1825,6 +1825,85 @@ func.func @omp_cancellationpoint2() {
// -----
+omp.declare_reduction @add_f32 : f32
+init {
+ ^bb0(%arg: f32):
+ %0 = arith.constant 0.0 : f32
+ omp.yield (%0 : f32)
+}
+combiner {
+ ^bb1(%arg0: f32, %arg1: f32):
+ %1 = arith.addf %arg0, %arg1 : f32
+ omp.yield (%1 : f32)
+}
+
+func.func @scan_test_1(%lb: i32, %ub: i32, %step: i32) {
+ %test1f32 = "test.f32"() : () -> (!llvm.ptr)
+ %test2f32 = "test.f32"() : () -> (!llvm.ptr)
+ omp.wsloop reduction(Id:InScan, @add_f32 %test1f32 -> %arg1 : !llvm.ptr) {
+ omp.loop_nest (%i, %j) : i32 = (%lb, %ub) to (%ub, %lb) step (%step, %step) {
+ // expected-error @below {{List item should appear in REDUCTION clause of the parent}}
+ omp.scan inclusive(%test2f32 : !llvm.ptr)
+ omp.yield
+ }
+ }
+ return
+}
+
+// -----
+
+omp.declare_reduction @add_f32 : f32
+init {
+ ^bb0(%arg: f32):
+ %0 = arith.constant 0.0 : f32
+ omp.yield (%0 : f32)
+}
+combiner {
+ ^bb1(%arg0: f32, %arg1: f32):
+ %1 = arith.addf %arg0, %arg1 : f32
+ omp.yield (%1 : f32)
+}
+
+func.func @scan_test_2(%lb: i32, %ub: i32, %step: i32) {
+ %test1f32 = "test.f32"() : () -> (!llvm.ptr)
+ omp.wsloop reduction(Id:InScan, @add_f32 %test1f32 -> %arg1 : !llvm.ptr) {
+ omp.loop_nest (%i, %j) : i32 = (%lb, %ub) to (%ub, %lb) step (%step, %step) {
+ // expected-error @below {{Exactly one of EXCLUSIVE or INCLUSIVE clause is expected}}
+ omp.scan inclusive(%test1f32 : !llvm.ptr) exclusive(%test1f32: !llvm.ptr)
+ omp.yield
+ }
+ }
+ return
+}
+
+// -----
+
+omp.declare_reduction @add_f32 : f32
+init {
+ ^bb0(%arg: f32):
+ %0 = arith.constant 0.0 : f32
+ omp.yield (%0 : f32)
+}
+combiner {
+ ^bb1(%arg0: f32, %arg1: f32):
+ %1 = arith.addf %arg0, %arg1 : f32
+ omp.yield (%1 : f32)
+}
+
+func.func @scan_test_2(%lb: i32, %ub: i32, %step: i32) {
+ %test1f32 = "test.f32"() : () -> (!llvm.ptr)
+ omp.taskloop reduction(Id:InScan, @add_f32 %test1f32 -> %arg1 : !llvm.ptr) {
+ omp.loop_nest (%i, %j) : i32 = (%lb, %ub) to (%ub, %lb) step (%step, %step) {
+ // expected-error @below {{Scan Operation should be enclosed within a parent WORSKSHARING LOOP or SIMD with INSCAN reduction modifier}}
+ omp.scan inclusive(%test1f32 : !llvm.ptr)
+ omp.yield
+ }
+ }
+ return
+}
+
+// -----
+
func.func @taskloop(%lb: i32, %ub: i32, %step: i32) {
%testmemref = "test.memref"() : () -> (memref<i32>)
// expected-error @below {{expected equal sizes for allocate and allocator variables}}
diff --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir
index c25a6ef4b4849b..4d0a068106dba7 100644
--- a/mlir/test/Dialect/OpenMP/ops.mlir
+++ b/mlir/test/Dialect/OpenMP/ops.mlir
@@ -900,6 +900,29 @@ func.func @wsloop_reduction(%lb : index, %ub : index, %step : index) {
return
}
+// CHECK-LABEL: func @wsloop_inscan_reduction
+func.func @wsloop_inscan_reduction(%lb : index, %ub : index, %step : index) {
+ %c1 = arith.constant 1 : i32
+ %0 = llvm.alloca %c1 x i32 : (i32) -> !llvm.ptr
+ // CHECK: reduction(Id: InScan, @add_f32 %{{.+}} -> %[[PRV:.+]] : !llvm.ptr)
+ omp.wsloop reduction(Id:InScan, @add_f32 %0 -> %prv : !llvm.ptr) {
+ omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
+ // CHECK: omp.scan inclusive(%{{.*}} : !llvm.ptr)
+ omp.scan inclusive(%0 : !llvm.ptr)
+ omp.yield
+ }
+ }
+ // CHECK: reduction(Id: InScan, @add_f32 %{{.+}} -> %[[PRV:.+]] : !llvm.ptr)
+ omp.wsloop reduction(Id:InScan, @add_f32 %0 -> %prv : !llvm.ptr) {
+ omp.loop_nest (%iv) : index = (%lb) to (%ub) step (%step) {
+ // CHECK: omp.scan exclusive(%{{.*}} : !llvm.ptr)
+ omp.scan exclusive(%0 : !llvm.ptr)
+ omp.yield
+ }
+ }
+ return
+}
+
// CHECK-LABEL: func @wsloop_reduction_byref
func.func @wsloop_reduction_byref(%lb : index, %ub : index, %step : index) {
%c1 = arith.constant 1 : i32
>From 3298ae175e339813e4796a7309d2b502c33a9829 Mon Sep 17 00:00:00 2001
From: Anchu Rajendran <asudhaku at amd.com>
Date: Mon, 18 Nov 2024 16:43:45 -0600
Subject: [PATCH 2/3] R2: Addressing a few review comments
---
mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td | 10 +++++-----
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 2 +-
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 8 +++++---
mlir/test/Dialect/OpenMP/invalid.mlir | 2 +-
4 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
index 23086556bbb2f5..8b1479b0692b83 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
@@ -182,16 +182,16 @@ def OrderModifierAttr : EnumAttr<OpenMP_Dialect, OrderModifier,
// reduction_modifier enum.
//===----------------------------------------------------------------------===//
-def ReductionModifierInScan : I32EnumAttrCase<"InScan", 0>;
-def ReductionModifierTask : I32EnumAttrCase<"Task", 1>;
-def ReductionModifierDefault : I32EnumAttrCase<"Default", 2>;
+def ReductionModifierDefault : I32EnumAttrCase<"Default", 0>;
+def ReductionModifierInScan : I32EnumAttrCase<"InScan", 1>;
+def ReductionModifierTask : I32EnumAttrCase<"Task", 2>;
def ReductionModifier : OpenMP_I32EnumAttr<
"ReductionModifier",
"reduction modifier", [
+ ReductionModifierDefault,
ReductionModifierInScan,
- ReductionModifierTask,
- ReductionModifierDefault
+ ReductionModifierTask
]>;
def ReductionModifierAttr : OpenMP_EnumAttr<ReductionModifier,
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index f37f26c50eff58..0ec90ac55d04cf 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -1659,7 +1659,7 @@ def ScanOp : OpenMP_Op<"scan", [
let summary = "scan directive";
let description = [{
The scan directive allows to specify scan reduction. Scan directive
- should be enclosed with in a parent directive along with which , a
+ should be enclosed with in a parent directive along with which, a
reduction clause with `InScan` modifier must be specified. Scan directive
allows to separate code blocks to input phase and scan phase in the region
enclosed by the parent.
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index c46724463b593f..51e4dedbd3e397 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -2026,7 +2026,8 @@ void WsloopOp::build(OpBuilder &builder, OperationState &state,
/*linear_vars=*/ValueRange(), /*linear_step_vars=*/ValueRange(),
/*nowait=*/false, /*order=*/nullptr, /*order_mod=*/nullptr,
/*ordered=*/nullptr, /*private_vars=*/{}, /*private_syms=*/nullptr,
- nullptr, /*reduction_vars=*/ValueRange(), /*reduction_byref=*/nullptr,
+ /*reduction_mod=*/nullptr, /*reduction_vars=*/ValueRange(),
+ /*reduction_byref=*/nullptr,
/*reduction_syms=*/nullptr, /*schedule_kind=*/nullptr,
/*schedule_chunk=*/nullptr, /*schedule_mod=*/nullptr,
/*schedule_simd=*/false);
@@ -2949,8 +2950,9 @@ LogicalResult ScanOp::verify() {
return success();
}
}
- return emitError("Scan Operation should be enclosed within a parent "
- "WORSKSHARING LOOP or SIMD with INSCAN reduction modifier");
+ return emitError("SCAN directive needs to be enclosed within a parent "
+ "worksharing loop construct or SIMD construct with INSCAN "
+ "reduction modifier");
}
#define GET_ATTRDEF_CLASSES
diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir
index d833cdba32821b..3c687594f3cec4 100644
--- a/mlir/test/Dialect/OpenMP/invalid.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid.mlir
@@ -1894,7 +1894,7 @@ func.func @scan_test_2(%lb: i32, %ub: i32, %step: i32) {
%test1f32 = "test.f32"() : () -> (!llvm.ptr)
omp.taskloop reduction(Id:InScan, @add_f32 %test1f32 -> %arg1 : !llvm.ptr) {
omp.loop_nest (%i, %j) : i32 = (%lb, %ub) to (%ub, %lb) step (%step, %step) {
- // expected-error @below {{Scan Operation should be enclosed within a parent WORSKSHARING LOOP or SIMD with INSCAN reduction modifier}}
+ // expected-error @below {{SCAN directive needs to be enclosed within a parent worksharing loop construct or SIMD construct with INSCAN reduction modifier}}
omp.scan inclusive(%test1f32 : !llvm.ptr)
omp.yield
}
>From fdb54cc2a77aa5d16d4820000a7f69b38dec95b0 Mon Sep 17 00:00:00 2001
From: Anchu Rajendran <asudhaku at amd.com>
Date: Fri, 6 Dec 2024 17:36:20 -0600
Subject: [PATCH 3/3] R3: Addressing a few review comments
---
mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td | 10 ++++++++--
mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td | 2 +-
mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 12 ++++++------
mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp | 7 ++-----
4 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td
index 826c5bccaf434c..04580c6a11d64c 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td
@@ -327,7 +327,7 @@ class OpenMP_ExclusiveClauseSkip<
let extraClassDeclaration = [{
bool hasExclusiveVars() {
- return getExclusiveVars().size()>0;
+ return !getExclusiveVars().empty();
}
}];
@@ -337,6 +337,9 @@ class OpenMP_ExclusiveClauseSkip<
is specified, the input phase excludes the preceding structured block
sequence and instead includes the following structured block sequence,
while the scan phase includes the preceding structured block sequence.
+
+ The `exclusive_vars` is a variadic list of operands that specifies the
+ scan-reduction accumulator symbols.
}];
}
@@ -471,7 +474,7 @@ class OpenMP_InclusiveClauseSkip<
let extraClassDeclaration = [{
bool hasInclusiveVars() {
- return getInclusiveVars().size()>0;
+ return !getInclusiveVars().empty();
}
}];
@@ -480,6 +483,9 @@ class OpenMP_InclusiveClauseSkip<
structured block into two structured block sequences. If it is specified,
the input phase includes the preceding structured block sequence and the
scan phase includes the following structured block sequence.
+
+ The `inclusive_vars` is a variadic list of operands that specifies the
+ scan-reduction accumulator symbols.
}];
}
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
index 8b1479b0692b83..cf11f119b71ecd 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPEnums.td
@@ -195,7 +195,7 @@ def ReductionModifier : OpenMP_I32EnumAttr<
]>;
def ReductionModifierAttr : OpenMP_EnumAttr<ReductionModifier,
- "reduction_modifier"> {
+ "reduction_modifier"> {
let assemblyFormat = "`(` $value `)`";
}
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 0ec90ac55d04cf..0812e4bcae4e41 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -461,7 +461,7 @@ def LoopOp : OpenMP_Op<"loop", traits = [
let assemblyFormat = clausesAssemblyFormat # [{
custom<PrivateReductionRegion>($region, $private_vars, type($private_vars),
- $private_syms, $reduction_vars, type($reduction_vars), $reduction_byref,
+ $private_syms, $reduction_mod, $reduction_vars, type($reduction_vars), $reduction_byref,
$reduction_syms) attr-dict
}];
@@ -1655,13 +1655,13 @@ def CancellationPointOp : OpenMP_Op<"cancellation_point", clauses = [
def ScanOp : OpenMP_Op<"scan", [
AttrSizedOperandSegments, RecipeInterface, IsolatedFromAbove
], clauses = [
- OpenMP_InclusiveClause, OpenMP_ExclusiveClause]> {
+ OpenMP_InclusiveClause, OpenMP_ExclusiveClause]> {
let summary = "scan directive";
let description = [{
- The scan directive allows to specify scan reduction. Scan directive
- should be enclosed with in a parent directive along with which, a
- reduction clause with `InScan` modifier must be specified. Scan directive
- allows to separate code blocks to input phase and scan phase in the region
+ The scan directive allows to specify scan reductions. It should be
+ enclosed within a parent directive along with which a reduction clause
+ with `inscan` modifier must be specified. The scan directive allows to
+ split code blocks into input phase and scan phase in the region
enclosed by the parent.
}] # clausesDescription;
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 51e4dedbd3e397..ab8418e2b280e0 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -506,7 +506,6 @@ struct ReductionParseArgs {
syms(syms) {}
};
-// specifies the arguments needs for `reduction` clause
struct AllRegionParseArgs {
std::optional<ReductionParseArgs> inReductionArgs;
std::optional<MapParseArgs> mapArgs;
@@ -616,7 +615,6 @@ static ParseResult parseBlockArgClause(
if (succeeded(parser.parseOptionalKeyword(keyword))) {
if (!reductionArgs)
return failure();
-
if (failed(parseClauseWithRegionArgs(parser, reductionArgs->vars,
reductionArgs->types, entryBlockArgs,
&reductionArgs->syms)))
@@ -834,9 +832,8 @@ static void printClauseWithRegionArgs(
p << clauseName << "(";
- if (reductionMod) {
+ if (reductionMod)
p << "Id: " << stringifyReductionModifier(reductionMod.getValue()) << ", ";
- }
if (!symbols) {
llvm::SmallVector<Attribute> values(operands.size(), nullptr);
@@ -1998,7 +1995,7 @@ void LoopOp::build(OpBuilder &builder, OperationState &state,
LoopOp::build(builder, state, clauses.bindKind, clauses.privateVars,
makeArrayAttr(ctx, clauses.privateSyms), clauses.order,
- clauses.orderMod, clauses.reductionVars,
+ clauses.orderMod, clauses.reductionMod, clauses.reductionVars,
makeDenseBoolArrayAttr(ctx, clauses.reductionByref),
makeArrayAttr(ctx, clauses.reductionSyms));
}
More information about the Mlir-commits
mailing list