[Mlir-commits] [flang] [mlir] [Flang][OpenMP][OpenACC][MLIR] Remove typechecks in atomic write/upda… (PR #85059)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Wed Mar 13 03:43:58 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-openacc

@llvm/pr-subscribers-mlir

Author: None (harishch4)

<details>
<summary>Changes</summary>

…te Op verifiers

The failures in issues #<!-- -->83144(lhsType : **_fir.logical<4>_**, rhsType : **_i1_**) and #<!-- -->83722(lhsType  : **_fp64_**, rhsType : **_fp32_**) stem from mismatches between types. 

However, since we already emit a fir.convert operation, I propose removing these checks. This simplification leverages the existing conversion operation to handle these cases appropriately.

---
Full diff: https://github.com/llvm/llvm-project/pull/85059.diff


6 Files Affected:

- (modified) flang/test/Lower/OpenMP/atomic-capture.f90 (+29) 
- (modified) flang/test/Lower/OpenMP/atomic-update.f90 (+21) 
- (modified) flang/test/Lower/OpenMP/atomic-write.f90 (+36) 
- (modified) mlir/include/mlir/Dialect/OpenACCMPCommon/Interfaces/AtomicInterfaces.td (-9) 
- (modified) mlir/test/Dialect/OpenACC/invalid.mlir (-20) 
- (modified) mlir/test/Dialect/OpenMP/invalid.mlir (-20) 


``````````diff
diff --git a/flang/test/Lower/OpenMP/atomic-capture.f90 b/flang/test/Lower/OpenMP/atomic-capture.f90
index cde0281dbdc849..4b19428e7b1e47 100644
--- a/flang/test/Lower/OpenMP/atomic-capture.f90
+++ b/flang/test/Lower/OpenMP/atomic-capture.f90
@@ -96,3 +96,32 @@ subroutine pointers_in_atomic_capture()
         b = a
     !$omp end atomic
 end subroutine
+
+!CHECK-LABEL: func.func @_QPreal_to_double() {
+subroutine real_to_double()
+    !CHECK:     %[[C_ALLOC:.*]] = fir.alloca f64
+    !CHECK:     %[[C_REF:.*]] = fir.alloca f32 {bindc_name = "c", uniq_name = "_QFreal_to_doubleEc"}
+    !CHECK:     %[[C_DECL:.*]]:2 = hlfir.declare %[[C_REF]] {uniq_name = "_QFreal_to_doubleEc"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+    !CHECK:     %[[C2_ALLOC:.*]] = fir.alloca f64 {bindc_name = "c2", uniq_name = "_QFreal_to_doubleEc2"}
+    !CHECK:     %[[C2_DECL:.*]]:2 = hlfir.declare %[[C2_ALLOC]] {uniq_name = "_QFreal_to_doubleEc2"} : (!fir.ref<f64>) -> (!fir.ref<f64>, !fir.ref<f64>)
+        real :: c
+        double precision :: c2
+    !CHECK:     %[[C_LOAD:.*]] = fir.load %[[C_DECL]]#0 : !fir.ref<f32>
+    !CHECK:     %[[C_CVT:.*]] = fir.convert %[[C_LOAD]] : (f32) -> f64
+    !CHECK:     fir.store %[[C_CVT]] to %[[C_ALLOC]] : !fir.ref<f64>
+    !CHECK:     %cst = arith.constant 2.000000e+00 : f32
+    !CHECK:     omp.atomic.capture {
+    !CHECK:       omp.atomic.read %[[C2_DECL]]#1 = %[[C_ALLOC]] : !fir.ref<f64>, f32
+    !CHECK:       omp.atomic.update %[[C_ALLOC]] : !fir.ref<f64> {
+    !CHECK:       ^bb0(%arg0: f32):
+    !CHECK:         %[[RES:.*]] = arith.mulf %cst, %arg0 fastmath<contract> : f32
+    !CHECK:         omp.yield(%[[RES]] : f32)
+    !CHECK:       }
+    !CHECK:     }
+    !CHECK:     return
+    !CHECK:   }
+    !$omp atomic capture
+        c2 = c
+        c = 2.0 * c
+    !$omp end atomic
+    end
diff --git a/flang/test/Lower/OpenMP/atomic-update.f90 b/flang/test/Lower/OpenMP/atomic-update.f90
index 10da97c68c24a1..eeb93dee85941d 100644
--- a/flang/test/Lower/OpenMP/atomic-update.f90
+++ b/flang/test/Lower/OpenMP/atomic-update.f90
@@ -184,3 +184,24 @@ program OmpAtomicUpdate
     w = max(w,x,y,z)
 
 end program OmpAtomicUpdate
+
+!CHECK-DAG: %[[X_REF:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "x", uniq_name = "_QFatomic_update_castingEx"}
+!CHECK-DAG: %[[X_DECL:.*]]:2 = hlfir.declare %[[X_REF]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFatomic_update_castingEx"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>) -> (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
+!CHECK-DAG: %[[Z_REF:.*]] = fir.alloca f32 {bindc_name = "z", fir.target, uniq_name = "_QFatomic_update_castingEz"}
+!CHECK-DAG: %[[Z_DECL:.*]]:2 = hlfir.declare %[[Z_REF]] {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFatomic_update_castingEz"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+!CHECK-DAG: %[[X_LOAD:.*]] = fir.load %[[X_DECL]]#0 : !fir.ref<!fir.box<!fir.ptr<i32>>>
+!CHECK-DAG: %[[X_ADDR:.*]] = fir.box_addr %[[X_LOAD]] : (!fir.box<!fir.ptr<i32>>) -> !fir.ptr<i32>
+!CHECK-DAG: %[[Z_LOAD:.*]] = fir.load %[[Z_DECL]]#0 : !fir.ref<f32>
+!CHECK-DAG: omp.atomic.update %[[X_ADDR]] : !fir.ptr<i32> {
+!CHECK-DAG: ^bb0(%[[ARG0:.*]]: i32):
+!CHECK-DAG:   %[[ARG0_CVT:.*]] = fir.convert %[[ARG0]] : (i32) -> f32
+!CHECK-DAG:   %[[RES:.*]] = arith.addf %[[ARG0_CVT]], %[[Z_LOAD]] fastmath<contract> : f32
+!CHECK-DAG:   %[[RES_CVT:.*]] = fir.convert %[[RES]] : (f32) -> i32
+!CHECK-DAG:   omp.yield(%[[RES_CVT]] : i32)
+!CHECK-DAG: }
+subroutine atomic_update_casting()
+    integer, pointer :: x
+    real, target :: z
+    !$omp atomic update
+        x = x + z
+end
diff --git a/flang/test/Lower/OpenMP/atomic-write.f90 b/flang/test/Lower/OpenMP/atomic-write.f90
index 119f60c1a92f56..908109fa359870 100644
--- a/flang/test/Lower/OpenMP/atomic-write.f90
+++ b/flang/test/Lower/OpenMP/atomic-write.f90
@@ -71,3 +71,39 @@ subroutine atomic_write_typed_assign
   !$omp atomic write
   r2 = 0
 end subroutine
+
+
+!CHECK-LABEL: func.func @_QPatomic_write_logical() {
+!CHECK: %[[L_REF:.*]] = fir.alloca !fir.logical<4> {bindc_name = "l", uniq_name = "_QFatomic_write_logicalEl"}
+!CHECK: %[[L_DECL:.*]]:2 = hlfir.declare %[[L_REF]] {uniq_name = "_QFatomic_write_logicalEl"} : (!fir.ref<!fir.logical<4>>) -> (!fir.ref<!fir.logical<4>>, !fir.ref<!fir.logical<4>>)
+!CHECK: %false = arith.constant false
+!CHECK: omp.atomic.write %[[L_DECL]]#1 = %false : !fir.ref<!fir.logical<4>>, i1
+!CHECK: return
+!CHECK: }
+
+subroutine atomic_write_logical()
+  logical :: l
+  !$omp atomic write
+  l = .false.
+end
+
+
+!CHECK-DAG: %[[X_REF:.*]] = fir.alloca !fir.box<!fir.ptr<i32>> {bindc_name = "x", uniq_name = "_QFatomic_write_castingEx"}
+!CHECK-DAG: %[[X_DECL:.*]]:2 = hlfir.declare %[[X_REF]] {fortran_attrs = #fir.var_attrs<pointer>, uniq_name = "_QFatomic_write_castingEx"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>) -> (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
+!CHECK-DAG: %[[Y_REF:.*]] = fir.alloca i32 {bindc_name = "y", fir.target, uniq_name = "_QFatomic_write_castingEy"}
+!CHECK-DAG: %[[Y_DECL:.*]]:2 = hlfir.declare %[[Y_REF]] {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFatomic_write_castingEy"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK-DAG: %[[Z_REF:.*]] = fir.alloca f32 {bindc_name = "z", fir.target, uniq_name = "_QFatomic_write_castingEz"}
+!CHECK-DAG: %[[Z_DECL:.*]]:2 = hlfir.declare %[[Z_REF]] {fortran_attrs = #fir.var_attrs<target>, uniq_name = "_QFatomic_write_castingEz"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
+!CHECK-DAG: %[[Z_LOAD:.*]] = fir.load %[[Z_DECL]]#0 : !fir.ref<f32>
+!CHECK-DAG: %[[Z_CVT:.*]] = fir.convert %[[Z_LOAD]] : (f32) -> i32
+!CHECK-DAG: %[[X_LOAD:.*]] = fir.load %[[X_DECL]]#0 : !fir.ref<!fir.box<!fir.ptr<i32>>>
+!CHECK-DAG: %[[X_ADDR:.*]] = fir.box_addr %[[X_LOAD]] : (!fir.box<!fir.ptr<i32>>) -> !fir.ptr<i32>
+!CHECK-DAG: omp.atomic.write %[[X_ADDR:.*]] = %[[Z_CVT]] : !fir.ptr<i32>, i32
+subroutine atomic_write_casting()
+  integer, pointer :: x
+  integer, target :: y
+  real, target :: z
+  x => y
+  !$omp atomic write
+      x = z
+end
diff --git a/mlir/include/mlir/Dialect/OpenACCMPCommon/Interfaces/AtomicInterfaces.td b/mlir/include/mlir/Dialect/OpenACCMPCommon/Interfaces/AtomicInterfaces.td
index 57063ca763d2f2..f460adffd68f8e 100644
--- a/mlir/include/mlir/Dialect/OpenACCMPCommon/Interfaces/AtomicInterfaces.td
+++ b/mlir/include/mlir/Dialect/OpenACCMPCommon/Interfaces/AtomicInterfaces.td
@@ -86,9 +86,6 @@ def AtomicWriteOpInterface : OpInterface<"AtomicWriteOpInterface"> {
       /*args=*/(ins),
       /*methodBody=*/"",
       /*defaultImplementation=*/[{
-        mlir::Type elementType = $_op.getX().getType().getElementType();
-        if (elementType && elementType != $_op.getExpr().getType())
-          return $_op.emitError("address must dereference to value type");
         return mlir::success();
       }]
     >,
@@ -197,12 +194,6 @@ def AtomicUpdateOpInterface : OpInterface<"AtomicUpdateOpInterface"> {
         if ($_op.getRegion().getNumArguments() != 1)
           return $_op.emitError("the region must accept exactly one argument");
 
-        Type elementType = $_op.getX().getType().getElementType();
-        if (elementType && elementType != $_op.getRegion().getArgument(0).getType()) {
-          return $_op.emitError("the type of the operand must be a pointer type whose "
-                          "element type is the same as that of the region argument");
-        }
-
         return mlir::success();
       }]
     >,
diff --git a/mlir/test/Dialect/OpenACC/invalid.mlir b/mlir/test/Dialect/OpenACC/invalid.mlir
index ec5430420524ce..d7700fcd247178 100644
--- a/mlir/test/Dialect/OpenACC/invalid.mlir
+++ b/mlir/test/Dialect/OpenACC/invalid.mlir
@@ -521,26 +521,6 @@ acc.set
 
 // -----
 
-func.func @acc_atomic_write(%addr : memref<memref<i32>>, %val : i32) {
-  // expected-error @below {{address must dereference to value type}}
-  acc.atomic.write %addr = %val : memref<memref<i32>>, i32
-  return
-}
-
-// -----
-
-func.func @acc_atomic_update(%x: memref<i32>, %expr: f32) {
-  // expected-error @below {{the type of the operand must be a pointer type whose element type is the same as that of the region argument}}
-  acc.atomic.update %x : memref<i32> {
-  ^bb0(%xval: f32):
-    %newval = llvm.fadd %xval, %expr : f32
-    acc.yield %newval : f32
-  }
-  return
-}
-
-// -----
-
 func.func @acc_atomic_update(%x: memref<i32>, %expr: i32) {
   // expected-error @+2 {{op expects regions to end with 'acc.yield', found 'acc.terminator'}}
   // expected-note @below {{in custom textual format, the absence of terminator implies 'acc.yield'}}
diff --git a/mlir/test/Dialect/OpenMP/invalid.mlir b/mlir/test/Dialect/OpenMP/invalid.mlir
index d9261b89e24e3a..f554dbaaafe154 100644
--- a/mlir/test/Dialect/OpenMP/invalid.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid.mlir
@@ -703,26 +703,6 @@ func.func @omp_atomic_write6(%addr : memref<i32>, %val : i32) {
 
 // -----
 
-func.func @omp_atomic_write(%addr : memref<memref<i32>>, %val : i32) {
-  // expected-error @below {{address must dereference to value type}}
-  omp.atomic.write %addr = %val : memref<memref<i32>>, i32
-  return
-}
-
-// -----
-
-func.func @omp_atomic_update1(%x: memref<i32>, %expr: f32) {
-  // expected-error @below {{the type of the operand must be a pointer type whose element type is the same as that of the region argument}}
-  omp.atomic.update %x : memref<i32> {
-  ^bb0(%xval: f32):
-    %newval = llvm.fadd %xval, %expr : f32
-    omp.yield (%newval : f32)
-  }
-  return
-}
-
-// -----
-
 func.func @omp_atomic_update2(%x: memref<i32>, %expr: i32) {
   // expected-error @+2 {{op expects regions to end with 'omp.yield', found 'omp.terminator'}}
   // expected-note @below {{in custom textual format, the absence of terminator implies 'omp.yield'}}

``````````

</details>


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


More information about the Mlir-commits mailing list