[flang-commits] [flang] [mlir] [Flang][OpenMP][OpenACC][MLIR] Remove typechecks in atomic write/upda… (PR #85059)
via flang-commits
flang-commits at lists.llvm.org
Wed Mar 13 03:43:27 PDT 2024
https://github.com/harishch4 created https://github.com/llvm/llvm-project/pull/85059
…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.
>From fa6bc5e8fd9a097c4fda4361c6c36e87b76e9791 Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Wed, 13 Mar 2024 15:39:13 +0530
Subject: [PATCH] [Flang][OpenMP][OpenACC][MLIR] Remove typechecks in atomic
write/update Op verifiers
---
flang/test/Lower/OpenMP/atomic-capture.f90 | 29 +++++++++++++++
flang/test/Lower/OpenMP/atomic-update.f90 | 21 +++++++++++
flang/test/Lower/OpenMP/atomic-write.f90 | 36 +++++++++++++++++++
.../Interfaces/AtomicInterfaces.td | 9 -----
mlir/test/Dialect/OpenACC/invalid.mlir | 20 -----------
mlir/test/Dialect/OpenMP/invalid.mlir | 20 -----------
6 files changed, 86 insertions(+), 49 deletions(-)
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'}}
More information about the flang-commits
mailing list