[clang] [clang][OpenMP] Slightly refactor EndOpenMPDSABlock for readability, NFC (PR #109003)
Krzysztof Parzyszek via cfe-commits
cfe-commits at lists.llvm.org
Tue Sep 17 09:50:48 PDT 2024
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/109003
>From d9bb31da5c897aad0dbc55781df2341db75aad6d Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 12 Aug 2024 08:46:12 -0500
Subject: [PATCH 1/2] [clang][OpenMP] Slightly refactor EndOpenMPDSABlock for
readability, NFC
Change the loop
```
if (isOpenMPExecutableDirective)
for (Clause)
if (Clause is kind1)
multi
line
do
something1;
else if (Clause is kind2)
...
...
```
to
```
auto do1 = ...do something1...;
auto do2 = ...do something2...;
...
if (isOpenMPExecutableDirective)
for (Clause)
if (Clause is kind1)
do1();
else if (Clause is kind2)
do2();
...
...
```
---
clang/lib/Sema/SemaOpenMP.cpp | 211 ++++++++++++++++++----------------
1 file changed, 109 insertions(+), 102 deletions(-)
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index b952ffbd69f5d5..0f4644531420b3 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -2861,113 +2861,120 @@ void SemaOpenMP::EndOpenMPDSABlock(Stmt *CurDirective) {
// clause requires an accessible, unambiguous default constructor for the
// class type, unless the list item is also specified in a firstprivate
// clause.
- if (const auto *D = dyn_cast_or_null<OMPExecutableDirective>(CurDirective)) {
- for (OMPClause *C : D->clauses()) {
- if (auto *Clause = dyn_cast<OMPLastprivateClause>(C)) {
- SmallVector<Expr *, 8> PrivateCopies;
- for (Expr *DE : Clause->varlist()) {
- if (DE->isValueDependent() || DE->isTypeDependent()) {
- PrivateCopies.push_back(nullptr);
- continue;
- }
- auto *DRE = cast<DeclRefExpr>(DE->IgnoreParens());
- auto *VD = cast<VarDecl>(DRE->getDecl());
- QualType Type = VD->getType().getNonReferenceType();
- const DSAStackTy::DSAVarData DVar =
- DSAStack->getTopDSA(VD, /*FromParent=*/false);
- if (DVar.CKind == OMPC_lastprivate) {
- // Generate helper private variable and initialize it with the
- // default value. The address of the original variable is replaced
- // by the address of the new private variable in CodeGen. This new
- // variable is not added to IdResolver, so the code in the OpenMP
- // region uses original variable for proper diagnostics.
- VarDecl *VDPrivate = buildVarDecl(
- SemaRef, DE->getExprLoc(), Type.getUnqualifiedType(),
- VD->getName(), VD->hasAttrs() ? &VD->getAttrs() : nullptr, DRE);
- SemaRef.ActOnUninitializedDecl(VDPrivate);
- if (VDPrivate->isInvalidDecl()) {
- PrivateCopies.push_back(nullptr);
- continue;
- }
- PrivateCopies.push_back(buildDeclRefExpr(
- SemaRef, VDPrivate, DE->getType(), DE->getExprLoc()));
- } else {
- // The variable is also a firstprivate, so initialization sequence
- // for private copy is generated already.
- PrivateCopies.push_back(nullptr);
- }
- }
- Clause->setPrivateCopies(PrivateCopies);
- continue;
- }
- // Finalize nontemporal clause by handling private copies, if any.
- if (auto *Clause = dyn_cast<OMPNontemporalClause>(C)) {
- SmallVector<Expr *, 8> PrivateRefs;
- for (Expr *RefExpr : Clause->varlist()) {
- assert(RefExpr && "NULL expr in OpenMP nontemporal clause.");
- SourceLocation ELoc;
- SourceRange ERange;
- Expr *SimpleRefExpr = RefExpr;
- auto Res = getPrivateItem(SemaRef, SimpleRefExpr, ELoc, ERange);
- if (Res.second)
- // It will be analyzed later.
- PrivateRefs.push_back(RefExpr);
- ValueDecl *D = Res.first;
- if (!D)
- continue;
- const DSAStackTy::DSAVarData DVar =
- DSAStack->getTopDSA(D, /*FromParent=*/false);
- PrivateRefs.push_back(DVar.PrivateCopy ? DVar.PrivateCopy
- : SimpleRefExpr);
- }
- Clause->setPrivateRefs(PrivateRefs);
+ auto finalizeLastprivate = [&](OMPLastprivateClause *Clause) {
+ SmallVector<Expr *, 8> PrivateCopies;
+ for (Expr *DE : Clause->varlist()) {
+ if (DE->isValueDependent() || DE->isTypeDependent()) {
+ PrivateCopies.push_back(nullptr);
continue;
}
- if (auto *Clause = dyn_cast<OMPUsesAllocatorsClause>(C)) {
- for (unsigned I = 0, E = Clause->getNumberOfAllocators(); I < E; ++I) {
- OMPUsesAllocatorsClause::Data D = Clause->getAllocatorData(I);
- auto *DRE = dyn_cast<DeclRefExpr>(D.Allocator->IgnoreParenImpCasts());
- if (!DRE)
- continue;
- ValueDecl *VD = DRE->getDecl();
- if (!VD || !isa<VarDecl>(VD))
- continue;
- DSAStackTy::DSAVarData DVar =
- DSAStack->getTopDSA(VD, /*FromParent=*/false);
- // OpenMP [2.12.5, target Construct]
- // Memory allocators that appear in a uses_allocators clause cannot
- // appear in other data-sharing attribute clauses or data-mapping
- // attribute clauses in the same construct.
- Expr *MapExpr = nullptr;
- if (DVar.RefExpr ||
- DSAStack->checkMappableExprComponentListsForDecl(
- VD, /*CurrentRegionOnly=*/true,
- [VD, &MapExpr](
- OMPClauseMappableExprCommon::MappableExprComponentListRef
- MapExprComponents,
- OpenMPClauseKind C) {
- auto MI = MapExprComponents.rbegin();
- auto ME = MapExprComponents.rend();
- if (MI != ME &&
- MI->getAssociatedDeclaration()->getCanonicalDecl() ==
- VD->getCanonicalDecl()) {
- MapExpr = MI->getAssociatedExpression();
- return true;
- }
- return false;
- })) {
- Diag(D.Allocator->getExprLoc(),
- diag::err_omp_allocator_used_in_clauses)
- << D.Allocator->getSourceRange();
- if (DVar.RefExpr)
- reportOriginalDsa(SemaRef, DSAStack, VD, DVar);
- else
- Diag(MapExpr->getExprLoc(), diag::note_used_here)
- << MapExpr->getSourceRange();
- }
+ auto *DRE = cast<DeclRefExpr>(DE->IgnoreParens());
+ auto *VD = cast<VarDecl>(DRE->getDecl());
+ QualType Type = VD->getType().getNonReferenceType();
+ const DSAStackTy::DSAVarData DVar =
+ DSAStack->getTopDSA(VD, /*FromParent=*/false);
+ if (DVar.CKind == OMPC_lastprivate) {
+ // Generate helper private variable and initialize it with the
+ // default value. The address of the original variable is replaced
+ // by the address of the new private variable in CodeGen. This new
+ // variable is not added to IdResolver, so the code in the OpenMP
+ // region uses original variable for proper diagnostics.
+ VarDecl *VDPrivate = buildVarDecl(
+ SemaRef, DE->getExprLoc(), Type.getUnqualifiedType(), VD->getName(),
+ VD->hasAttrs() ? &VD->getAttrs() : nullptr, DRE);
+ SemaRef.ActOnUninitializedDecl(VDPrivate);
+ if (VDPrivate->isInvalidDecl()) {
+ PrivateCopies.push_back(nullptr);
+ continue;
}
+ PrivateCopies.push_back(buildDeclRefExpr(
+ SemaRef, VDPrivate, DE->getType(), DE->getExprLoc()));
+ } else {
+ // The variable is also a firstprivate, so initialization sequence
+ // for private copy is generated already.
+ PrivateCopies.push_back(nullptr);
+ }
+ }
+ Clause->setPrivateCopies(PrivateCopies);
+ };
+
+ auto finalizeNontemporal = [&](OMPNontemporalClause *Clause) {
+ // Finalize nontemporal clause by handling private copies, if any.
+ SmallVector<Expr *, 8> PrivateRefs;
+ for (Expr *RefExpr : Clause->varlist()) {
+ assert(RefExpr && "NULL expr in OpenMP nontemporal clause.");
+ SourceLocation ELoc;
+ SourceRange ERange;
+ Expr *SimpleRefExpr = RefExpr;
+ auto Res = getPrivateItem(SemaRef, SimpleRefExpr, ELoc, ERange);
+ if (Res.second)
+ // It will be analyzed later.
+ PrivateRefs.push_back(RefExpr);
+ ValueDecl *D = Res.first;
+ if (!D)
continue;
+
+ const DSAStackTy::DSAVarData DVar =
+ DSAStack->getTopDSA(D, /*FromParent=*/false);
+ PrivateRefs.push_back(DVar.PrivateCopy ? DVar.PrivateCopy
+ : SimpleRefExpr);
+ }
+ Clause->setPrivateRefs(PrivateRefs);
+ };
+
+ auto finalizeAllocators = [&](OMPUsesAllocatorsClause *Clause) {
+ for (unsigned I = 0, E = Clause->getNumberOfAllocators(); I < E; ++I) {
+ OMPUsesAllocatorsClause::Data D = Clause->getAllocatorData(I);
+ auto *DRE = dyn_cast<DeclRefExpr>(D.Allocator->IgnoreParenImpCasts());
+ if (!DRE)
+ continue;
+ ValueDecl *VD = DRE->getDecl();
+ if (!VD || !isa<VarDecl>(VD))
+ continue;
+ DSAStackTy::DSAVarData DVar =
+ DSAStack->getTopDSA(VD, /*FromParent=*/false);
+ // OpenMP [2.12.5, target Construct]
+ // Memory allocators that appear in a uses_allocators clause cannot
+ // appear in other data-sharing attribute clauses or data-mapping
+ // attribute clauses in the same construct.
+ Expr *MapExpr = nullptr;
+ if (DVar.RefExpr ||
+ DSAStack->checkMappableExprComponentListsForDecl(
+ VD, /*CurrentRegionOnly=*/true,
+ [VD, &MapExpr](
+ OMPClauseMappableExprCommon::MappableExprComponentListRef
+ MapExprComponents,
+ OpenMPClauseKind C) {
+ auto MI = MapExprComponents.rbegin();
+ auto ME = MapExprComponents.rend();
+ if (MI != ME &&
+ MI->getAssociatedDeclaration()->getCanonicalDecl() ==
+ VD->getCanonicalDecl()) {
+ MapExpr = MI->getAssociatedExpression();
+ return true;
+ }
+ return false;
+ })) {
+ Diag(D.Allocator->getExprLoc(), diag::err_omp_allocator_used_in_clauses)
+ << D.Allocator->getSourceRange();
+ if (DVar.RefExpr)
+ reportOriginalDsa(SemaRef, DSAStack, VD, DVar);
+ else
+ Diag(MapExpr->getExprLoc(), diag::note_used_here)
+ << MapExpr->getSourceRange();
+ }
+ }
+ };
+
+ if (const auto *D = dyn_cast_or_null<OMPExecutableDirective>(CurDirective)) {
+ for (OMPClause *C : D->clauses()) {
+ if (auto *Clause = dyn_cast<OMPLastprivateClause>(C)) {
+ finalizeLastprivate(Clause);
+ } else if (auto *Clause = dyn_cast<OMPNontemporalClause>(C)) {
+ finalizeNontemporal(Clause);
+ } else if (auto *Clause = dyn_cast<OMPUsesAllocatorsClause>(C)) {
+ finalizeAllocators(Clause);
}
}
// Check allocate clauses.
>From 8bcdd129744c98b1389eb151448c8031e353e5b3 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 17 Sep 2024 11:50:03 -0500
Subject: [PATCH 2/2] Capitalize local variables, use early continue in loop
---
clang/lib/Sema/SemaOpenMP.cpp | 46 +++++++++++++++++------------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 0f4644531420b3..9afb8cea26fe78 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -2862,7 +2862,7 @@ void SemaOpenMP::EndOpenMPDSABlock(Stmt *CurDirective) {
// class type, unless the list item is also specified in a firstprivate
// clause.
- auto finalizeLastprivate = [&](OMPLastprivateClause *Clause) {
+ auto FinalizeLastprivate = [&](OMPLastprivateClause *Clause) {
SmallVector<Expr *, 8> PrivateCopies;
for (Expr *DE : Clause->varlist()) {
if (DE->isValueDependent() || DE->isTypeDependent()) {
@@ -2874,32 +2874,32 @@ void SemaOpenMP::EndOpenMPDSABlock(Stmt *CurDirective) {
QualType Type = VD->getType().getNonReferenceType();
const DSAStackTy::DSAVarData DVar =
DSAStack->getTopDSA(VD, /*FromParent=*/false);
- if (DVar.CKind == OMPC_lastprivate) {
- // Generate helper private variable and initialize it with the
- // default value. The address of the original variable is replaced
- // by the address of the new private variable in CodeGen. This new
- // variable is not added to IdResolver, so the code in the OpenMP
- // region uses original variable for proper diagnostics.
- VarDecl *VDPrivate = buildVarDecl(
- SemaRef, DE->getExprLoc(), Type.getUnqualifiedType(), VD->getName(),
- VD->hasAttrs() ? &VD->getAttrs() : nullptr, DRE);
- SemaRef.ActOnUninitializedDecl(VDPrivate);
- if (VDPrivate->isInvalidDecl()) {
- PrivateCopies.push_back(nullptr);
- continue;
- }
- PrivateCopies.push_back(buildDeclRefExpr(
- SemaRef, VDPrivate, DE->getType(), DE->getExprLoc()));
- } else {
+ if (DVar.CKind != OMPC_lastprivate) {
// The variable is also a firstprivate, so initialization sequence
// for private copy is generated already.
PrivateCopies.push_back(nullptr);
+ continue;
+ }
+ // Generate helper private variable and initialize it with the
+ // default value. The address of the original variable is replaced
+ // by the address of the new private variable in CodeGen. This new
+ // variable is not added to IdResolver, so the code in the OpenMP
+ // region uses original variable for proper diagnostics.
+ VarDecl *VDPrivate = buildVarDecl(
+ SemaRef, DE->getExprLoc(), Type.getUnqualifiedType(), VD->getName(),
+ VD->hasAttrs() ? &VD->getAttrs() : nullptr, DRE);
+ SemaRef.ActOnUninitializedDecl(VDPrivate);
+ if (VDPrivate->isInvalidDecl()) {
+ PrivateCopies.push_back(nullptr);
+ continue;
}
+ PrivateCopies.push_back(buildDeclRefExpr(
+ SemaRef, VDPrivate, DE->getType(), DE->getExprLoc()));
}
Clause->setPrivateCopies(PrivateCopies);
};
- auto finalizeNontemporal = [&](OMPNontemporalClause *Clause) {
+ auto FinalizeNontemporal = [&](OMPNontemporalClause *Clause) {
// Finalize nontemporal clause by handling private copies, if any.
SmallVector<Expr *, 8> PrivateRefs;
for (Expr *RefExpr : Clause->varlist()) {
@@ -2923,7 +2923,7 @@ void SemaOpenMP::EndOpenMPDSABlock(Stmt *CurDirective) {
Clause->setPrivateRefs(PrivateRefs);
};
- auto finalizeAllocators = [&](OMPUsesAllocatorsClause *Clause) {
+ auto FinalizeAllocators = [&](OMPUsesAllocatorsClause *Clause) {
for (unsigned I = 0, E = Clause->getNumberOfAllocators(); I < E; ++I) {
OMPUsesAllocatorsClause::Data D = Clause->getAllocatorData(I);
auto *DRE = dyn_cast<DeclRefExpr>(D.Allocator->IgnoreParenImpCasts());
@@ -2970,11 +2970,11 @@ void SemaOpenMP::EndOpenMPDSABlock(Stmt *CurDirective) {
if (const auto *D = dyn_cast_or_null<OMPExecutableDirective>(CurDirective)) {
for (OMPClause *C : D->clauses()) {
if (auto *Clause = dyn_cast<OMPLastprivateClause>(C)) {
- finalizeLastprivate(Clause);
+ FinalizeLastprivate(Clause);
} else if (auto *Clause = dyn_cast<OMPNontemporalClause>(C)) {
- finalizeNontemporal(Clause);
+ FinalizeNontemporal(Clause);
} else if (auto *Clause = dyn_cast<OMPUsesAllocatorsClause>(C)) {
- finalizeAllocators(Clause);
+ FinalizeAllocators(Clause);
}
}
// Check allocate clauses.
More information about the cfe-commits
mailing list