[MLIR][OpenMP]Basic OpenMP target operation

[abidmalikwaterloo]

Summary

This includes a basic implementation for the OpenMP target
operation. Currently, the if, thread_limit, private, shared, device, and nowait clauses are included in this implementation.

Co-authored-by: Kiran Chandramohan <[e-mail]>

[ftynse]

LGTM with comments addressed.

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

17

Nit: spurious empty line

291–292

Let's drop these empty lines please.

mlir/test/Dialect/OpenMP/ops.mlir

285

Please use full sentences with the first letter capitalized and a full stop in comments (https://llvm.org/docs/CodingStandards.html#commenting).

286

We usually align these with the line below.

[abidmalikwaterloo]

Took care of the comments.

[kiranchandramohan]

I am requesting a change for the nowait attribute. Have a few questions as well.

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

285–288

Can nowait be a unit attribute like in the worksharing loop (UnitAttr:$nowait)?

Does AnyInteger cover boolean types as well?

Will AnyInteger work with out-of-tree dialects like FIR? I guess it will because FIR now uses mlir integer types.

Can the suffix _op be dropped?

mlir/test/Dialect/OpenMP/ops.mlir

287

Nit: space before %device.

[abidmalikwaterloo]

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

285–288

I went through the OpenACC dialect, work-sharing loop op, and MLIR operation definition specifications. I agree, making nowait a unit attribute makes sense.

[abidmalikwaterloo]

Took care of the feedback and comments.

[SouraVX]

Thanks @abidmalikwaterloo for the patch!

Couple of NITs:

• Please prefix the patch title with [mlir][OpenMP], this is convention we/community follow :)
• And if possible can you make the title/subject/summary more specific to the intent, usually people first read those to get an idea :)

[kiranchandramohan]

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

285–288

In the OpenMP Lowering flow, the OpenMP operation can exist along with the LLVM dialect so the Integer that an OpenMP clause can take can be an LLVM Integer. I believe the LLVM Integer was made similar to the mlir Integer but I can see the existence of LLVM_AnyInteger.
@ftynse Is there any difference between LLVM_AnyInteger and AnyInteger. They use the same predicate. If there is no difference can LLVM_AnyInteger be removed?

// Type constraint accepting LLVM integer types.
def LLVM_AnyInteger : Type<CPred<"$_self.isa<::mlir::IntegerType>()">, "LLVM integer type">;

// Any integer type irrespective of its width and signedness semantics.
def AnyInteger : Type<CPred<"$_self.isa<::mlir::IntegerType>()">, "integer", "::mlir::IntegerType">;

[clementval]

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

285

Shouldn't if_expr be of type I1?

[abidmalikwaterloo]

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

285

@clementval being boolean, it can be. I tried to keep the types close to the parallel operation

285–288

@ftynse any feedback on Kiran's comments.

[kiranchandramohan]

LGTM. One request for change.

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

285

Please switch to i1, we can generalise later if required.

[ftynse]

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

285–288

LLVM dialect no longer has its own integer type and uses built-in integers. A cleanup to remove LLVM_AnyInteger predicate is welcome.

[abidmalikwaterloo]

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

285

Ok will do

[abidmalikwaterloo]

Made changes according to the comments from the reviewers.
SI32 for If condition changes to I1 (boolean).

[kiranchandramohan]

@abidmalikwaterloo Do you have commit access? If not, I can submit this for you.

[abidmalikwaterloo]

@kiranchandramohan I am not sure if I have commit access. You can do this for me.

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

 //===-- OpenMPOps.td - OpenMP dialect operation definitions *- tablegen -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines the basic operations for the OpenMP dialect.
 //
 //===----------------------------------------------------------------------===//
 
 
 #ifndef OPENMP_OPS
 #define OPENMP_OPS
 
 include "mlir/IR/OpBase.td"
 include "mlir/Interfaces/SideEffectInterfaces.td"
 include "mlir/Interfaces/ControlFlowInterfaces.td"
 include "mlir/Dialect/LLVMIR/LLVMOpBase.td"
 include "mlir/Dialect/OpenMP/OmpCommon.td"
 
 def OpenMP_Dialect : Dialect {
   let name = "omp";
   let cppNamespace = "::mlir::omp";
let description = [{
     enforces an order on the memory operations of the variables explicitly
     specified or implied.
   }];
 
   let arguments = (ins Variadic<AnyType>:$varList);
 
   let assemblyFormat = [{ ( `(` $varList^ `:` type($varList) `)` )? attr-dict}];
 }
+//===----------------------------------------------------------------------===//
+// 2.14.5 target construct
+//===----------------------------------------------------------------------===//
+
+def TargetOp : OpenMP_Op<"target",[AttrSizedOperandSegments]> {
+  let summary = "target construct";
+  let description = [{
+    The target construct includes a region of code which is to be executed
+    on a device.
+
+    The optional $if_expr parameter specifies a boolean result of a
+    conditional check. If this value is 1 or is not provided then the target
+    region runs on a device, if it is 0 then the target region is executed on the
+    host device.
+
+    The optional $device parameter specifies the device number for the target region.
+
+    The optional $thread_limit specifies the limit on the number of threads
+    
+    The optional $nowait elliminates the implicit barrier so the parent task can make progress
+    even if the target task is not yet completed.
+    
+    TODO:  private, map, is_device_ptr, firstprivate, depend, defaultmap, in_reduction
+    
+  }];
+
+  let arguments = (ins Optional<I1>:$if_expr,
+                       Optional<AnyInteger>:$device,
+                       Optional<AnyInteger>:$thread_limit,
+                       UnitAttr:$nowait);
+
+  let regions = (region AnyRegion:$region);
+}
+
 
 //===----------------------------------------------------------------------===//
 // 2.16 master Construct
 //===----------------------------------------------------------------------===//
 def MasterOp : OpenMP_Op<"master"> {
   let summary = "master construct";
   let description = [{
     The master construct specifies a structured block that is executed by

mlir/test/Dialect/OpenMP/ops.mlir

omp.wsloop (%iv1, %iv2) : i32 = (%lb1, %lb2) to (%ub1, %ub2) step (%step1, %step2) {
     %2 = "test.payload"(%iv2) : (i32) -> (index)
     memref.store %iv1, %data1[%1] : memref<?xi32>
     memref.store %iv2, %data1[%2] : memref<?xi32>
     omp.yield
   }
 
   return
 }
+
+// CHECK-LABEL: omp_target
+func @omp_target(%if_cond : i1, %device : si32,  %num_threads : si32) -> () {
+
+    // Test with optional operands; if_expr, device, thread_limit, and nowait.
+    // CHECK: omp.target
+    "omp.target"(%if_cond, %device, %num_threads) ({
+       // CHECK: omp.terminator
+       omp.terminator
+    }) {operand_segment_sizes = dense<[1,1,1]>: vector<3xi32>, nowait } : ( i1, si32, si32  ) -> ()
+
+    // CHECK: omp.barrier
+    omp.barrier
+
+    return
+}