[clang][OpenMP] Slightly refactor EndOpenMPDSABlock for readability, NFC #109003
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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();
...
...
```
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Krzysztof Parzyszek (kparzysz) ChangesChange the loop to Full diff: https://github.com/llvm/llvm-project/pull/109003.diff 1 Files Affected:
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.
|
clang/lib/Sema/SemaOpenMP.cpp
Outdated
| : SimpleRefExpr); | ||
| } | ||
| Clause->setPrivateRefs(PrivateRefs); | ||
| auto finalizeLastprivate = [&](OMPLastprivateClause *Clause) { |
clang/lib/Sema/SemaOpenMP.cpp
Outdated
| QualType Type = VD->getType().getNonReferenceType(); | ||
| const DSAStackTy::DSAVarData DVar = | ||
| DSAStack->getTopDSA(VD, /*FromParent=*/false); | ||
| if (DVar.CKind == OMPC_lastprivate) { |
There was a problem hiding this comment.
Use early exit where possible
clang/lib/Sema/SemaOpenMP.cpp
Outdated
| Clause->setPrivateCopies(PrivateCopies); | ||
| }; | ||
|
|
||
| auto finalizeNontemporal = [&](OMPNontemporalClause *Clause) { |
clang/lib/Sema/SemaOpenMP.cpp
Outdated
| Clause->setPrivateRefs(PrivateRefs); | ||
| }; | ||
|
|
||
| auto finalizeAllocators = [&](OMPUsesAllocatorsClause *Clause) { |
hamphet
pushed a commit
to hamphet/llvm-project
that referenced
this pull request
Sep 18, 2024
…NFC (llvm#109003) 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(); ... ... ```
tmsri
pushed a commit
to tmsri/llvm-project
that referenced
this pull request
Sep 19, 2024
…NFC (llvm#109003) 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(); ... ... ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change the loop
to