llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Zahira Ammarguellat (zahiraam) <details> <summary>Changes</summary> This patch fixes loop variable finalization for OpenMP 6.0 loop-transformations constructs: `tile`, `stripe`, `reverse`, `interchange` and `fuse` to comply with spec requirement page 371, lines 19-21. The spec requires that "After the execution of the loop-transforming construct, the loop-iteration variables of any of its transformation-affected loops have the values that they would have without the loop-transforming directive". --- Patch is 385.41 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/208533.diff 10 Files Affected: - (modified) clang/include/clang/AST/StmtOpenMP.h (+52-16) - (modified) clang/lib/AST/StmtOpenMP.cpp (+24-24) - (modified) clang/lib/CodeGen/CGStmtOpenMP.cpp (+25) - (modified) clang/lib/Sema/SemaOpenMP.cpp (+104-17) - (modified) clang/test/OpenMP/fuse_codegen.cpp (+1328-1156) - (modified) clang/test/OpenMP/interchange_codegen.cpp (+134-45) - (added) clang/test/OpenMP/loop_transform_final_iv.c (+114) - (modified) clang/test/OpenMP/reverse_codegen.cpp (+180-85) - (modified) clang/test/OpenMP/stripe_codegen.cpp (+224-110) - (modified) clang/test/OpenMP/tile_codegen.cpp (+224-110) ``````````diff diff --git a/clang/include/clang/AST/StmtOpenMP.h b/clang/include/clang/AST/StmtOpenMP.h index dbc76e7df8ecd..82c093b9219d7 100644 --- a/clang/include/clang/AST/StmtOpenMP.h +++ b/clang/include/clang/AST/StmtOpenMP.h @@ -5601,6 +5601,7 @@ class OMPTileDirective final enum { PreInitsOffset = 0, TransformedStmtOffset, + FinalsOffset, }; explicit OMPTileDirective(SourceLocation StartLoc, SourceLocation EndLoc, @@ -5617,6 +5618,8 @@ class OMPTileDirective final Data->getChildren()[TransformedStmtOffset] = S; } + void setFinals(Stmt *Finals) { Data->getChildren()[FinalsOffset] = Finals; } + public: /// Create a new AST node representation for '#pragma omp tile'. /// @@ -5630,11 +5633,11 @@ class OMPTileDirective final /// \param TransformedStmt The loop nest after tiling, or nullptr in /// dependent contexts. /// \param PreInits Helper preinits statements for the loop nest. - static OMPTileDirective *Create(const ASTContext &C, SourceLocation StartLoc, - SourceLocation EndLoc, - ArrayRef<OMPClause *> Clauses, - unsigned NumLoops, Stmt *AssociatedStmt, - Stmt *TransformedStmt, Stmt *PreInits); + /// \param Finals Loop variable finalization statements. + static OMPTileDirective * + Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, + ArrayRef<OMPClause *> Clauses, unsigned NumLoops, Stmt *AssociatedStmt, + Stmt *TransformedStmt, Stmt *PreInits, Stmt *Finals); /// Build an empty '#pragma omp tile' AST node for deserialization. /// @@ -5662,6 +5665,9 @@ class OMPTileDirective final /// Return preinits statement. Stmt *getPreInits() const { return Data->getChildren()[PreInitsOffset]; } + /// Return finals statement (loop variable finalization). + Stmt *getFinals() const { return Data->getChildren()[FinalsOffset]; } + static bool classof(const Stmt *T) { return T->getStmtClass() == OMPTileDirectiveClass; } @@ -5677,6 +5683,7 @@ class OMPStripeDirective final enum { PreInitsOffset = 0, TransformedStmtOffset, + FinalsOffset, }; explicit OMPStripeDirective(SourceLocation StartLoc, SourceLocation EndLoc, @@ -5693,6 +5700,8 @@ class OMPStripeDirective final Data->getChildren()[TransformedStmtOffset] = S; } + void setFinals(Stmt *Finals) { Data->getChildren()[FinalsOffset] = Finals; } + public: /// Create a new AST node representation for '#pragma omp stripe'. /// @@ -5706,10 +5715,11 @@ class OMPStripeDirective final /// \param TransformedStmt The loop nest after striping, or nullptr in /// dependent contexts. /// \param PreInits Helper preinits statements for the loop nest. + /// \param Finals Loop variable finalization statements. static OMPStripeDirective * Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, ArrayRef<OMPClause *> Clauses, unsigned NumLoops, Stmt *AssociatedStmt, - Stmt *TransformedStmt, Stmt *PreInits); + Stmt *TransformedStmt, Stmt *PreInits, Stmt *Finals); /// Build an empty '#pragma omp stripe' AST node for deserialization. /// @@ -5736,6 +5746,9 @@ class OMPStripeDirective final /// Return preinits statement. Stmt *getPreInits() const { return Data->getChildren()[PreInitsOffset]; } + /// Return finals statement (loop variable finalization). + Stmt *getFinals() const { return Data->getChildren()[FinalsOffset]; } + static bool classof(const Stmt *T) { return T->getStmtClass() == OMPStripeDirectiveClass; } @@ -5756,6 +5769,7 @@ class OMPUnrollDirective final enum { PreInitsOffset = 0, TransformedStmtOffset, + FinalsOffset, }; explicit OMPUnrollDirective(SourceLocation StartLoc, SourceLocation EndLoc) @@ -5832,6 +5846,7 @@ class OMPReverseDirective final enum { PreInitsOffset = 0, TransformedStmtOffset, + FinalsOffset, }; explicit OMPReverseDirective(SourceLocation StartLoc, SourceLocation EndLoc, @@ -5848,6 +5863,8 @@ class OMPReverseDirective final Data->getChildren()[TransformedStmtOffset] = S; } + void setFinals(Stmt *Finals) { Data->getChildren()[FinalsOffset] = Finals; } + public: /// Create a new AST node representation for '#pragma omp reverse'. /// @@ -5859,11 +5876,11 @@ class OMPReverseDirective final /// \param TransformedStmt The loop nest after tiling, or nullptr in /// dependent contexts. /// \param PreInits Helper preinits statements for the loop nest. - static OMPReverseDirective *Create(const ASTContext &C, - SourceLocation StartLoc, - SourceLocation EndLoc, - Stmt *AssociatedStmt, unsigned NumLoops, - Stmt *TransformedStmt, Stmt *PreInits); + /// \param Finals Loop variable finalization statements. + static OMPReverseDirective * + Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, + Stmt *AssociatedStmt, unsigned NumLoops, Stmt *TransformedStmt, + Stmt *PreInits, Stmt *Finals); /// Build an empty '#pragma omp reverse' AST node for deserialization. /// @@ -5881,6 +5898,9 @@ class OMPReverseDirective final /// Return preinits statement. Stmt *getPreInits() const { return Data->getChildren()[PreInitsOffset]; } + /// Return finals statement. + Stmt *getFinals() const { return Data->getChildren()[FinalsOffset]; } + static bool classof(const Stmt *T) { return T->getStmtClass() == OMPReverseDirectiveClass; } @@ -5903,6 +5923,7 @@ class OMPInterchangeDirective final enum { PreInitsOffset = 0, TransformedStmtOffset, + FinalsOffset, }; explicit OMPInterchangeDirective(SourceLocation StartLoc, @@ -5919,6 +5940,8 @@ class OMPInterchangeDirective final Data->getChildren()[TransformedStmtOffset] = S; } + void setFinals(Stmt *Finals) { Data->getChildren()[FinalsOffset] = Finals; } + public: /// Create a new AST node representation for '#pragma omp interchange'. /// @@ -5932,10 +5955,11 @@ class OMPInterchangeDirective final /// \param TransformedStmt The loop nest after tiling, or nullptr in /// dependent contexts. /// \param PreInits Helper preinits statements for the loop nest. + /// \param Finals Loop variable finalization statements. static OMPInterchangeDirective * Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, ArrayRef<OMPClause *> Clauses, unsigned NumLoops, Stmt *AssociatedStmt, - Stmt *TransformedStmt, Stmt *PreInits); + Stmt *TransformedStmt, Stmt *PreInits, Stmt *Finals); /// Build an empty '#pragma omp interchange' AST node for deserialization. /// @@ -5954,6 +5978,9 @@ class OMPInterchangeDirective final /// Return preinits statement. Stmt *getPreInits() const { return Data->getChildren()[PreInitsOffset]; } + /// Return finals statement. + Stmt *getFinals() const { return Data->getChildren()[FinalsOffset]; } + static bool classof(const Stmt *T) { return T->getStmtClass() == OMPInterchangeDirectiveClass; } @@ -6009,6 +6036,7 @@ class OMPFuseDirective final enum { PreInitsOffset = 0, TransformedStmtOffset, + FinalsOffset, }; explicit OMPFuseDirective(SourceLocation StartLoc, SourceLocation EndLoc) @@ -6023,6 +6051,8 @@ class OMPFuseDirective final Data->getChildren()[TransformedStmtOffset] = S; } + void setFinals(Stmt *Finals) { Data->getChildren()[FinalsOffset] = Finals; } + public: /// Create a new AST node representation for #pragma omp fuse' /// @@ -6038,10 +6068,13 @@ class OMPFuseDirective final /// \param TransformedStmt The loop nest after fusion, or nullptr in /// dependent /// \param PreInits Helper preinits statements for the loop nest - static OMPFuseDirective * - Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, - ArrayRef<OMPClause *> Clauses, unsigned NumGeneratedTopLevelLoops, - Stmt *AssociatedStmt, Stmt *TransformedStmt, Stmt *PreInits); + /// \param Finals Loop variable finalization statements + static OMPFuseDirective *Create(const ASTContext &C, SourceLocation StartLoc, + SourceLocation EndLoc, + ArrayRef<OMPClause *> Clauses, + unsigned NumGeneratedTopLevelLoops, + Stmt *AssociatedStmt, Stmt *TransformedStmt, + Stmt *PreInits, Stmt *Finals); /// Build an empty '#pragma omp fuse' AST node for deserialization /// @@ -6060,6 +6093,9 @@ class OMPFuseDirective final /// Return preinits statement. Stmt *getPreInits() const { return Data->getChildren()[PreInitsOffset]; } + /// Return finals statement. + Stmt *getFinals() const { return Data->getChildren()[FinalsOffset]; } + static bool classof(const Stmt *T) { return T->getStmtClass() == OMPFuseDirectiveClass; } diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp index 9d6b315effb41..13f1d2d94aa85 100644 --- a/clang/lib/AST/StmtOpenMP.cpp +++ b/clang/lib/AST/StmtOpenMP.cpp @@ -452,12 +452,12 @@ OMPTileDirective * OMPTileDirective::Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, ArrayRef<OMPClause *> Clauses, unsigned NumLoops, Stmt *AssociatedStmt, - Stmt *TransformedStmt, Stmt *PreInits) { + Stmt *TransformedStmt, Stmt *PreInits, Stmt *Finals) { OMPTileDirective *Dir = createDirective<OMPTileDirective>( - C, Clauses, AssociatedStmt, TransformedStmtOffset + 1, StartLoc, EndLoc, - NumLoops); + C, Clauses, AssociatedStmt, FinalsOffset + 1, StartLoc, EndLoc, NumLoops); Dir->setTransformedStmt(TransformedStmt); Dir->setPreInits(PreInits); + Dir->setFinals(Finals); return Dir; } @@ -465,20 +465,19 @@ OMPTileDirective *OMPTileDirective::CreateEmpty(const ASTContext &C, unsigned NumClauses, unsigned NumLoops) { return createEmptyDirective<OMPTileDirective>( - C, NumClauses, /*HasAssociatedStmt=*/true, TransformedStmtOffset + 1, + C, NumClauses, /*HasAssociatedStmt=*/true, FinalsOffset + 1, SourceLocation(), SourceLocation(), NumLoops); } -OMPStripeDirective * -OMPStripeDirective::Create(const ASTContext &C, SourceLocation StartLoc, - SourceLocation EndLoc, ArrayRef<OMPClause *> Clauses, - unsigned NumLoops, Stmt *AssociatedStmt, - Stmt *TransformedStmt, Stmt *PreInits) { +OMPStripeDirective *OMPStripeDirective::Create( + const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, + ArrayRef<OMPClause *> Clauses, unsigned NumLoops, Stmt *AssociatedStmt, + Stmt *TransformedStmt, Stmt *PreInits, Stmt *Finals) { OMPStripeDirective *Dir = createDirective<OMPStripeDirective>( - C, Clauses, AssociatedStmt, TransformedStmtOffset + 1, StartLoc, EndLoc, - NumLoops); + C, Clauses, AssociatedStmt, FinalsOffset + 1, StartLoc, EndLoc, NumLoops); Dir->setTransformedStmt(TransformedStmt); Dir->setPreInits(PreInits); + Dir->setFinals(Finals); return Dir; } @@ -486,7 +485,7 @@ OMPStripeDirective *OMPStripeDirective::CreateEmpty(const ASTContext &C, unsigned NumClauses, unsigned NumLoops) { return createEmptyDirective<OMPStripeDirective>( - C, NumClauses, /*HasAssociatedStmt=*/true, TransformedStmtOffset + 1, + C, NumClauses, /*HasAssociatedStmt=*/true, FinalsOffset + 1, SourceLocation(), SourceLocation(), NumLoops); } @@ -516,31 +515,31 @@ OMPReverseDirective * OMPReverseDirective::Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, Stmt *AssociatedStmt, unsigned NumLoops, Stmt *TransformedStmt, - Stmt *PreInits) { + Stmt *PreInits, Stmt *Finals) { OMPReverseDirective *Dir = createDirective<OMPReverseDirective>( - C, {}, AssociatedStmt, TransformedStmtOffset + 1, StartLoc, EndLoc, - NumLoops); + C, {}, AssociatedStmt, FinalsOffset + 1, StartLoc, EndLoc, NumLoops); Dir->setTransformedStmt(TransformedStmt); Dir->setPreInits(PreInits); + Dir->setFinals(Finals); return Dir; } OMPReverseDirective *OMPReverseDirective::CreateEmpty(const ASTContext &C, unsigned NumLoops) { return createEmptyDirective<OMPReverseDirective>( - C, /*NumClauses=*/0, /*HasAssociatedStmt=*/true, - TransformedStmtOffset + 1, SourceLocation(), SourceLocation(), NumLoops); + C, /*NumClauses=*/0, /*HasAssociatedStmt=*/true, FinalsOffset + 1, + SourceLocation(), SourceLocation(), NumLoops); } OMPInterchangeDirective *OMPInterchangeDirective::Create( const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, ArrayRef<OMPClause *> Clauses, unsigned NumLoops, Stmt *AssociatedStmt, - Stmt *TransformedStmt, Stmt *PreInits) { + Stmt *TransformedStmt, Stmt *PreInits, Stmt *Finals) { OMPInterchangeDirective *Dir = createDirective<OMPInterchangeDirective>( - C, Clauses, AssociatedStmt, TransformedStmtOffset + 1, StartLoc, EndLoc, - NumLoops); + C, Clauses, AssociatedStmt, FinalsOffset + 1, StartLoc, EndLoc, NumLoops); Dir->setTransformedStmt(TransformedStmt); Dir->setPreInits(PreInits); + Dir->setFinals(Finals); return Dir; } @@ -548,7 +547,7 @@ OMPInterchangeDirective * OMPInterchangeDirective::CreateEmpty(const ASTContext &C, unsigned NumClauses, unsigned NumLoops) { return createEmptyDirective<OMPInterchangeDirective>( - C, NumClauses, /*HasAssociatedStmt=*/true, TransformedStmtOffset + 1, + C, NumClauses, /*HasAssociatedStmt=*/true, FinalsOffset + 1, SourceLocation(), SourceLocation(), NumLoops); } @@ -576,12 +575,13 @@ OMPSplitDirective *OMPSplitDirective::CreateEmpty(const ASTContext &C, OMPFuseDirective *OMPFuseDirective::Create( const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, ArrayRef<OMPClause *> Clauses, unsigned NumGeneratedTopLevelLoops, - Stmt *AssociatedStmt, Stmt *TransformedStmt, Stmt *PreInits) { + Stmt *AssociatedStmt, Stmt *TransformedStmt, Stmt *PreInits, Stmt *Finals) { OMPFuseDirective *Dir = createDirective<OMPFuseDirective>( - C, Clauses, AssociatedStmt, TransformedStmtOffset + 1, StartLoc, EndLoc); + C, Clauses, AssociatedStmt, FinalsOffset + 1, StartLoc, EndLoc); Dir->setTransformedStmt(TransformedStmt); Dir->setPreInits(PreInits); + Dir->setFinals(Finals); Dir->setNumGeneratedTopLevelLoops(NumGeneratedTopLevelLoops); return Dir; } @@ -589,7 +589,7 @@ OMPFuseDirective *OMPFuseDirective::Create( OMPFuseDirective *OMPFuseDirective::CreateEmpty(const ASTContext &C, unsigned NumClauses) { OMPFuseDirective *Dir = createEmptyDirective<OMPFuseDirective>( - C, NumClauses, /*HasAssociatedStmt=*/true, TransformedStmtOffset + 1, + C, NumClauses, /*HasAssociatedStmt=*/true, FinalsOffset + 1, SourceLocation(), SourceLocation()); return Dir; } diff --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp b/clang/lib/CodeGen/CGStmtOpenMP.cpp index 88f698f38cce0..247cb65512b68 100644 --- a/clang/lib/CodeGen/CGStmtOpenMP.cpp +++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp @@ -3221,18 +3221,33 @@ void CodeGenFunction::EmitOMPTileDirective(const OMPTileDirective &S) { // Emit the de-sugared statement. OMPTransformDirectiveScopeRAII TileScope(*this, &S); EmitStmt(S.getTransformedStmt()); + + // Emit loop variable finalization as required by OpenMP 6.0 spec to restore + // original loop variable values after the loop-transformation construct. + if (auto *Finals = S.getFinals()) + EmitStmt(Finals); } void CodeGenFunction::EmitOMPStripeDirective(const OMPStripeDirective &S) { // Emit the de-sugared statement. OMPTransformDirectiveScopeRAII StripeScope(*this, &S); EmitStmt(S.getTransformedStmt()); + + // Emit loop variable finalization as required by OpenMP 6.0 spec to restore + // original loop variable values after the loop-transformation construct. + if (auto *Finals = S.getFinals()) + EmitStmt(Finals); } void CodeGenFunction::EmitOMPReverseDirective(const OMPReverseDirective &S) { // Emit the de-sugared statement. OMPTransformDirectiveScopeRAII ReverseScope(*this, &S); EmitStmt(S.getTransformedStmt()); + + // Emit loop variable finalization as required by OpenMP 6.0 spec to restore + // original loop variable values after the loop-transformation construct. + if (auto *Finals = S.getFinals()) + EmitStmt(Finals); } void CodeGenFunction::EmitOMPSplitDirective(const OMPSplitDirective &S) { @@ -3246,12 +3261,22 @@ void CodeGenFunction::EmitOMPInterchangeDirective( // Emit the de-sugared statement. OMPTransformDirectiveScopeRAII InterchangeScope(*this, &S); EmitStmt(S.getTransformedStmt()); + + // Emit loop variable finalization as required by OpenMP 6.0 spec to restore + // original loop variable values after the loop-transformation construct. + if (auto *Finals = S.getFinals()) + EmitStmt(Finals); } void CodeGenFunction::EmitOMPFuseDirective(const OMPFuseDirective &S) { // Emit the de-sugared statement OMPTransformDirectiveScopeRAII FuseScope(*this, &S); EmitStmt(S.getTransformedStmt()); + + // Emit loop variable finalization as required by OpenMP 6.0 spec to restore + // original loop variable values after the loop-transformation construct. + if (auto *Finals = S.getFinals()) + EmitStmt(Finals); } void CodeGenFunction::EmitOMPUnrollDirective(const OMPUnrollDirective &S) { diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 0f45a21c5e461..f9d8e233e0f03 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -14960,6 +14960,24 @@ static Expr *makeFloorIVRef(Sema &SemaRef, ArrayRef<VarDecl *> FloorIndVars, OrigCntVar->getExprLoc()); } +/// Build loop variable finalization statement from HelperExprs.Finals. +/// Returns a CompoundStmt containing all finalization statements, or nullptr +/// if there are no finalization statements. +static Stmt *buildLoopFinalization( + ASTContext &Context, + ArrayRef<OMPLoopBasedDirective::HelperExprs> LoopHelpers) { + SmallVector<Stmt *, 8> FinalizationStmts; + for (const auto &Helper : LoopHelpers) { + if (!Helper.Finals.empty() && Helper.Finals[0]) { + FinalizationStmts.push_back(Helper.Finals[0]); + } + } + if (FinalizationStmts.empty()) + return nullptr; + return CompoundStmt::Create(Context, FinalizationStmts, FPOptionsOverride(), + SourceLocation(), SourceLocation()); +} + StmtResult SemaOpenMP::ActOnOpenMPTileDirective(ArrayRef<OMPClause *> Clauses, Stmt *AStmt, SourceLocation StartLoc, @@ -14989,7 +15007,7 @@ StmtResult SemaOpenMP::ActOnOpenMPTileDirective(ArrayRef<OMPClause *> Clauses, // Delay tiling to when template is completely instantiated. if (SemaRef.CurContext->isDependentContext()) return OMPTileDirective::Create(Context, StartLoc, EndLoc, Clauses, - NumLoops, AStmt, nullptr, nullptr); + NumLoops, AStmt, nullptr, nullptr, nullptr); assert(LoopHelpers.size() == NumLoops && "Expecting loop iteration space dimensionality to match number of " @@ -15232,7 +15250,8 @@ StmtResult SemaOpenMP::ActOnOpenMPTileDirective(ArrayRef<OMPClause *> Clauses, return OMPTileDirective::Create(Context, StartLoc, EndLoc, Clauses, NumLoops, ... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/208533 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
