[flang-commits] [flang] [flang][OpenMP] Add TODO messages for partially implemented atomic types (PR #99817)
Tom Eccles via flang-commits
flang-commits at lists.llvm.org
Mon Jul 22 04:26:24 PDT 2024
https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/99817
>From da0e298e196c3b8eb5334d62caeba5e49b9eb573 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Sun, 21 Jul 2024 17:04:25 +0000
Subject: [PATCH 1/3] [flang][OpenMP] Add TODO messages for partially
implemented atomic types
There is ongoing work for fir.complex but this looks unlikely to land
before the LLVM branch. Adding a TODO message gives cleaner output to
users. Currently fir.complex leads to a compiler assertion failure.
Some uses of character types lead to invalid fir.convert type
conversions, others make it far enough to hit the same assertion failure
as for fir.complex.
---
flang/lib/Lower/DirectivesCommon.h | 23 +++++++++++++++
.../Lower/OpenMP/Todo/atomic-character.f90 | 8 ++++++
.../test/Lower/OpenMP/Todo/atomic-complex.f90 | 8 ++++++
flang/test/Lower/OpenMP/atomic-read.f90 | 28 ++++++++-----------
4 files changed, 51 insertions(+), 16 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/Todo/atomic-character.f90
create mode 100644 flang/test/Lower/OpenMP/Todo/atomic-complex.f90
diff --git a/flang/lib/Lower/DirectivesCommon.h b/flang/lib/Lower/DirectivesCommon.h
index e8d6bb591e361..8bd3132aaa3cd 100644
--- a/flang/lib/Lower/DirectivesCommon.h
+++ b/flang/lib/Lower/DirectivesCommon.h
@@ -33,6 +33,7 @@
#include "flang/Optimizer/Builder/FIRBuilder.h"
#include "flang/Optimizer/Builder/HLFIRTools.h"
#include "flang/Optimizer/Builder/Todo.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
#include "flang/Optimizer/HLFIR/HLFIROps.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/openmp-directive-sets.h"
@@ -135,6 +136,22 @@ static inline void genOmpAtomicHintAndMemoryOrderClauses(
}
}
+template <typename AtomicListT>
+static void processAtomicTODO(mlir::Type elementType, mlir::Location loc) {
+ if (!elementType)
+ return;
+ if constexpr (std::is_same<AtomicListT,
+ Fortran::parser::OmpAtomicClauseList>()) {
+ // based on assertion for supported element types in OMPIRBuilder.cpp
+ // createAtomicRead
+ mlir::Type unwrappedEleTy = fir::unwrapRefType(elementType);
+ bool supportedAtomicType =
+ (!fir::isa_complex(unwrappedEleTy) && fir::isa_trivial(unwrappedEleTy));
+ if (!supportedAtomicType)
+ TODO(loc, "Unsupported atomic type");
+ }
+}
+
/// Used to generate atomic.read operation which is created in existing
/// location set by builder.
template <typename AtomicListT>
@@ -147,6 +164,8 @@ static inline void genOmpAccAtomicCaptureStatement(
// Generate `atomic.read` operation for atomic assigment statements
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ processAtomicTODO<AtomicListT>(elementType, loc);
+
if constexpr (std::is_same<AtomicListT,
Fortran::parser::OmpAtomicClauseList>()) {
// If no hint clause is specified, the effect is as if
@@ -183,6 +202,8 @@ static inline void genOmpAccAtomicWriteStatement(
mlir::Type varType = fir::unwrapRefType(lhsAddr.getType());
rhsExpr = firOpBuilder.createConvert(loc, varType, rhsExpr);
+ processAtomicTODO<AtomicListT>(varType, loc);
+
if constexpr (std::is_same<AtomicListT,
Fortran::parser::OmpAtomicClauseList>()) {
// If no hint clause is specified, the effect is as if
@@ -323,6 +344,8 @@ static inline void genOmpAccAtomicUpdateStatement(
currentLocation, lhsAddr);
}
+ processAtomicTODO<AtomicListT>(varType, loc);
+
llvm::SmallVector<mlir::Type> varTys = {varType};
llvm::SmallVector<mlir::Location> locs = {currentLocation};
firOpBuilder.createBlock(&atomicUpdateOp->getRegion(0), {}, varTys, locs);
diff --git a/flang/test/Lower/OpenMP/Todo/atomic-character.f90 b/flang/test/Lower/OpenMP/Todo/atomic-character.f90
new file mode 100644
index 0000000000000..88effa4a2a515
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/atomic-character.f90
@@ -0,0 +1,8 @@
+! RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: Unsupported atomic type
+subroutine character_atomic
+ character :: l, r
+ !$omp atomic read
+ l = r
+end subroutine
diff --git a/flang/test/Lower/OpenMP/Todo/atomic-complex.f90 b/flang/test/Lower/OpenMP/Todo/atomic-complex.f90
new file mode 100644
index 0000000000000..6d6e4399ee192
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/atomic-complex.f90
@@ -0,0 +1,8 @@
+! RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
+
+! CHECK: not yet implemented: Unsupported atomic type
+subroutine complex_atomic
+ complex :: l, r
+ !$omp atomic read
+ l = r
+end subroutine
diff --git a/flang/test/Lower/OpenMP/atomic-read.f90 b/flang/test/Lower/OpenMP/atomic-read.f90
index 8c3f37c94975e..9559df171111d 100644
--- a/flang/test/Lower/OpenMP/atomic-read.f90
+++ b/flang/test/Lower/OpenMP/atomic-read.f90
@@ -5,22 +5,18 @@
! This test checks the lowering of atomic read
!CHECK: func @_QQmain() attributes {fir.bindc_name = "ompatomic"} {
-!CHECK: %[[A_C1:.*]] = arith.constant 1 : index
-!CHECK: %[[A_REF:.*]] = fir.alloca !fir.char<1> {bindc_name = "a", uniq_name = "_QFEa"}
-!CHECK: %[[A_DECL:.*]]:2 = hlfir.declare %[[A_REF]] typeparams %[[A_C1]] {uniq_name = "_QFEa"} : (!fir.ref<!fir.char<1>>, index) -> (!fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>)
-!CHECK: %[[B_C1:.*]] = arith.constant 1 : index
-!CHECK: %[[B_REF:.*]] = fir.alloca !fir.char<1> {bindc_name = "b", uniq_name = "_QFEb"}
-!CHECK: %[[B_DECL:.*]]:2 = hlfir.declare %[[B_REF]] typeparams %[[B_C1]] {uniq_name = "_QFEb"} : (!fir.ref<!fir.char<1>>, index) -> (!fir.ref<!fir.char<1>>, !fir.ref<!fir.char<1>>)
+!CHECK: %[[A_REF:.*]] = fir.alloca i32 {bindc_name = "a", uniq_name = "_QFEa"}
+!CHECK: %[[A_DECL:.*]]:2 = hlfir.declare %[[A_REF]] {uniq_name = "_QFEa"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK: %[[B_REF:.*]] = fir.alloca i32 {bindc_name = "b", uniq_name = "_QFEb"}
+!CHECK: %[[B_DECL:.*]]:2 = hlfir.declare %[[B_REF]] {uniq_name = "_QFEb"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
!CHECK: %[[C_REF:.*]] = fir.alloca !fir.logical<4> {bindc_name = "c", uniq_name = "_QFEc"}
!CHECK: %[[C_DECL:.*]]:2 = hlfir.declare %[[C_REF]] {uniq_name = "_QFEc"} : (!fir.ref<!fir.logical<4>>) -> (!fir.ref<!fir.logical<4>>, !fir.ref<!fir.logical<4>>)
!CHECK: %[[D_REF:.*]] = fir.alloca !fir.logical<4> {bindc_name = "d", uniq_name = "_QFEd"}
!CHECK: %[[D_DECL:.*]]:2 = hlfir.declare %[[D_REF]] {uniq_name = "_QFEd"} : (!fir.ref<!fir.logical<4>>) -> (!fir.ref<!fir.logical<4>>, !fir.ref<!fir.logical<4>>)
-!CHECK: %[[E_C8:.*]] = arith.constant 8 : index
-!CHECK: %[[E_REF:.*]] = fir.alloca !fir.char<1,8> {bindc_name = "e", uniq_name = "_QFEe"}
-!CHECK: %[[E_DECL:.*]]:2 = hlfir.declare %[[E_REF]] typeparams %[[E_C8]] {uniq_name = "_QFEe"} : (!fir.ref<!fir.char<1,8>>, index) -> (!fir.ref<!fir.char<1,8>>, !fir.ref<!fir.char<1,8>>)
-!CHECK: %[[F_C8:.*]] = arith.constant 8 : index
-!CHECK: %[[F_REF:.*]] = fir.alloca !fir.char<1,8> {bindc_name = "f", uniq_name = "_QFEf"}
-!CHECK: %[[F_DECL:.*]]:2 = hlfir.declare %[[F_REF]] typeparams %[[F_C8]] {uniq_name = "_QFEf"} : (!fir.ref<!fir.char<1,8>>, index) -> (!fir.ref<!fir.char<1,8>>, !fir.ref<!fir.char<1,8>>)
+!CHECK: %[[E_REF:.*]] = fir.alloca i32 {bindc_name = "e", uniq_name = "_QFEe"}
+!CHECK: %[[E_DECL:.*]]:2 = hlfir.declare %[[E_REF]] {uniq_name = "_QFEe"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+!CHECK: %[[F_REF:.*]] = fir.alloca i32 {bindc_name = "f", uniq_name = "_QFEf"}
+!CHECK: %[[F_DECL:.*]]:2 = hlfir.declare %[[F_REF]] {uniq_name = "_QFEf"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
!CHECK: %[[G_REF:.*]] = fir.alloca f32 {bindc_name = "g", uniq_name = "_QFEg"}
!CHECK: %[[G_DECL:.*]]:2 = hlfir.declare %[[G_REF]] {uniq_name = "_QFEg"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
!CHECK: %[[H_REF:.*]] = fir.alloca f32 {bindc_name = "h", uniq_name = "_QFEh"}
@@ -30,9 +26,9 @@
!CHECK: %[[Y_REF:.*]] = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFEy"}
!CHECK: %[[Y_DECL:.*]]:2 = hlfir.declare %[[Y_REF]] {uniq_name = "_QFEy"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
!CHECK: omp.atomic.read %[[X_DECL]]#1 = %[[Y_DECL]]#1 memory_order(acquire) hint(uncontended) : !fir.ref<i32>, i32
-!CHECK: omp.atomic.read %[[A_DECL]]#1 = %[[B_DECL]]#1 memory_order(relaxed) : !fir.ref<!fir.char<1>>, !fir.char<1>
+!CHECK: omp.atomic.read %[[A_DECL]]#1 = %[[B_DECL]]#1 memory_order(relaxed) : !fir.ref<i32>, i32
!CHECK: omp.atomic.read %[[C_DECL]]#1 = %[[D_DECL]]#1 memory_order(seq_cst) hint(contended) : !fir.ref<!fir.logical<4>>, !fir.logical<4>
-!CHECK: omp.atomic.read %[[E_DECL]]#1 = %[[F_DECL]]#1 hint(speculative) : !fir.ref<!fir.char<1,8>>, !fir.char<1,8>
+!CHECK: omp.atomic.read %[[E_DECL]]#1 = %[[F_DECL]]#1 hint(speculative) : !fir.ref<i32>, i32
!CHECK: omp.atomic.read %[[G_DECL]]#1 = %[[H_DECL]]#1 hint(nonspeculative) : !fir.ref<f32>, f32
!CHECK: omp.atomic.read %[[G_DECL]]#1 = %[[H_DECL]]#1 : !fir.ref<f32>, f32
@@ -40,9 +36,9 @@ program OmpAtomic
use omp_lib
integer :: x, y
- character :: a, b
+ integer :: a, b
logical :: c, d
- character(8) :: e, f
+ integer :: e, f
real g, h
!$omp atomic acquire read hint(omp_sync_hint_uncontended)
x = y
>From 91cd72e0870dbdcc6bd4af0207ae81f366b5a71b Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Mon, 22 Jul 2024 10:43:35 +0000
Subject: [PATCH 2/3] Fix comment
---
flang/lib/Lower/DirectivesCommon.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Lower/DirectivesCommon.h b/flang/lib/Lower/DirectivesCommon.h
index 8bd3132aaa3cd..6ff648e0970aa 100644
--- a/flang/lib/Lower/DirectivesCommon.h
+++ b/flang/lib/Lower/DirectivesCommon.h
@@ -142,7 +142,7 @@ static void processAtomicTODO(mlir::Type elementType, mlir::Location loc) {
return;
if constexpr (std::is_same<AtomicListT,
Fortran::parser::OmpAtomicClauseList>()) {
- // based on assertion for supported element types in OMPIRBuilder.cpp
+ // Based on assertion for supported element types in OMPIRBuilder.cpp
// createAtomicRead
mlir::Type unwrappedEleTy = fir::unwrapRefType(elementType);
bool supportedAtomicType =
>From e49a77e9203ba2783090db8e8932e4d8c95ed446 Mon Sep 17 00:00:00 2001
From: Tom Eccles <t at freedommail.info>
Date: Mon, 22 Jul 2024 12:26:15 +0100
Subject: [PATCH 3/3] Rename to processsOmpAtomicTODO
Co-authored-by: Kiran Chandramohan <kiranchandramohan at gmail.com>
---
flang/lib/Lower/DirectivesCommon.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Lower/DirectivesCommon.h b/flang/lib/Lower/DirectivesCommon.h
index 6ff648e0970aa..468e5f1cbe6d7 100644
--- a/flang/lib/Lower/DirectivesCommon.h
+++ b/flang/lib/Lower/DirectivesCommon.h
@@ -137,7 +137,7 @@ static inline void genOmpAtomicHintAndMemoryOrderClauses(
}
template <typename AtomicListT>
-static void processAtomicTODO(mlir::Type elementType, mlir::Location loc) {
+static void processOmpAtomicTODO(mlir::Type elementType, mlir::Location loc) {
if (!elementType)
return;
if constexpr (std::is_same<AtomicListT,
More information about the flang-commits
mailing list