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

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-fir-hlfir

Author: None (harishch4)

<details>
<summary>Changes</summary>

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.



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


2 Files Affected:

- (modified) flang/lib/Lower/DirectivesCommon.h (+3) 
- (modified) flang/test/Lower/OpenMP/atomic-write.f90 (+14) 


``````````diff
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..e0fa802976d96 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

``````````

</details>


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