[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?

790–801

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

884

Nit: Please add an empty line after this function.

mlir/test/Dialect/OpenMP/ops.mlir

93

Nit: Accidental change?

166–167

Nit: some formatting accidentally added?

284–297

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

304

Nit: One or two spaces at the beginning here.

308

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

309

Nit: One or two spaces at the beginning here.

[clementval]

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

790–801

+1

809

810

nit: added spaces just to fit with previous style

831

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

857

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.

810

OK.

831

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

857

OK

884

OK

mlir/test/Dialect/OpenMP/ops.mlir

93

Yes.. Will fix it

166–167

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

808

Why not using SmallVector?

[abidmalikwaterloo]

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

808

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?

808

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.

813

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.

808

SmallVector<int> should be fine.

813

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

795

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

865–866

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

[abidmalikwaterloo]

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

795

Got it.

865–866

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

371

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

371

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

817

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

817

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

817

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

[abidmalikwaterloo]

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

817

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 eliminates 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 extraClassDeclaration = [{
+    static StringRef getIfKeyword() { return "if";}
+    static StringRef getThreadLimitKeyword() { return "thread_limit";}
+    static StringRef getDeviceKeyword() { return "device";}
+    static StringRef getNoWaitKeyword() { return "nowait";}
+    // TODO:  the remaining optional clauses
+    }];
+
+  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 mlir;
 using namespace mlir::omp;
 
 void OpenMPDialect::initialize() {
   addOperations<
 #define GET_OP_LIST
 #include "mlir/Dialect/OpenMP/OpenMPOps.cpp.inc"
       >();

 }
 
 /// Parse an allocate clause with allocators and a list of operands with types.
 ///
 /// operand-and-type-list ::= `(` allocate-operand-list `)`
 /// allocate-operand-list :: = allocate-operand |
 ///                            allocator-operand `,` allocate-operand-list
 /// allocate-operand :: = ssa-id-and-type -> ssa-id-and-type
-/// ssa-id-and-type ::= ssa-id `:` type
+/// ssa-id-and-type ::= ssa-id-and-type `:` type
 static ParseResult parseAllocateAndAllocator(
     OpAsmParser &parser,
     SmallVectorImpl<OpAsmParser::OperandType> &operandsAllocate,
     SmallVectorImpl<Type> &typesAllocate,
     SmallVectorImpl<OpAsmParser::OperandType> &operandsAllocator,
     SmallVectorImpl<Type> &typesAllocator) {
   if (parser.parseLParen())
     return failure();
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;
+  SmallVector<Type> 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
+//===----------------------------------------------------------------------===//
+
+/// 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;
+
+  SmallVector<int> segments{0, 0, 0};
+
+  StringRef keyword;
+  const StringRef opName = result.name.getStringRef();
+
+  constexpr int ifClausePos = 0;
+  constexpr int deviceNumPos = 1;
+  constexpr int threadLimitPos = 2;
+
+  while (succeeded(parser.parseOptionalKeyword(&keyword))) {
+    if (keyword == TargetOp::getIfKeyword()) {
+      // Fail if there was already another if condition.
+      if (segments[ifClausePos])
+        return allowedOnce(parser, TargetOp::getIfKeyword(), opName);
+      if (parser.parseLParen() || parser.parseOperand(ifCond.first) ||
+          parser.parseColonType(ifCond.second) || parser.parseRParen())
+        return failure();
+      segments[ifClausePos] = 1;
+    } else if (keyword == TargetOp::getThreadLimitKeyword()) {
+      // Fail if there was already another thread_limit clause.
+      if (segments[threadLimitPos])
+        return allowedOnce(parser, TargetOp::getThreadLimitKeyword(), opName);
+      if (parser.parseLParen() || parser.parseOperand(threadLimit.first) ||
+          parser.parseColonType(threadLimit.second) || parser.parseRParen())
+        return failure();
+      segments[threadLimitPos] = 1;
+    } else if (keyword == TargetOp::getDeviceKeyword()) {
+      // Fail if there was already another device clause.
+      if (segments[deviceNumPos])
+        return allowedOnce(parser, TargetOp::getDeviceKeyword(), opName);
+      if (parser.parseLParen() || parser.parseOperand(deviceNum.first) ||
+          parser.parseColonType(deviceNum.second) || parser.parseRParen())
+        return failure();
+      segments[deviceNumPos] = 1;
+    } else if (keyword == TargetOp::getNoWaitKeyword()) {
+      auto attr = UnitAttr::get(parser.getBuilder().getContext());
+      result.addAttribute(TargetOp::getNoWaitKeyword(), 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
  }
 
  // CHECK: omp.parallel allocate(%{{.*}} : memref<i32> -> %{{.*}} : memref<i32>)
"omp.wsloop" (%lb, %ub, %step, %data_var, %data_var, %data_var, %data_var, %linear_var, %chunk_var) ({
     (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 (%iv) : index = (%lb) to (%ub) step (%step) private(%data_var : memref<i32>) collapse(2) ordered(2) {
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_pretty
+func @omp_target_pretty( %if_cond : i1, %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.target if
+  omp.target if(%if_cond: i1) {
+   omp.terminator
+  }   
+  
+  // CHECK: omp.target nowait
+  omp.target nowait {
+   omp.terminator 
+  }
+
+  // CHECK: omp.target thread_limit
+  omp.target thread_limit(%thread_limit : si32) {
+   omp.terminator
+  } 
+
+  // CHECK: omp.barrier
+  omp.barrier
+
+  return
+}