Skip to content

Commit 237a910

Browse files
authored
[MLIR][OpenMP] Remove the ReductionClauseInterface, NFC (#130978)
This patch removes the `ReductionClauseInterface` and all definitions of its associated `getAllReductionVars` method. The method mandated by this interface is not used anywhere and the conflicts its definition produces when multiple reduction clauses are present in an operation result in a more convoluted operation definition, so it seems better to remove it and only add something like this if there's a clear advantage to it.
1 parent c3c97ea commit 237a910

File tree

4 files changed

+6
-51
lines changed

4 files changed

+6
-51
lines changed

mlir/include/mlir/Dialect/OpenMP/OpenMPClauses.td

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ class OpenMP_InReductionClauseSkip<
606606
> : OpenMP_Clause<traits, arguments, assemblyFormat, description,
607607
extraClassDeclaration> {
608608
let traits = [
609-
BlockArgOpenMPOpInterface, ReductionClauseInterface
609+
BlockArgOpenMPOpInterface
610610
];
611611

612612
let arguments = (ins
@@ -615,14 +615,6 @@ class OpenMP_InReductionClauseSkip<
615615
OptionalAttr<SymbolRefArrayAttr>:$in_reduction_syms
616616
);
617617

618-
let extraClassDeclaration = [{
619-
/// Returns the reduction variables.
620-
SmallVector<Value> getAllReductionVars() {
621-
return SmallVector<Value>(getInReductionVars().begin(),
622-
getInReductionVars().end());
623-
}
624-
}];
625-
626618
// Description varies depending on the operation. Assembly format not defined
627619
// because this clause must be processed together with the first region of the
628620
// operation, as it defines entry block arguments.
@@ -1156,7 +1148,7 @@ class OpenMP_ReductionClauseSkip<
11561148
> : OpenMP_Clause<traits, arguments, assemblyFormat, description,
11571149
extraClassDeclaration> {
11581150
let traits = [
1159-
BlockArgOpenMPOpInterface, ReductionClauseInterface
1151+
BlockArgOpenMPOpInterface
11601152
];
11611153

11621154
let arguments = (ins
@@ -1287,7 +1279,7 @@ class OpenMP_TaskReductionClauseSkip<
12871279
> : OpenMP_Clause<traits, arguments, assemblyFormat, description,
12881280
extraClassDeclaration> {
12891281
let traits = [
1290-
BlockArgOpenMPOpInterface, ReductionClauseInterface
1282+
BlockArgOpenMPOpInterface
12911283
];
12921284

12931285
let arguments = (ins
@@ -1296,14 +1288,6 @@ class OpenMP_TaskReductionClauseSkip<
12961288
OptionalAttr<SymbolRefArrayAttr>:$task_reduction_syms
12971289
);
12981290

1299-
let extraClassDeclaration = [{
1300-
/// Returns the reduction variables.
1301-
SmallVector<Value> getAllReductionVars() {
1302-
return SmallVector<Value>(getTaskReductionVars().begin(),
1303-
getTaskReductionVars().end());
1304-
}
1305-
}];
1306-
13071291
let description = [{
13081292
The `task_reduction` clause specifies a reduction among tasks. For each list
13091293
item, the number of copies is unspecified. Any copies associated with the

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -751,11 +751,9 @@ def TaskloopOp : OpenMP_Op<"taskloop", traits = [
751751
RecursiveMemoryEffects, SingleBlock
752752
], clauses = [
753753
OpenMP_AllocateClause, OpenMP_FinalClause, OpenMP_GrainsizeClause,
754-
OpenMP_IfClause, OpenMP_InReductionClauseSkip<extraClassDeclaration = true>,
755-
OpenMP_MergeableClause, OpenMP_NogroupClause, OpenMP_NumTasksClause,
756-
OpenMP_PriorityClause, OpenMP_PrivateClause,
757-
OpenMP_ReductionClauseSkip<extraClassDeclaration = true>,
758-
OpenMP_UntiedClause
754+
OpenMP_IfClause, OpenMP_InReductionClause, OpenMP_MergeableClause,
755+
OpenMP_NogroupClause, OpenMP_NumTasksClause, OpenMP_PriorityClause,
756+
OpenMP_PrivateClause, OpenMP_ReductionClause, OpenMP_UntiedClause
759757
], singleRegion = true> {
760758
let summary = "taskloop construct";
761759
let description = [{
@@ -821,9 +819,6 @@ def TaskloopOp : OpenMP_Op<"taskloop", traits = [
821819
}];
822820

823821
let extraClassDeclaration = [{
824-
/// Returns the reduction variables
825-
SmallVector<Value> getAllReductionVars();
826-
827822
void getEffects(SmallVectorImpl<MemoryEffects::EffectInstance> &effects);
828823
}] # clausesExtraClassDeclaration;
829824

mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -216,22 +216,6 @@ def MapClauseOwningOpInterface : OpInterface<"MapClauseOwningOpInterface"> {
216216
];
217217
}
218218

219-
def ReductionClauseInterface : OpInterface<"ReductionClauseInterface"> {
220-
let description = [{
221-
OpenMP operations that support reduction clause have this interface.
222-
}];
223-
224-
let cppNamespace = "::mlir::omp";
225-
226-
let methods = [
227-
InterfaceMethod<
228-
"Get reduction vars", "::mlir::SmallVector<::mlir::Value>",
229-
"getAllReductionVars", (ins), [{}], [{
230-
return $_op.getReductionVars();
231-
}]>,
232-
];
233-
}
234-
235219
def LoopWrapperInterface : OpInterface<"LoopWrapperInterface"> {
236220
let description = [{
237221
OpenMP operations that wrap a single loop nest. They must only contain a

mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2726,14 +2726,6 @@ void TaskloopOp::build(OpBuilder &builder, OperationState &state,
27262726
makeArrayAttr(ctx, clauses.reductionSyms), clauses.untied);
27272727
}
27282728

2729-
SmallVector<Value> TaskloopOp::getAllReductionVars() {
2730-
SmallVector<Value> allReductionNvars(getInReductionVars().begin(),
2731-
getInReductionVars().end());
2732-
allReductionNvars.insert(allReductionNvars.end(), getReductionVars().begin(),
2733-
getReductionVars().end());
2734-
return allReductionNvars;
2735-
}
2736-
27372729
LogicalResult TaskloopOp::verify() {
27382730
if (getAllocateVars().size() != getAllocatorVars().size())
27392731
return emitError(

0 commit comments

Comments
 (0)