[mlir][OpenMP] omp.parallel side effects

[shraiysh]

Summary

Add NoSideEffects trait to omp.parallel operation.

[ftynse]

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

55

This looks contradictory to have both recursive side effects and no side effects.

[shraiysh]

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

55

Hmm, the way I understood this would work is -

• omp.parallel in itself does not add any side effects. (NoSideEffect)
• While calculating side effects for a particular omp.parallel operation, include the side effects on the containing region. (Recursive Side effects).

That means omp.parallel only has side effects when the operations under it have side effects.

I got this understanding from one of River's replies with lambda in https://reviews.llvm.org/D74439. (Phabricator doesn't let me copy link to comment, but its on the file mlir/include/mlir/IR/OpDefinition.h)Please let me know if I have misunderstood something here.

[shraiysh]

Removed NoSideEffect.

[ftynse]

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

55

I guessed how you wanted to interpret that mix of traits, but I still think it is highly confusing to denote it that way. Note that dropping the trait gives you the same behavior and all your tests still pass.

River's comment on that diff needs slight clarification. I suppose it was meant to illustrate the difference between a loop-like operation that would have RecursiveSideEffects and a lambda-like operation that itself would not.

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

 
 //===----------------------------------------------------------------------===//
 // 2.6 parallel Construct
 //===----------------------------------------------------------------------===//
 
 def ParallelOp : OpenMP_Op<"parallel", [
                  AutomaticAllocationScope, AttrSizedOperandSegments,
                  DeclareOpInterfaceMethods<OutlineableOpenMPOpInterface>,
-                 RecursiveSideEffects, ReductionClauseInterface]> {
+                 RecursiveSideEffects, NoSideEffect, ReductionClauseInterface]> {
   let summary = "parallel construct";
   let description = [{
     The parallel construct includes a region of code which is to be executed
     by a team of threads.
 
     The optional $if_expr_var parameter specifies a boolean result of a
     conditional check. If this value is 1 or is not provided then the parallel
     region runs as normal, if it is 0 then the parallel region is executed with
let extraClassDeclaration = [{
     /// Returns the reduction variables
     SmallVector<Value> getReductionVars() {
       return SmallVector<Value>(reduction_vars().begin(),
                                 reduction_vars().end());
     }
   }];
 }
 
-def TerminatorOp : OpenMP_Op<"terminator", [Terminator]> {
+def TerminatorOp : OpenMP_Op<"terminator", [Terminator, NoSideEffect]> {
   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
     enclosing op.
   }];
 

mlir/test/Dialect/OpenMP/canonicalize.mlir

^bb0(%xval: i32):
     omp.yield(%newval: i32)
   }
   return
 }
 
 // CHECK-LABEL: func.func @update_unnecessary_computations
 // CHECK-NOT: omp.atomic.update
 // CHECK: omp.atomic.write
+
+// -----
+
+// CHECK-LABEL: func.func @parallel_empty
+func.func @parallel_empty() {
+  omp.parallel {}
+  return
+}
+
+// CHECK-NOT: omp.parallel
+
+// -----
+
+// CHECK-LABEL: func.func @parallel_only_terminator
+func.func @parallel_only_terminator() {
+  omp.parallel {
+    omp.terminator
+  }
+  return
+}
+
+// CHECK-NOT: omp.parallel
+// CHECK-NOT: omp.terminator
+
+// -----
+
+// CHECK-LABEL: func.func @parallel_no_side_effects
+func.func @parallel_no_side_effects(%a: i32, %b: i32) {
+  omp.parallel {
+    %x = arith.addi %a, %b : i32
+    omp.terminator
+  }
+  return
+}
+
+// CHECK-NOT: omp.parallel
+// CHECK-NOT: omp.terminator
+
+// -----
+
+// CHECK-LABEL: func.func @parallel_maybe_side_effects
+func.func @parallel_maybe_side_effects(%a: i32, %b: i32) {
+  omp.parallel {
+    func.call @foo() : () -> ()
+    omp.terminator
+  }
+  return
+}
+
+func.func private @foo() -> () 
+
+// CHECK: omp.parallel
+// CHECK: func.call @foo() : () -> ()
+// CHECK: omp.terminator