From b439a728e8d187b96bb84095818d48be74eea2bd Mon Sep 17 00:00:00 2001 From: Akash Banerjee Date: Mon, 5 Aug 2024 15:31:25 +0100 Subject: [PATCH 1/3] [MLIR][OpenMP] Add a new ComposableLoopWrapperInterface to check and set a composite unitAttr. --- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 9 +++-- .../Dialect/OpenMP/OpenMPOpsInterfaces.td | 34 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td index 68f92e6952694..44210025dfe18 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td @@ -129,6 +129,7 @@ def PrivateClauseOp : OpenMP_Op<"private", [IsolatedFromAbove, RecipeInterface]> def ParallelOp : OpenMP_Op<"parallel", traits = [ AttrSizedOperandSegments, AutomaticAllocationScope, DeclareOpInterfaceMethods, + DeclareOpInterfaceMethods, DeclareOpInterfaceMethods, RecursiveMemoryEffects ], clauses = [ @@ -357,6 +358,7 @@ def LoopNestOp : OpenMP_Op<"loop_nest", traits = [ def WsloopOp : OpenMP_Op<"wsloop", traits = [ AttrSizedOperandSegments, DeclareOpInterfaceMethods, + DeclareOpInterfaceMethods, RecursiveMemoryEffects, SingleBlock ], clauses = [ OpenMP_AllocateClauseSkip, @@ -433,6 +435,7 @@ def WsloopOp : OpenMP_Op<"wsloop", traits = [ def SimdOp : OpenMP_Op<"simd", traits = [ AttrSizedOperandSegments, DeclareOpInterfaceMethods, + DeclareOpInterfaceMethods, RecursiveMemoryEffects, SingleBlock ], clauses = [ OpenMP_AlignedClause, OpenMP_IfClause, OpenMP_LinearClause, @@ -500,6 +503,7 @@ def YieldOp : OpenMP_Op<"yield", //===----------------------------------------------------------------------===// def DistributeOp : OpenMP_Op<"distribute", traits = [ AttrSizedOperandSegments, DeclareOpInterfaceMethods, + DeclareOpInterfaceMethods, RecursiveMemoryEffects, SingleBlock ], clauses = [ OpenMP_AllocateClause, OpenMP_DistScheduleClause, OpenMP_OrderClause, @@ -587,8 +591,9 @@ def TaskOp : OpenMP_Op<"task", traits = [ def TaskloopOp : OpenMP_Op<"taskloop", traits = [ AttrSizedOperandSegments, AutomaticAllocationScope, - DeclareOpInterfaceMethods, RecursiveMemoryEffects, - SingleBlock + DeclareOpInterfaceMethods, + DeclareOpInterfaceMethods, + RecursiveMemoryEffects, SingleBlock ], clauses = [ OpenMP_AllocateClause, OpenMP_FinalClause, OpenMP_GrainsizeClause, OpenMP_IfClause, OpenMP_InReductionClauseSkip, diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td index 2d1de37239c82..50394eaf236f9 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td @@ -142,6 +142,40 @@ def LoopWrapperInterface : OpInterface<"LoopWrapperInterface"> { ]; } +def ComposableLoopWrapperInterface : OpInterface<"ComposableLoopWrapperInterface"> { + let description = [{ + OpenMP operations that can wrap a single loop nest. When taking a wrapper + role, these operations must only contain a single region with a single block + in which there's a single operation and a terminator. That nested operation + must be another loop wrapper or an `omp.loop_nest`. + }]; + + let cppNamespace = "::mlir::omp"; + + let methods = [ + InterfaceMethod< + /*description=*/[{ + Check whether the operation is composable. + }], + /*retTy=*/"bool", + /*methodName=*/"isComposite", + (ins ), [{}], [{ + return $_op->hasAttr("omp.composite"); + }] + >, + InterfaceMethod< + /*description=*/[{ + Set the CompositeAttr for the op. + }], + /*retTy=*/"void", + /*methodName=*/"setComposite", + (ins), [{}], [{ + $_op->setDiscardableAttr("omp.composite", mlir::UnitAttr::get($_op->getContext())); + }] + > + ]; +} + def DeclareTargetInterface : OpInterface<"DeclareTargetInterface"> { let description = [{ OpenMP operations that support declare target have this interface. From 1fa21a12714190c32eb91c00881e3ecb4734c0ad Mon Sep 17 00:00:00 2001 From: Akash Banerjee Date: Tue, 6 Aug 2024 13:25:06 +0100 Subject: [PATCH 2/3] Address reviewer comments. --- mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | 19 ++++++++++-------- .../Dialect/OpenMP/OpenMPOpsInterfaces.td | 20 ++++++++++--------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td index 44210025dfe18..d63fdd88f7910 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td @@ -128,8 +128,8 @@ def PrivateClauseOp : OpenMP_Op<"private", [IsolatedFromAbove, RecipeInterface]> def ParallelOp : OpenMP_Op<"parallel", traits = [ AttrSizedOperandSegments, AutomaticAllocationScope, + DeclareOpInterfaceMethods, DeclareOpInterfaceMethods, - DeclareOpInterfaceMethods, DeclareOpInterfaceMethods, RecursiveMemoryEffects ], clauses = [ @@ -357,8 +357,9 @@ def LoopNestOp : OpenMP_Op<"loop_nest", traits = [ //===----------------------------------------------------------------------===// def WsloopOp : OpenMP_Op<"wsloop", traits = [ - AttrSizedOperandSegments, DeclareOpInterfaceMethods, - DeclareOpInterfaceMethods, + AttrSizedOperandSegments, + DeclareOpInterfaceMethods, + DeclareOpInterfaceMethods, RecursiveMemoryEffects, SingleBlock ], clauses = [ OpenMP_AllocateClauseSkip, @@ -434,8 +435,9 @@ def WsloopOp : OpenMP_Op<"wsloop", traits = [ //===----------------------------------------------------------------------===// def SimdOp : OpenMP_Op<"simd", traits = [ - AttrSizedOperandSegments, DeclareOpInterfaceMethods, - DeclareOpInterfaceMethods, + AttrSizedOperandSegments, + DeclareOpInterfaceMethods, + DeclareOpInterfaceMethods, RecursiveMemoryEffects, SingleBlock ], clauses = [ OpenMP_AlignedClause, OpenMP_IfClause, OpenMP_LinearClause, @@ -502,8 +504,9 @@ def YieldOp : OpenMP_Op<"yield", // Distribute construct [2.9.4.1] //===----------------------------------------------------------------------===// def DistributeOp : OpenMP_Op<"distribute", traits = [ - AttrSizedOperandSegments, DeclareOpInterfaceMethods, - DeclareOpInterfaceMethods, + AttrSizedOperandSegments, + DeclareOpInterfaceMethods, + DeclareOpInterfaceMethods, RecursiveMemoryEffects, SingleBlock ], clauses = [ OpenMP_AllocateClause, OpenMP_DistScheduleClause, OpenMP_OrderClause, @@ -591,8 +594,8 @@ def TaskOp : OpenMP_Op<"task", traits = [ def TaskloopOp : OpenMP_Op<"taskloop", traits = [ AttrSizedOperandSegments, AutomaticAllocationScope, + DeclareOpInterfaceMethods, DeclareOpInterfaceMethods, - DeclareOpInterfaceMethods, RecursiveMemoryEffects, SingleBlock ], clauses = [ OpenMP_AllocateClause, OpenMP_FinalClause, OpenMP_GrainsizeClause, diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td index 50394eaf236f9..4cf847f55be21 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td @@ -142,12 +142,10 @@ def LoopWrapperInterface : OpInterface<"LoopWrapperInterface"> { ]; } -def ComposableLoopWrapperInterface : OpInterface<"ComposableLoopWrapperInterface"> { +def ComposableOpInterface : OpInterface<"ComposableOpInterface"> { let description = [{ - OpenMP operations that can wrap a single loop nest. When taking a wrapper - role, these operations must only contain a single region with a single block - in which there's a single operation and a terminator. That nested operation - must be another loop wrapper or an `omp.loop_nest`. + OpenMP operations that can represent a single leaf of a composite OpenMP + construct. }]; let cppNamespace = "::mlir::omp"; @@ -155,7 +153,8 @@ def ComposableLoopWrapperInterface : OpInterface<"ComposableLoopWrapperInterface let methods = [ InterfaceMethod< /*description=*/[{ - Check whether the operation is composable. + Check whether the operation is representing a leaf of a composite OpenMP + construct. }], /*retTy=*/"bool", /*methodName=*/"isComposite", @@ -165,12 +164,15 @@ def ComposableLoopWrapperInterface : OpInterface<"ComposableLoopWrapperInterface >, InterfaceMethod< /*description=*/[{ - Set the CompositeAttr for the op. + Mark the operation as part of an OpenMP composite construct. }], /*retTy=*/"void", /*methodName=*/"setComposite", - (ins), [{}], [{ - $_op->setDiscardableAttr("omp.composite", mlir::UnitAttr::get($_op->getContext())); + (ins "bool":$val), [{}], [{ + if(val) + $_op->setDiscardableAttr("omp.composite", mlir::UnitAttr::get($_op->getContext())); + else + $_op->removeDiscardableAttr("omp.composite"); }] > ]; From 047dea1149455c516385241b8a6e1d26e44266f5 Mon Sep 17 00:00:00 2001 From: Akash Banerjee Date: Thu, 8 Aug 2024 18:12:23 +0100 Subject: [PATCH 3/3] Add formatting space. --- mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td index 4cf847f55be21..78637f0ab8c2d 100644 --- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td +++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOpsInterfaces.td @@ -169,7 +169,7 @@ def ComposableOpInterface : OpInterface<"ComposableOpInterface"> { /*retTy=*/"void", /*methodName=*/"setComposite", (ins "bool":$val), [{}], [{ - if(val) + if (val) $_op->setDiscardableAttr("omp.composite", mlir::UnitAttr::get($_op->getContext())); else $_op->removeDiscardableAttr("omp.composite");