[flang-commits] [flang] [flang][OpenMP] Use iterator_range/range-for for FindClauses, NFC (PR #115749)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Mon Nov 11 10:16:39 PST 2024
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/115749
>From 7374375731e1eb1f27bc072580c14bf9ab9a6094 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 11 Nov 2024 11:07:26 -0600
Subject: [PATCH 1/2] [flang][OpenMP] Use iterator_range/range-for for
FindClauses, NFC
Implement a thin wrapper `GetClauses` that returns llvm::iterator_range
made from the pair of iterators returned by FindClauses. This enables the
use of range-for, which in turn makes the code a little more readable.
---
flang/lib/Semantics/check-omp-structure.cpp | 92 ++++++++-------------
flang/lib/Semantics/check-omp-structure.h | 8 ++
2 files changed, 44 insertions(+), 56 deletions(-)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index dc90b4cccabd26..605bcdb5f2f2c4 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -337,24 +337,22 @@ void OmpStructureChecker::CheckMultListItems() {
semantics::UnorderedSymbolSet listVars;
// Aligned clause
- auto alignedClauses{FindClauses(llvm::omp::Clause::OMPC_aligned)};
- for (auto itr = alignedClauses.first; itr != alignedClauses.second; ++itr) {
- const auto &alignedClause{
- std::get<parser::OmpClause::Aligned>(itr->second->u)};
+ for (auto [_, clause] : GetClauses(llvm::omp::Clause::OMPC_aligned)) {
+ const auto &alignedClause{std::get<parser::OmpClause::Aligned>(clause->u)};
const auto &alignedList{std::get<0>(alignedClause.v.t)};
std::list<parser::Name> alignedNameList;
for (const auto &ompObject : alignedList.v) {
if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
if (name->symbol) {
if (FindCommonBlockContaining(*(name->symbol))) {
- context_.Say(itr->second->source,
+ context_.Say(clause->source,
"'%s' is a common block name and can not appear in an "
"ALIGNED clause"_err_en_US,
name->ToString());
} else if (!(IsBuiltinCPtr(*(name->symbol)) ||
IsAllocatableOrObjectPointer(
&name->symbol->GetUltimate()))) {
- context_.Say(itr->second->source,
+ context_.Say(clause->source,
"'%s' in ALIGNED clause must be of type C_PTR, POINTER or "
"ALLOCATABLE"_err_en_US,
name->ToString());
@@ -368,18 +366,16 @@ void OmpStructureChecker::CheckMultListItems() {
}
}
CheckMultipleOccurrence(
- listVars, alignedNameList, itr->second->source, "ALIGNED");
+ listVars, alignedNameList, clause->source, "ALIGNED");
}
// Nontemporal clause
- auto nonTemporalClauses{FindClauses(llvm::omp::Clause::OMPC_nontemporal)};
- for (auto itr = nonTemporalClauses.first; itr != nonTemporalClauses.second;
- ++itr) {
+ for (auto [_, clause] : GetClauses(llvm::omp::Clause::OMPC_nontemporal)) {
const auto &nontempClause{
- std::get<parser::OmpClause::Nontemporal>(itr->second->u)};
+ std::get<parser::OmpClause::Nontemporal>(clause->u)};
const auto &nontempNameList{nontempClause.v};
CheckMultipleOccurrence(
- listVars, nontempNameList, itr->second->source, "NONTEMPORAL");
+ listVars, nontempNameList, clause->source, "NONTEMPORAL");
}
}
@@ -1688,21 +1684,18 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() {
}};
// Visit the DEPEND and DOACROSS clauses.
- auto depClauses{FindClauses(llvm::omp::Clause::OMPC_depend)};
- for (auto itr{depClauses.first}; itr != depClauses.second; ++itr) {
- const auto &dependClause{
- std::get<parser::OmpClause::Depend>(itr->second->u)};
+ for (auto [_, clause] : GetClauses(llvm::omp::Clause::OMPC_depend)) {
+ const auto &dependClause{std::get<parser::OmpClause::Depend>(clause->u)};
if (auto *doAcross{std::get_if<parser::OmpDoacross>(&dependClause.v.u)}) {
- visitDoacross(*doAcross, itr->second->source);
+ visitDoacross(*doAcross, clause->source);
} else {
- context_.Say(itr->second->source,
+ context_.Say(clause->source,
"Only SINK or SOURCE dependence types are allowed when ORDERED construct is a standalone construct with no ORDERED region"_err_en_US);
}
}
- auto doaClauses{FindClauses(llvm::omp::Clause::OMPC_doacross)};
- for (auto itr{doaClauses.first}; itr != doaClauses.second; ++itr) {
- auto &doaClause{std::get<parser::OmpClause::Doacross>(itr->second->u)};
- visitDoacross(doaClause.v.v, itr->second->source);
+ for (auto [_, clause] : GetClauses(llvm::omp::Clause::OMPC_doacross)) {
+ auto &doaClause{std::get<parser::OmpClause::Doacross>(clause->u)};
+ visitDoacross(doaClause.v.v, clause->source);
}
bool isNestedInDoOrderedWithPara{false};
@@ -1741,17 +1734,15 @@ void OmpStructureChecker::CheckOrderedDependClause(
}
}
}};
- auto depClauses{FindClauses(llvm::omp::Clause::OMPC_depend)};
- for (auto itr{depClauses.first}; itr != depClauses.second; ++itr) {
- auto &dependClause{std::get<parser::OmpClause::Depend>(itr->second->u)};
+ for (auto [_, clause] : GetClauses(llvm::omp::Clause::OMPC_depend)) {
+ auto &dependClause{std::get<parser::OmpClause::Depend>(clause->u)};
if (auto *doAcross{std::get_if<parser::OmpDoacross>(&dependClause.v.u)}) {
- visitDoacross(*doAcross, itr->second->source);
+ visitDoacross(*doAcross, clause->source);
}
}
- auto doaClauses = FindClauses(llvm::omp::Clause::OMPC_doacross);
- for (auto itr{doaClauses.first}; itr != doaClauses.second; ++itr) {
- auto &doaClause{std::get<parser::OmpClause::Doacross>(itr->second->u)};
- visitDoacross(doaClause.v.v, itr->second->source);
+ for (auto [_, clause] : GetClauses(llvm::omp::Clause::OMPC_doacross)) {
+ auto &doaClause{std::get<parser::OmpClause::Doacross>(clause->u)};
+ visitDoacross(doaClause.v.v, clause->source);
}
}
@@ -3829,19 +3820,16 @@ void OmpStructureChecker::Enter(const parser::OmpClause::UseDevicePtr &x) {
SymbolSourceMap currSymbols;
GetSymbolsInObjectList(x.v, currSymbols);
semantics::UnorderedSymbolSet listVars;
- auto useDevicePtrClauses{FindClauses(llvm::omp::Clause::OMPC_use_device_ptr)};
- for (auto itr = useDevicePtrClauses.first; itr != useDevicePtrClauses.second;
- ++itr) {
+ for (auto [_, clauses] : GetClauses(llvm::omp::Clause::OMPC_use_device_ptr)) {
const auto &useDevicePtrClause{
- std::get<parser::OmpClause::UseDevicePtr>(itr->second->u)};
+ std::get<parser::OmpClause::UseDevicePtr>(clause->u)};
const auto &useDevicePtrList{useDevicePtrClause.v};
std::list<parser::Name> useDevicePtrNameList;
for (const auto &ompObject : useDevicePtrList.v) {
if (const auto *name{parser::Unwrap<parser::Name>(ompObject)}) {
if (name->symbol) {
if (!(IsBuiltinCPtr(*(name->symbol)))) {
- context_.Warn(common::UsageWarning::OpenMPUsage,
- itr->second->source,
+ context_.Warn(common::UsageWarning::OpenMPUsage, clause->source,
"Use of non-C_PTR type '%s' in USE_DEVICE_PTR is deprecated, use USE_DEVICE_ADDR instead"_warn_en_US,
name->ToString());
} else {
@@ -3851,7 +3839,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::UseDevicePtr &x) {
}
}
CheckMultipleOccurrence(
- listVars, useDevicePtrNameList, itr->second->source, "USE_DEVICE_PTR");
+ listVars, useDevicePtrNameList, clause->source, "USE_DEVICE_PTR");
}
}
@@ -3861,12 +3849,9 @@ void OmpStructureChecker::Enter(const parser::OmpClause::UseDeviceAddr &x) {
SymbolSourceMap currSymbols;
GetSymbolsInObjectList(x.v, currSymbols);
semantics::UnorderedSymbolSet listVars;
- auto useDeviceAddrClauses{
- FindClauses(llvm::omp::Clause::OMPC_use_device_addr)};
- for (auto itr = useDeviceAddrClauses.first;
- itr != useDeviceAddrClauses.second; ++itr) {
+ for (auto [_, clause] : GetClauses(llvm::omp::Clause::OMPC_use_device_addr)) {
const auto &useDeviceAddrClause{
- std::get<parser::OmpClause::UseDeviceAddr>(itr->second->u)};
+ std::get<parser::OmpClause::UseDeviceAddr>(clause->u)};
const auto &useDeviceAddrList{useDeviceAddrClause.v};
std::list<parser::Name> useDeviceAddrNameList;
for (const auto &ompObject : useDeviceAddrList.v) {
@@ -3876,8 +3861,8 @@ void OmpStructureChecker::Enter(const parser::OmpClause::UseDeviceAddr &x) {
}
}
}
- CheckMultipleOccurrence(listVars, useDeviceAddrNameList,
- itr->second->source, "USE_DEVICE_ADDR");
+ CheckMultipleOccurrence(
+ listVars, useDeviceAddrNameList, clause->source, "USE_DEVICE_ADDR");
}
}
@@ -3886,26 +3871,24 @@ void OmpStructureChecker::Enter(const parser::OmpClause::IsDevicePtr &x) {
SymbolSourceMap currSymbols;
GetSymbolsInObjectList(x.v, currSymbols);
semantics::UnorderedSymbolSet listVars;
- auto isDevicePtrClauses{FindClauses(llvm::omp::Clause::OMPC_is_device_ptr)};
- for (auto itr = isDevicePtrClauses.first; itr != isDevicePtrClauses.second;
- ++itr) {
+ for (auto [_, clause] : GetClauses(llvm::omp::Clause::OMPC_is_device_ptr)) {
const auto &isDevicePtrClause{
- std::get<parser::OmpClause::IsDevicePtr>(itr->second->u)};
+ std::get<parser::OmpClause::IsDevicePtr>(clause->u)};
const auto &isDevicePtrList{isDevicePtrClause.v};
SymbolSourceMap currSymbols;
GetSymbolsInObjectList(isDevicePtrList, currSymbols);
for (auto &[symbol, source] : currSymbols) {
if (!(IsBuiltinCPtr(*symbol))) {
- context_.Say(itr->second->source,
+ context_.Say(clause->source,
"Variable '%s' in IS_DEVICE_PTR clause must be of type C_PTR"_err_en_US,
source.ToString());
} else if (!(IsDummy(*symbol))) {
- context_.Warn(common::UsageWarning::OpenMPUsage, itr->second->source,
+ context_.Warn(common::UsageWarning::OpenMPUsage, clause->source,
"Variable '%s' in IS_DEVICE_PTR clause must be a dummy argument. "
"This semantic check is deprecated from OpenMP 5.2 and later."_warn_en_US,
source.ToString());
} else if (IsAllocatableOrPointer(*symbol) || IsValue(*symbol)) {
- context_.Warn(common::UsageWarning::OpenMPUsage, itr->second->source,
+ context_.Warn(common::UsageWarning::OpenMPUsage, clause->source,
"Variable '%s' in IS_DEVICE_PTR clause must be a dummy argument "
"that does not have the ALLOCATABLE, POINTER or VALUE attribute. "
"This semantic check is deprecated from OpenMP 5.2 and later."_warn_en_US,
@@ -3920,12 +3903,9 @@ void OmpStructureChecker::Enter(const parser::OmpClause::HasDeviceAddr &x) {
SymbolSourceMap currSymbols;
GetSymbolsInObjectList(x.v, currSymbols);
semantics::UnorderedSymbolSet listVars;
- auto hasDeviceAddrClauses{
- FindClauses(llvm::omp::Clause::OMPC_has_device_addr)};
- for (auto itr = hasDeviceAddrClauses.first;
- itr != hasDeviceAddrClauses.second; ++itr) {
+ for (auto [_, clause] : GetClauses(llvm::omp::Clause::OMPC_has_device_addr)) {
const auto &hasDeviceAddrClause{
- std::get<parser::OmpClause::HasDeviceAddr>(itr->second->u)};
+ std::get<parser::OmpClause::HasDeviceAddr>(clause->u)};
const auto &hasDeviceAddrList{hasDeviceAddrClause.v};
std::list<parser::Name> hasDeviceAddrNameList;
for (const auto &ompObject : hasDeviceAddrList.v) {
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 8c13dd20d1e399..73979b5301f6bc 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -141,6 +141,9 @@ class OmpStructureChecker
#include "llvm/Frontend/OpenMP/OMP.inc"
private:
+ inline llvm::iterator_range<typename ClauseMapTy::iterator>
+ GetClauses(llvm::omp::Clause clauseId);
+
bool CheckAllowedClause(llvmOmpClause clause);
bool IsVariableListItem(const Symbol &sym);
bool IsExtendedListItem(const Symbol &sym);
@@ -291,5 +294,10 @@ const T *OmpStructureChecker::FindDuplicateEntry(const std::list<T> &list) {
return nullptr;
}
+llvm::iterator_range<typename OmpStructureChecker::ClauseMapTy::iterator>
+OmpStructureChecker::GetClauses(llvm::omp::Clause clauseId) {
+ return llvm::make_range(FindClauses(clauseId));
+}
+
} // namespace Fortran::semantics
#endif // FORTRAN_SEMANTICS_CHECK_OMP_STRUCTURE_H_
>From b6620a6c6420711abbb5d368c9bdbde9fe8faa17 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 11 Nov 2024 12:16:26 -0600
Subject: [PATCH 2/2] format
---
flang/lib/Semantics/check-omp-structure.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index 73979b5301f6bc..0c5f97f743e2e1 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -141,8 +141,8 @@ class OmpStructureChecker
#include "llvm/Frontend/OpenMP/OMP.inc"
private:
- inline llvm::iterator_range<typename ClauseMapTy::iterator>
- GetClauses(llvm::omp::Clause clauseId);
+ inline llvm::iterator_range<typename ClauseMapTy::iterator> GetClauses(
+ llvm::omp::Clause clauseId);
bool CheckAllowedClause(llvmOmpClause clause);
bool IsVariableListItem(const Symbol &sym);
More information about the flang-commits
mailing list