[Mlir-commits] [mlir] 268ff38 - [MLIR][OpenMP] Attribute to include WsLoop upperbound

[Kiran Chandramohan]

Author: Kiran Chandramohan
Date: 2021-01-08T14:42:18Z
New Revision: 268ff38a716157c362b8d463e2e5655f25972e42

URL: https://github.com/llvm/llvm-project/commit/268ff38a716157c362b8d463e2e5655f25972e42
DIFF: https://github.com/llvm/llvm-project/commit/268ff38a716157c362b8d463e2e5655f25972e42.diff

LOG: [MLIR][OpenMP] Attribute to include WsLoop upperbound

This patch adds an attribute `inclusive` which if present causes
the upperbound to be included in the loop iteration interval.

Reviewed By: ftynse
Differential Revision: https://reviews.llvm.org/D94235

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
    mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
    mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
    mlir/test/Target/openmp-llvm.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index 6c6230f0c2e8..92e418fba7af 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -122,7 +122,9 @@ def WsLoopOp : OpenMP_Op<"wsloop", [AttrSizedOperandSegments]> {
     The workshare loop construct specifies that the iterations of the loop(s)
     will be executed in parallel by threads in the current context. These
     iterations are spread across threads that already exist in the enclosing
-    parallel region.
+    parallel region. The lower and upper bounds specify a half-open range: the
+    range includes the lower bound but does not include the upper bound. If the
+    `inclusive` attribute is specified then the upper bound is also included.
 
     The body region can contain any number of blocks. The region is terminated
     by "omp.yield" instruction without operands.
@@ -174,9 +176,10 @@ def WsLoopOp : OpenMP_Op<"wsloop", [AttrSizedOperandSegments]> {
              OptionalAttr<ScheduleKind>:$schedule_val,
              Optional<AnyType>:$schedule_chunk_var,
              Confined<OptionalAttr<I64Attr>, [IntMinValue<0>]>:$collapse_val,
-             OptionalAttr<UnitAttr>:$nowait,
+             UnitAttr:$nowait,
              Confined<OptionalAttr<I64Attr>, [IntMinValue<0>]>:$ordered_val,
-             OptionalAttr<OrderKind>:$order_val);
+             OptionalAttr<OrderKind>:$order_val,
+             UnitAttr:$inclusive);
 
   let builders = [
     OpBuilderDAG<(ins "ValueRange":$lowerBound, "ValueRange":$upperBound,

diff  --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index f4b76b635128..907ba65c07b7 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -386,7 +386,8 @@ void WsLoopOp::build(OpBuilder &builder, OperationState &state,
         /*linear_vars=*/ValueRange(), /*linear_step_vars=*/ValueRange(),
         /*schedule_val=*/nullptr, /*schedule_chunk_var=*/nullptr,
         /*collapse_val=*/nullptr,
-        /*nowait=*/nullptr, /*ordered_val=*/nullptr, /*order_val=*/nullptr);
+        /*nowait=*/false, /*ordered_val=*/nullptr, /*order_val=*/nullptr,
+        /*inclusive=*/false);
   state.addAttributes(attributes);
 }
 

diff  --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 492025ba37b4..70e35c7c7997 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -590,13 +590,12 @@ LogicalResult ModuleTranslation::convertOmpWsLoop(Operation &opInst,
 
   // Delegate actual loop construction to the OpenMP IRBuilder.
   // TODO: this currently assumes WsLoop is semantically similar to SCF loop,
-  // i.e. it has a positive step, uses signed integer semantics, and its upper
-  // bound is not included. Reconsider this code when WsLoop clearly supports
-  // more cases.
+  // i.e. it has a positive step, uses signed integer semantics. Reconsider
+  // this code when WsLoop clearly supports more cases.
   llvm::BasicBlock *insertBlock = builder.GetInsertBlock();
   llvm::CanonicalLoopInfo *loopInfo = ompBuilder->createCanonicalLoop(
       ompLoc, bodyGen, lowerBound, upperBound, step, /*IsSigned=*/true,
-      /*InclusiveStop=*/false);
+      /*InclusiveStop=*/loop.inclusive());
   if (failed(bodyGenStatus))
     return failure();
 
@@ -606,9 +605,8 @@ LogicalResult ModuleTranslation::convertOmpWsLoop(Operation &opInst,
   // Put them at the start of the current block for now.
   llvm::OpenMPIRBuilder::InsertPointTy allocaIP(
       insertBlock, insertBlock->getFirstInsertionPt());
-  loopInfo = ompBuilder->createStaticWorkshareLoop(
-      ompLoc, loopInfo, allocaIP,
-      !loop.nowait().hasValue() || loop.nowait().getValue(), chunk);
+  loopInfo = ompBuilder->createStaticWorkshareLoop(ompLoc, loopInfo, allocaIP,
+                                                   !loop.nowait(), chunk);
 
   // Continue building IR after the loop.
   builder.restoreIP(loopInfo->getAfterIP());

diff  --git a/mlir/test/Target/openmp-llvm.mlir b/mlir/test/Target/openmp-llvm.mlir
index a4d00c510abb..0e7ace7ec91f 100644
--- a/mlir/test/Target/openmp-llvm.mlir
+++ b/mlir/test/Target/openmp-llvm.mlir
@@ -323,3 +323,35 @@ llvm.func @wsloop_simple(%arg0: !llvm.ptr<float>) {
   }
   llvm.return
 }
+
+// CHECK-LABEL: @wsloop_inclusive_1
+llvm.func @wsloop_inclusive_1(%arg0: !llvm.ptr<float>) {
+  %0 = llvm.mlir.constant(42 : index) : !llvm.i64
+  %1 = llvm.mlir.constant(10 : index) : !llvm.i64
+  %2 = llvm.mlir.constant(1 : index) : !llvm.i64
+  // CHECK: store i64 31, i64* %{{.*}}upperbound
+  "omp.wsloop"(%1, %0, %2) ( {
+  ^bb0(%arg1: !llvm.i64):
+    %3 = llvm.mlir.constant(2.000000e+00 : f32) : !llvm.float
+    %4 = llvm.getelementptr %arg0[%arg1] : (!llvm.ptr<float>, !llvm.i64) -> !llvm.ptr<float>
+    llvm.store %3, %4 : !llvm.ptr<float>
+    omp.yield
+  }) {operand_segment_sizes = dense<[1, 1, 1, 0, 0, 0, 0, 0, 0]> : vector<9xi32>} : (!llvm.i64, !llvm.i64, !llvm.i64) -> ()
+  llvm.return
+}
+
+// CHECK-LABEL: @wsloop_inclusive_2
+llvm.func @wsloop_inclusive_2(%arg0: !llvm.ptr<float>) {
+  %0 = llvm.mlir.constant(42 : index) : !llvm.i64
+  %1 = llvm.mlir.constant(10 : index) : !llvm.i64
+  %2 = llvm.mlir.constant(1 : index) : !llvm.i64
+  // CHECK: store i64 32, i64* %{{.*}}upperbound
+  "omp.wsloop"(%1, %0, %2) ( {
+  ^bb0(%arg1: !llvm.i64):
+    %3 = llvm.mlir.constant(2.000000e+00 : f32) : !llvm.float
+    %4 = llvm.getelementptr %arg0[%arg1] : (!llvm.ptr<float>, !llvm.i64) -> !llvm.ptr<float>
+    llvm.store %3, %4 : !llvm.ptr<float>
+    omp.yield
+  }) {inclusive, operand_segment_sizes = dense<[1, 1, 1, 0, 0, 0, 0, 0, 0]> : vector<9xi32>} : (!llvm.i64, !llvm.i64, !llvm.i64) -> ()
+  llvm.return
+}