[flang-commits] [flang] [Flang][OpenMP]Missing convert to lhsType in atomic write (PR #92346)

[via flang-commits]

https://github.com/harishch4 created https://github.com/llvm/llvm-project/pull/92346

Fixes test.f90 in #83144.

This issue is observed only when a boolean constant is assigned to a logical variable. In non-openmp flow, a conversion op is inserted before assigning it to a logical variable. This patch will insert a fir.convert operation when the types are not the same, before generating the atomic write operation.

I've proposed another patch(#85059 ) which removes checks at MLIR level and looks like it's too permissive. I'm planning to abandon this patch and address it here.



>From d513697873755720c4e13257c1baddc26b354881 Mon Sep 17 00:00:00 2001
From: Harish Chambeti <harishcse44 at gmail.com>
Date: Thu, 16 May 2024 09:41:38 +0530
Subject: [PATCH 1/2] [Flang][OpenMP]Missing convert to lhsType in atomic write

---
 flang/lib/Lower/DirectivesCommon.h       |  3 +++
 flang/test/Lower/OpenMP/atomic-write.f90 | 14 ++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/flang/lib/Lower/DirectivesCommon.h b/flang/lib/Lower/DirectivesCommon.h
index 42bd3868196b2..48b090f6d2dbe 100644
--- a/flang/lib/Lower/DirectivesCommon.h
+++ b/flang/lib/Lower/DirectivesCommon.h
@@ -180,6 +180,9 @@ static inline void genOmpAccAtomicWriteStatement(
   // Generate `atomic.write` operation for atomic assignment statements
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
 
+  mlir::Type varType = fir::unwrapRefType(lhsAddr.getType());
+  rhsExpr = firOpBuilder.createConvert(loc, varType, rhsExpr);
+
   if constexpr (std::is_same<AtomicListT,
                              Fortran::parser::OmpAtomicClauseList>()) {
     // If no hint clause is specified, the effect is as if
diff --git a/flang/test/Lower/OpenMP/atomic-write.f90 b/flang/test/Lower/OpenMP/atomic-write.f90
index 85955af64bbe2..cf7f1726022ec 100644
--- a/flang/test/Lower/OpenMP/atomic-write.f90
+++ b/flang/test/Lower/OpenMP/atomic-write.f90
@@ -73,3 +73,17 @@ 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:    %true = arith.constant true
+!CHECK:    %[[CVT:.*]] = fir.convert %true : (i1) -> !fir.logical<4>
+!CHECK:    omp.atomic.write %[[L_DECL]]#1 = %[[CVT]] : !fir.ref<!fir.logical<4>>, !fir.logical<4>
+
+subroutine atomic_write_logical
+  logical :: l
+  !$omp atomic write
+      l = .true.
+  !$omp end atomic
+end
\ No newline at end of file

>From 70438474788e2dc7515ebc37063cde8d6d7c570a Mon Sep 17 00:00:00 2001
From: harishch4 <harishcse44 at gmail.com>
Date: Thu, 16 May 2024 09:48:49 +0530
Subject: [PATCH 2/2] Update atomic-write.f90

---
 flang/test/Lower/OpenMP/atomic-write.f90 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/test/Lower/OpenMP/atomic-write.f90 b/flang/test/Lower/OpenMP/atomic-write.f90
index cf7f1726022ec..e0fa802976d96 100644
--- a/flang/test/Lower/OpenMP/atomic-write.f90
+++ b/flang/test/Lower/OpenMP/atomic-write.f90
@@ -86,4 +86,4 @@ subroutine atomic_write_logical
   !$omp atomic write
       l = .true.
   !$omp end atomic
-end
\ No newline at end of file
+end