[PATCH] D123828: [mlir][OpenMP] Add omp.cancel and omp.cancellationpoint.

[Raghu via Phabricator via llvm-commits]

raghavendhra requested review of this revision.
raghavendhra added inline comments.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:1808
 CHECK_SIMPLE_CLAUSE(Compare, OMPC_compare)
+CHECK_SIMPLE_CLAUSE(CancelConstruct, OMPC_cancel_construct)
 
----------------
kiranchandramohan wrote:
> On a first look, it looks like we have a construct here instead of a clause. Would `CancelConstructType` be a better name? 
If I change CancelConstruct to CancelConstructType, I am getting

llvm-project/flang/lib/Semantics/check-omp-structure.cpp:1808:21: error: ‘CancelConstructType’ in ‘struct Fortran::parser::OmpClause’ does not name a type
 1808 | CHECK_SIMPLE_CLAUSE(CancelConstructType, OMPC_cancel_construct)

Also, it looks like code in flang has been using CancelConstruct in multiple places.


================
Comment at: mlir/test/Dialect/OpenMP/ops.mlir:1014-1037
+func @omp_cancel(%if_cond : i1) -> () {
+  // Test with optional operand; if_expr.
+  // CHECK: omp.cancel cancel_construct(parallel) if(%{{.*}})
+  omp.cancel cancel_construct(parallel) if(%if_cond)
+  // CHECK: omp.cancel cancel_construct(loop)
+  omp.cancel cancel_construct(loop)
+  // CHECK: omp.cancel cancel_construct(sections)
----------------
kiranchandramohan wrote:
> There are a few restrictions regarding closely nested constructs. Some of the above might not be valid usage. Could you try implementing them? 
Something like this?


```
func @omp_cancel(%if_cond : i1) -> () {
  // Test with optional operand; if_expr.
  omp.parallel {
    // CHECK: omp.cancel cancel_construct(parallel) if(%{{.*}})
    omp.cancel cancel_construct(parallel) if(%if_cond)
    // CHECK: omp.terminator
    omp.terminator
  }
  return
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123828/new/

https://reviews.llvm.org/D123828