[MLIR][OpenMP]Add custom parser and pretty printer for target construct

[abidmalikwaterloo]

Summary

added custom parser and pretty printer for target construct with if, thread_limit, nowait, and device clauses

[kiranchandramohan]

Requesting a few changes.

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

100

Nit: Is there an extra space before '=' now?

260–301

You can remove this and add D102816 as a parent patch.

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

26

Is this required?

789–800

This build function is probably not required. The autogenerated one will probably do this I guess. Do you need this for a particular reason?

883

Nit: Please add an empty line after this function.

mlir/test/Dialect/OpenMP/ops.mlir

93

Nit: Accidental change?

167–168

Nit: some formatting accidentally added?

286–299

This test is using the generic syntax and probably not required.

306

Nit: One or two spaces at the beginning here.

310

Add the if, thread_limit, nowait tests here or in another omp.target op.

311

Nit: One or two spaces at the beginning here.

[clementval]

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

789–800

+1

808

809

nit: added spaces just to fit with previous style

830

Might be good to have these keywords declared as constexpr somewhere (extraClassDeclaration) so you can reuse them in the printer.

856

Would be nice to have the attribute name declared in the class and not hard-coded here. It's pretty sure that it will be used in conversion and translation.

[abidmalikwaterloo]

Took care of the comments.

[abidmalikwaterloo]

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

100

No. I will correct it.

260–301

I have made D102816 a parent task. However, I am introducing

let parser = [{ return parseTargetOp(parser, result); }];
let printer = [{ return printTargetOp(p, *this); }];

here.

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

26

No, I will remove it.

809

OK.

830

OK. Should we do it to the other operations as well? ParallelOps

856

OK

883

OK

mlir/test/Dialect/OpenMP/ops.mlir

93

Yes.. Will fix it

167–168

Got it.

[abidmalikwaterloo]

Took care of the comments from the reviewers.

[clementval]

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

295

Not aligned correctly.

300

Indentation is not correct for this line and the rest inside should be indented as well.

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

807

Why not using SmallVector?

[abidmalikwaterloo]

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

807

I will change it to SmallVector<int,3>

[abidmalikwaterloo]

Made changes according to the reviewrs' comments and feedback.

[abidmalikwaterloo]

Remove std:array<int,3> and replace it with SmallVector<int,3>

[clementval]

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

26

What's the reason to remove this blank line?

171

Unrelated change?

807

It's recommended to not specify the number of inlined elements for SmallVector unless you have a good reason. I would just use SmallVector<unsigned> in this case.

812

Could be constexpr?

[abidmalikwaterloo]

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

26

My mistake. Will put it back.

171

I will put the space. Accidentally removed.

807

SmallVector<int> should be fine.

812

OK

[abidmalikwaterloo]

Add spaces which were accidently removed OpenMPDialect.cpp
Change SmallVector<int,3> to SmallVector<int>
Change const int to constexpr int
Made changes as per reviewer's comments

[kiranchandramohan]

Looks mostly OK. A few more comments.

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

279

Nit: Spelling : eliminates

293

I think this builder is doing the same thing that the default builder does. So is it required?

Also, the if_expr, device and thread_limit are operands and not attributes. So I think their type should be Value and not IntegerAttr.

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

794

Nit: ssa-id -> ssa-id-and-type ?

864–865

I guess @clementval's SmallVector comment about not specifying the size applies here as well.

[abidmalikwaterloo]

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

794

Got it.

864–865

Ok

[abidmalikwaterloo]

Did the code alignments and corrected the codes as per reviewers' comments.

[clementval]

There are still some comments not addressed so far. I guess once you have addressed all the comments we can move forward,

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

370

Seems like unrelated change.

[abidmalikwaterloo]

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

293

No expert in "TableGen". We should remove it if this is the case.

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

370

I did it in line with the comments at line# 866. Though forgot to correct that.

[clementval]

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

293

Yeah I think you can remove it. Seems like you don't need a specific builder for the time being.

[abidmalikwaterloo]

remove the builder as per suggested by the reviewer

[shraiysh]

Hi @abidmalikwaterloo Thanks for this patch. There are a couple of issues with the styling which is causing the builds to fail. For example trailing spaces, 80-character line wraps and maybe some indentation. I have a few comments.

In the OpenMP 5.0 standard, there is no thread_limit clause for the target construct. Is it necessary?

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

295

These functions are used only in the parser, and they can be removed - the strings can directly be used. (You will not have to use them too once you use the parseClauses function).

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

816

This code can now be condensed using the parseClauses function in the latest version of the file (like SectionsOp and ParallelOp).

[clementval]

Is there still any development on this?

[abidmalikwaterloo]

Will try to finalize this week.

[peixin]

In D103723#3169181, @shraiysh wrote:

In the OpenMP 5.0 standard, there is no thread_limit clause for the target construct. Is it necessary?

@shraiysh I think this patch is following OpenMP 5.1 standard. In line 261 of OpenMPOps.td, section 2.14.5 is refered and it is the section in openmp 5.1 standard.

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

264

Should there be one space between "target", and [AttrSizedOperandSegments]?

287

The device modifier is missed. Will it be implemented later?

[abidmalikwaterloo]

Are there any other issues with the patch? I will clean it and submit it?

[kiranchandramohan]

Unfortunately, there have been several developments since we last looked at this patch. This includes using assembly format for the printer/parser instead of the custom printTargetOp/parseTargetOp. See the following for reference.
https://github.com/llvm/llvm-project/blob/6f73bd781305266a747055875ce8352e5a36c809/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td#L115

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

286

You can use I1 here instead of AnyInteger.

[abidmalikwaterloo]

In D103723#3472382, @kiranchandramohan wrote:

Unfortunately, there have been several developments since we last looked at this patch. This includes using assembly format for the printer/parser instead of the custom printTargetOp/parseTargetOp. See the following for reference.
https://github.com/llvm/llvm-project/blob/6f73bd781305266a747055875ce8352e5a36c809/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td#L115

Does this mean we need to resubmit a new patch for TargetOp?

[abidmalikwaterloo]

This has been taken care of. I think need to abandon this patch?

[abidmalikwaterloo]

This has been taken care of. I think need to abandon this patch?

[kiranchandramohan]

You can update this patch itself here.

[abidmalikwaterloo]

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

816

Should we need to do it? I can come to it later for another

[abidmalikwaterloo]

This patch need rebasing and some formating.

[shraiysh]

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

816

Not anymore. We should use the assemblyformat now and eliminate custom parsers entirely.

[abidmalikwaterloo]

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

816

I see the assembly format for the target operation. Does this mean the patch is redundant?

Abid_ReadME.txt

-Start date : May 4th, 2021
----------------------------
-
-How to install LLVM on Mac:
-1) There is a problem in installing with libcxxabi and libcxx. So, on my Mac, I  installed it with out these two projects. However, I need to understand why it was not building on Mac with the LLVM. It works on Shilie' MacBook
-
-2) The configuration that works for me:
-
-cmake -G "Ninja" -DCMAKE_BUILD_TYPE=Release -DLLVM_INCLUDE_TESTS=OFF ../llvm    -DLLVM_ENABLE_PROJECTS="mlir;clang;clang-tools-extra;lld;openmp" -DLLVM_TARGETS_TO_BUILD="X86;NVPTX;AMDGPU"    -DCMAKE_BUILD_TYPE=Release    -DLLVM_ENABLE_ASSERTIONS=ON  -DCMAKE_INSTALL_PREFIX=/Users/abidmalik/Programming/MLIR_target/llvm-project/myInstall -DCMAKE_CXX_FLAGS="-I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/ -std=c++14"
-
-ninja all for builfing
-ninja install for installing
-
-I am not sure about the -DCMAKE_CXX_FLAGS="-I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/ -std=c++14" flag as I played with it to overcome the problem with libcxxabi and libcxx. I will try to build that same with this flag.
-
-The build works without the aformentioned flag as well.
-
-Python3 is required
-if there is a problem on Mac about the python instalation check the following website
-https://mattbanderson.com/so-you-hosed-your-mac-os-python-install/
-
-
-May 5th, 2021
---------------
--DCMAKE_CXX_FLAGS="-I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/ -std=c++14"
-
-Python 3.* is needed.
-On my Mac, the default was Python 2.7. You can alias python = 'Python3' to make the python3 as your default.
-** Still need to check why libcxxabi and libcxx could not build on my Mac. However, we donot need them for my MLIR OpenMP dialect work
-
-May 7th, 2021
---------------
-
-When I cmake: The following error was raised:
-
-
-No rule to make target /usr/lib/libz.dylib', needed by `lib/libzip.5.3.dylib'.  Stop.
-
-The solution was found on the following link
-
-https://stackoverflow.com/questions/62719000/can-not-find-libz-dylib-when-build-libzip-by-homebrew-in-mac-os-10-16-big-sur
-
-I reinstall the libbz with brew and also use the DZLIB_LIBRARY_RELEASE flag to the library. It works and error was not there.
-
-
-Also an interesting link for building LLVM
-
-https://lists.llvm.org/pipermail/llvm-dev/2019-September/135333.html
-
-May 8th, 2021
-==============
-
-For LLVM_9 , the following configuration work
-
-cmake -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Release -DLLVM_INCLUDE_TESTS=OFF ../llvm    -DLLVM_ENABLE_PROJECTS="mlir;clang;clang-tools-extra;lld;openmp" -DLLVM_TARGETS_TO_BUILD="X86;NVPTX;AMDGPU"    -DCMAKE_BUILD_TYPE=Release    -DLLVM_ENABLE_ASSERTIONS=ON  -DCMAKE_INSTALL_PREFIX=/Users/abidmalik/Programming/LLVM_9/llvm-project/INSATLL -DCMAKE_CXX_FLAGS="-I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/ -std=c++14"
-
-
-I am not pointing anything. for libz.dylib which is needed for LLVM_11.
-
-Question: Any reason why? Is it related to the LLVM version? Need to check with the LLVM community.
-
-
-
-May 17th, 2021
-===============
-
-Build the MLIR with
-
-
-cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DLLVM_INCLUDE_TESTS=ON ../llvm    -DLLVM_ENABLE_PROJECTS=mlir -DLLVM_TARGETS_TO_BUILD="X86;NVPTX;AMDGPU"    -DCMAKE_BUILD_TYPE=Release    -DLLVM_ENABLE_ASSERTIONS=ON  -DCMAKE_INSTALL_PREFIX=/Users/abidmalik/Programming/MLIR_target/llvm-project/INSTALL/ -DCMAKE_CXX_FLAGS="-I/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1/ -std=c++14"
-
-
-and then
-
-cmake --build . --target check-mlir
-
-Now all tests are passing
-
-
-May 18th, 2021
-===============
-
-FYI,
-
-TableGen syntax sometimes can be obscure; reading the generated content can be a very helpful way to understand and debug issues. 
-To build mlir-tblgen, run cmake --build . --target mlir-tblgen in your build directory and find the mlir-tblgen binary in the bin/ subdirectory. 
-All the supported generators can be found via mlir-tblgen --help. For example, --gen-op-decls and --gen-op-defs as explained in Generated C++ code .

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

def ParallelOp : OpenMP_Op<"parallel", [AttrSizedOperandSegments]> {
 
   let regions = (region AnyRegion:$region);
 
   let builders = [
     OpBuilder<(ins CArg<"ArrayRef<NamedAttribute>", "{}">:$attributes)>
   ];
   let parser = [{ return parseParallelOp(parser, result); }];
   let printer = [{ return printParallelOp(p, *this); }];
-  let verifier = [{ return ::verifyParallelOp(*this); }];
+  let verifier  = [{ return verifyParallelOp(*this); }];
 }
 
 def TerminatorOp : OpenMP_Op<"terminator", [Terminator]> {
   let summary = "terminator for OpenMP regions";
   let description = [{
     A terminator operation for regions that appear in the body of OpenMP
     operation.  These regions are not expected to return any value so the
     terminator takes no operands. The terminator op returns control to the
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<AnyInteger>:$if_expr,
+                       Optional<AnyInteger>:$device,
+                       Optional<AnyInteger>:$thread_limit,
+                       UnitAttr:$nowait);
+
+  let regions = (region AnyRegion:$region);
+  
+  let builders = [
+       OpBuilder<(ins "IntegerAttr":$if_expr,
+                       "IntegerAttr":$device,
+                       "IntegerAttr":$thread_limit,
+                       "UnitAttr":$nowait)>
+  ];
+    let parser   = [{ return parseTargetOp(parser, result); }];
+    let printer  = [{ return printTargetOp(p, *this); }];
+}
+
 
 //===----------------------------------------------------------------------===//
 // 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/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp

 
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
 #include <cstddef>
 
 #include "mlir/Dialect/OpenMP/OpenMPOpsEnums.cpp.inc"
-
+using namespace std;
 using namespace mlir;
 using namespace mlir::omp;
 
 void OpenMPDialect::initialize() {
   addOperations<
 #define GET_OP_LIST
 #include "mlir/Dialect/OpenMP/OpenMPOps.cpp.inc"
       >();
static void printParallelOp(OpAsmPrinter &p, ParallelOp op) {
   printDataVars("copyin", op.copyin_vars());
   printAllocateAndAllocator(op.allocate_vars(), op.allocators_vars());
 
   if (auto def = op.default_val())
     p << " default(" << def->drop_front(3) << ")";
 
   if (auto bind = op.proc_bind_val())
     p << " proc_bind(" << bind << ")";
-
   p.printRegion(op.getRegion());
 }
 
 /// Emit an error if the same clause is present more than once on an operation.
 static ParseResult allowedOnce(OpAsmParser &parser, StringRef clause,
                                StringRef operation) {
   return parser.emitError(parser.getNameLoc())
          << " at most one " << clause << " clause can appear on the "
if (segments[allocatorPos] &&
                              result.operands))
     return failure();
 
   result.addAttribute("operand_segment_sizes",
                       parser.getBuilder().getI32VectorAttr(segments));
 
   Region *body = result.addRegion();
   SmallVector<OpAsmParser::OperandType, 4> regionArgs;
   SmallVector<Type, 4> regionArgTypes;
   if (parser.parseRegion(*body, regionArgs, regionArgTypes))
     return failure();
   return success();
 }
 
 /// linear ::= `linear` `(` linear-list `)`
 /// linear-list := linear-val | linear-val linear-list
 /// linear-val := ssa-id-and-type `=` ssa-id-and-type
void WsLoopOp::build(OpBuilder &builder, OperationState &result,
   if (buildBody) {
     OpBuilder::InsertionGuard guard(builder);
     unsigned numIVs = steps.size();
     SmallVector<Type, 8> argTypes(numIVs, steps.getType().front());
     builder.createBlock(bodyRegion, {}, argTypes);
   }
 }
 
+//===----------------------------------------------------------------------===//
+// TargetOp
+//===----------------------------------------------------------------------===//
+
+void TargetOp::build(OpBuilder &builder, OperationState &state,
+                     IntegerAttr if_expr, IntegerAttr device,
+                     IntegerAttr thread_limit, UnitAttr nowait) {
+  if (if_expr)
+    state.addAttribute("if_expr", if_expr);
+  if (device)
+    state.addAttribute("device", device);
+  if (thread_limit)
+    state.addAttribute("thread_limit", thread_limit);
+  if (nowait)
+    state.addAttribute("nowait", nowait);
+}
+
+/// Parse a target operation.
+///
+/// operation ::= `omp.target' clause-list
+/// clause-list ::= clause | clause clause-list
+/// clause ::= if | thread_limit | device | nowait
+/// if ::= `if` `(` ssa-id `)`
+/// thread_limit ::= `thread_limit` `(` ssa-id-and-type`)`
+/// device ::= `device` `(`ssa-id-and-type`)`
+/// nowait ::= `nowait`
+/// TODO: other clauses
+/// TODO: Will update as soon as the other clauses will be finalized
+
+static ParseResult parseTargetOp(OpAsmParser &parser, OperationState &result) {
+
+  std::pair<OpAsmParser::OperandType, Type> ifCond;
+  std::pair<OpAsmParser::OperandType, Type> deviceNum;
+  std::pair<OpAsmParser::OperandType, Type> threadLimit;
+
+  std::array<int, 3> segments{0, 0, 0};
+
+  StringRef keyword;
+  const StringRef opName = result.name.getStringRef();
+
+  const int ifClausePos = 0;
+  const int deviceNumPos = 1;
+  const int threadLimitPos = 2;
+
+  while (succeeded(parser.parseOptionalKeyword(&keyword))) {
+    if (keyword == "if") {
+      // Fail if there was already another if condition.
+      if (segments[ifClausePos])
+        return allowedOnce(parser, "if", opName);
+      if (parser.parseLParen() || parser.parseOperand(ifCond.first) ||
+          parser.parseColonType(ifCond.second) || parser.parseRParen())
+        return failure();
+      segments[ifClausePos] = 1;
+    } else if (keyword == "thread_limit") {
+      // Fail if there was already another thread_limit clause.
+      if (segments[threadLimitPos])
+        return allowedOnce(parser, "thread_limit", opName);
+      if (parser.parseLParen() || parser.parseOperand(threadLimit.first) ||
+          parser.parseColonType(threadLimit.second) || parser.parseRParen())
+        return failure();
+      segments[threadLimitPos] = 1;
+    } else if (keyword == "device") {
+      // Fail if there was already another device clause.
+      if (segments[deviceNumPos])
+        return allowedOnce(parser, "device", opName);
+      if (parser.parseLParen() || parser.parseOperand(deviceNum.first) ||
+          parser.parseColonType(deviceNum.second) || parser.parseRParen())
+        return failure();
+      segments[deviceNumPos] = 1;
+    } else if (keyword == "nowait") {
+      auto attr = UnitAttr::get(parser.getBuilder().getContext());
+      result.addAttribute("nowait", attr);
+    }
+  }
+
+  if (segments[ifClausePos] &&
+      parser.resolveOperand(ifCond.first, ifCond.second, result.operands))
+    return failure();
+
+  if (segments[threadLimitPos] &&
+      parser.resolveOperand(threadLimit.first, threadLimit.second,
+                            result.operands))
+    return failure();
+
+  if (segments[deviceNumPos] &&
+      parser.resolveOperand(deviceNum.first, deviceNum.second, result.operands))
+    return failure();
+
+  result.addAttribute("operand_segment_sizes",
+                      parser.getBuilder().getI32VectorAttr(segments));
+
+  Region *body = result.addRegion();
+  SmallVector<OpAsmParser::OperandType, 4> regionArgs;
+  SmallVector<Type, 4> regionArgTypes;
+  if (parser.parseRegion(*body, regionArgs, regionArgTypes))
+    return failure();
+
+  return success();
+}
+// Print if, device, thread_limit, and nowait
+static void printTargetOp(OpAsmPrinter &p, TargetOp op) {
+  p << "omp.target";
+  if (auto ifCond = op.if_expr())
+    p << " if (" << ifCond << " : " << ifCond.getType() << ")";
+  if (auto threads = op.thread_limit())
+    p << " thread_limit (" << threads << " : " << threads.getType() << ")";
+  if (auto devices = op.device())
+    p << " device (" << devices << " : " << devices.getType() << ")";
+  if (op.nowait())
+    p << " nowait";
+
+  p.printRegion(op.getRegion());
+}
+
 #define GET_OP_CLASSES
 #include "mlir/Dialect/OpenMP/OpenMPOps.cpp.inc"

mlir/test/Dialect/OpenMP/ops.mlir

"omp.parallel" (%data_var, %data_var, %data_var, %data_var, %data_var, %data_var, %data_var) ({
     omp.terminator
   }) {operand_segment_sizes = dense<[0,0,1,2,1,1,1,1]> : vector<8xi32>} : (memref<i32>, memref<i32>, memref<i32>, memref<i32>, memref<i32>, memref<i32>, memref<i32>) -> ()
 
   return
 }
 
 func @omp_parallel_pretty(%data_var : memref<i32>, %if_cond : i1, %num_threads : si32, %allocator : si32) -> () {
  // CHECK: omp.parallel
   omp.parallel {
   omp.terminator
  }
 
  // CHECK: omp.parallel num_threads(%{{.*}} : si32)
  omp.parallel num_threads(%num_threads : si32) {
    omp.terminator
  }
 
func @omp_wsloop(%lb : index, %ub : index, %step : index, %data_var : memref<i32>, %linear_var : i32, %chunk_var : i32) -> () {
 
   // CHECK: omp.wsloop (%{{.*}}) : index = (%{{.*}}) to (%{{.*}}) step (%{{.*}}) private(%{{.*}} : memref<i32>) firstprivate(%{{.*}} : memref<i32>) lastprivate(%{{.*}} : memref<i32>) linear(%{{.*}} = %{{.*}} : memref<i32>) schedule(dynamic = %{{.*}}) collapse(3) ordered(2)
   "omp.wsloop" (%lb, %ub, %step, %data_var, %data_var, %data_var, %data_var, %linear_var, %chunk_var) ({
     ^bb0(%iv: index):
       omp.yield
   }) {operand_segment_sizes = dense<[1,1,1,1,1,1,1,1,1]> : vector<9xi32>, schedule_val = "Dynamic", collapse_val = 3, ordered_val = 2} :
     (index, index, index, memref<i32>, memref<i32>, memref<i32>, memref<i32>, i32, i32) -> ()
 
+
   // CHECK: omp.wsloop (%{{.*}}) : index = (%{{.*}}) to (%{{.*}}) step (%{{.*}}) private(%{{.*}} : memref<i32>) schedule(auto) nowait
   "omp.wsloop" (%lb, %ub, %step, %data_var) ({
     ^bb0(%iv: index):
       omp.yield
   }) {operand_segment_sizes = dense<[1,1,1,1,0,0,0,0,0]> : vector<9xi32>, nowait, schedule_val = "Auto"} :
     (index, index, index, memref<i32>) -> ()
     
+    
   return
 }
 
 // CHECK-LABEL: omp_wsloop_pretty
 func @omp_wsloop_pretty(%lb : index, %ub : index, %step : index,
                  %data_var : memref<i32>, %linear_var : i32, %chunk_var : i32) -> () {
 
   // CHECK: omp.wsloop (%{{.*}}) : index = (%{{.*}}) to (%{{.*}}) step (%{{.*}}) private(%{{.*}} : memref<i32>)
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 : si32, %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 } : ( si32, si32, si32  ) -> ()
+
+    // CHECK: omp.barrier
+    omp.barrier
+
+    return
+}
+
+
+
+func @omp_target_pretty( %if_cond : si32, %device : si32, %thread_limit : si32) -> () {
+  // CHECK: omp.target
+  omp.target {
+  omp.terminator
+  }
+
+  // CHECK: omp.target device
+  omp.target device(%device : si32) {
+  omp.terminator
+  }   
+
+  // CHECK: omp.barrier
+  omp.barrier
+
+ return
+}