Review comments

Author: tblah

mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp

@@ -162,7 +162,7 @@ static LogicalResult checkImplementationStatus(Operation &op) {
     omp::ClauseCancellationConstructType cancelledDirective =
         op.getCancelDirective();
     if (cancelledDirective != omp::ClauseCancellationConstructType::Parallel)
-      result = todo("cancel directive");
+      result = todo("cancel directive construct type not yet supported");
   };
   auto checkDepend = [&todo](auto op, LogicalResult &result) {
     if (!op.getDependVars().empty() || op.getDependKinds())
@@ -1591,15 +1591,13 @@ cleanupPrivateVars(llvm::IRBuilderBase &builder,
 static bool constructIsCancellable(Operation *op) {
   // omp.cancel must be "closely nested" so it will be visible and not inside of
   // funcion calls. This is enforced by the verifier.
-  bool containsCancel = false;
-  op->walk([&containsCancel](Operation *child) {
-    if (mlir::isa<omp::CancelOp>(child)) {
-      containsCancel = true;
-      return WalkResult::interrupt();
-    }
-    return WalkResult::advance();
-  });
-  return containsCancel;
+  return op
+      ->walk([](Operation *child) {
+        if (mlir::isa<omp::CancelOp>(child))
+          return WalkResult::interrupt();
+        return WalkResult::advance();
+      })
+      .wasInterrupted();
 }
 
 static LogicalResult

mlir/test/Target/LLVMIR/openmp-todo.mlir

@@ -31,7 +31,7 @@ llvm.func @cancel_wsloop(%lb : i32, %ub : i32, %step: i32) {
   omp.wsloop {
     // expected-error@below {{LLVM Translation failed for operation: omp.loop_nest}}
     omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) {
-      // expected-error@below {{not yet implemented: Unhandled clause cancel directive in omp.cancel operation}}
+      // expected-error@below {{not yet implemented: Unhandled clause cancel directive construct type not yet supported in omp.cancel operation}}
       // expected-error@below {{LLVM Translation failed for operation: omp.cancel}}
       omp.cancel cancellation_construct_type(loop)
       omp.yield
@@ -46,7 +46,7 @@ llvm.func @cancel_sections() {
   // expected-error@below {{LLVM Translation failed for operation: omp.sections}}
   omp.sections {
     omp.section {
-      // expected-error@below {{not yet implemented: Unhandled clause cancel directive in omp.cancel operation}}
+      // expected-error@below {{not yet implemented: Unhandled clause cancel directive construct type not yet supported in omp.cancel operation}}
       // expected-error@below {{LLVM Translation failed for operation: omp.cancel}}
       omp.cancel cancellation_construct_type(sections)
       omp.terminator
@@ -63,7 +63,7 @@ llvm.func @cancel_taskgroup() {
   omp.taskgroup {
     // expected-error@below {{LLVM Translation failed for operation: omp.task}}
     omp.task {
-      // expected-error@below {{not yet implemented: Unhandled clause cancel directive in omp.cancel operation}}
+      // expected-error@below {{not yet implemented: Unhandled clause cancel directive construct type not yet supported in omp.cancel operation}}
       // expected-error@below {{LLVM Translation failed for operation: omp.cancel}}
       omp.cancel cancellation_construct_type(taskgroup)
       omp.terminator