[Mlir-commits] [mlir] [MLIR][ODS] Require the value after oilist keyword (PR #229334)

[llvmlistbot at llvm.org]

================
@@ -3002,8 +3002,16 @@ func.func @omp_target_update_invalid_motion_type(%map1 : memref<?xi32>) {
 
 // -----
 
-func.func @omp_target_map_must_specify_both_var_ptr_ptr_args(%arg : memref<?xi32>) {
+func.func @omp_map_var_ptr_ptr_missing_type(%arg : memref<?xi32>) {
+  // expected-error @below {{expected attribute value}}
   %map1 = omp.map.info var_ptr(%arg : memref<?xi32>, tensor<?xi32>) map_clauses(to) capture(ByRef) var_ptr_ptr(%arg : memref<?xi32>, ) name("") -> memref<?xi32>
+  return
+}
+
+// -----
+
+func.func @omp_target_map_must_specify_both_var_ptr_ptr_args(%arg : memref<?xi32>) {
+  %map1 = "omp.map.info"(%arg, %arg) <{map_capture_type = #omp.variable_capture_kind<ByRef>, map_type = #omp.clause_map_flags<to>, operandSegmentSizes = array<i32: 1, 1, 0, 0>, var_ptr_type = tensor<?xi32>}> : (memref<?xi32>, memref<?xi32>) -> memref<?xi32>
----------------
agozillon wrote:

Yeah, a large transition isn't going to be particularly feasible in the near future, that's a fairly significant and painful overhaul, the dialect changes would likely be the easy part. It would be nice to do long term as we sort of just picked up the map operation in the state it was 4 years ago and ran with it without thinking too hard about it (naivety of being new to MLIR).

In the meantime I would suggest removing the operation specific verification to adhere closer to the original design intent of the verifier. It's useful but not exactly a critical piece of the verification process, the runtime (that we have today at least) will ignore the map types that aren't relevant for the directives for the most part. There's much easier ways to shoot yourself in the foot that we can't/don't verify at the moment. 

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