[Mlir-commits] [mlir] [flang][MLIR][OpenMP] Add support for `target update` directive. (PR #75047)

[Kareem Ergawy]

================
@@ -915,6 +925,33 @@ static LogicalResult verifyMapClause(Operation *op, OperandRange mapOperands) {
       if (isa<ExitDataOp>(op) && to)
         return emitError(op->getLoc(),
                          "from, release and delete map types are permitted");
+
+      if (isa<UpdateDataOp>(op)) {
+        if (del) {
+          return emitError(op->getLoc(),
+                           "at least one of to or from map types must be "
+                           "specified, other map types are not permitted");
+        }
+
+        if (to & from) {
+          return emitError(
+              op->getLoc(),
+              "either to or from map types can be specified, not both");
+        }
----------------
ergawy wrote:

> However, I'm not sure if that should be an enforced rule.

I am not trying to treat this as in enforced rule. What I meant is that if you have input where the **same value** is used in both `to` and `from`  `map_info` ops, we should error out. For example, see the following IR:
```
func.func @omp_target_update_invalid_motion_modifier_4(%map1 : memref<?xi32>) {
  %mapv = omp.map_info var_ptr(%map1 : memref<?xi32>, tensor<?xi32>) map_clauses(to) capture(ByRef) -> memref<?xi32> {name = ""}
  %mapv2 = omp.map_info var_ptr(%map1 : memref<?xi32>, tensor<?xi32>) map_clauses(from) capture(ByRef) -> memref<?xi32> {name = ""}

  // expected-error @below {{either to or from map types can be specified, not both}}
  omp.target_update_data motion_entries(%mapv, %mapv2 : memref<?xi32>, memref<?xi32>)
  return
}
```
According to the spec: `A list item can only appear in a to or from clause, but not in both.` ([see](https://www.openmp.org/spec-html/5.1/openmpsu69.html#x97-1070002.14.6)).

For the equivalent Fortran code:
```
subroutine foo
   integer :: a(1024)
   integer :: b(1024)
   logical :: i
   !$omp target update from(a) to(a, b) nowait
end subroutine foo
```
Here is what `flang-new` currently produces:
```
error: Semantic errors in /tmp/test.f90
/tmp/test.f90:17:29: because: 'a' appears in the FROM clause.
     !$omp target update from(a) to(a, b) nowait
                              ^
/tmp/test.f90:17:35: error: A list item ('a') can only appear in a TO or FROM clause, but not in both.
     !$omp target update from(a) to(a, b) nowait
                                    ^
/tmp/test.f90:17:35: because: 'a' appears in the TO clause.
     !$omp target update from(a) to(a, b) nowait
                                    ^
```
which is the correct behavior that we still need to reflect in the IR.

> Otherwise, how to you lower smth like `!$omp target update to(a,b,c) from(x,y,z)`

That should be totally fine. Since each `map_info` op models motion/mapping information for a **single** value, we will have one `map_info` op for each one of these variables and we are currently able to handle it fine.

Let me know if I misunderstood either of you :).

https://github.com/llvm/llvm-project/pull/75047