Author: Walter J.T.V Date: 2025-06-18T20:52:41+02:00 New Revision: 8c3fbaf0ee7322e948403d2234a7230bd6137c98
URL: https://github.com/llvm/llvm-project/commit/8c3fbaf0ee7322e948403d2234a7230bd6137c98 DIFF: https://github.com/llvm/llvm-project/commit/8c3fbaf0ee7322e948403d2234a7230bd6137c98.diff LOG: [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (#140532) This patch is closely related to #139293 and addresses an existing issue in the loop transformation codebase. Specifically, it corrects the handling of the `NumGeneratedLoops` variable in `OMPLoopTransformationDirective` AST nodes and its inheritors (such as OMPUnrollDirective, OMPTileDirective, etc.). Previously, this variable was inaccurately set for certain transformations like reverse or tile. While this did not lead to functional bugs, since the value was only checked to determine whether it was greater than zero or equal to zero, the inconsistency could introduce problems when supporting more complex directives in the future. Added: Modified: clang/include/clang/AST/StmtOpenMP.h clang/lib/AST/StmtOpenMP.cpp clang/lib/Sema/SemaOpenMP.cpp clang/lib/Serialization/ASTReaderStmt.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/StmtOpenMP.h b/clang/include/clang/AST/StmtOpenMP.h index 736bcabbad1f7..e2fd2114026f7 100644 --- a/clang/include/clang/AST/StmtOpenMP.h +++ b/clang/include/clang/AST/StmtOpenMP.h @@ -5787,10 +5787,13 @@ class OMPReverseDirective final : public OMPLoopTransformationDirective { TransformedStmtOffset, }; - explicit OMPReverseDirective(SourceLocation StartLoc, SourceLocation EndLoc) + explicit OMPReverseDirective(SourceLocation StartLoc, SourceLocation EndLoc, + unsigned NumLoops) : OMPLoopTransformationDirective(OMPReverseDirectiveClass, llvm::omp::OMPD_reverse, StartLoc, - EndLoc, 1) {} + EndLoc, NumLoops) { + setNumGeneratedLoops(NumLoops); + } void setPreInits(Stmt *PreInits) { Data->getChildren()[PreInitsOffset] = PreInits; @@ -5806,19 +5809,23 @@ class OMPReverseDirective final : public OMPLoopTransformationDirective { /// \param C Context of the AST. /// \param StartLoc Location of the introducer (e.g. the 'omp' token). /// \param EndLoc Location of the directive's end (e.g. the tok::eod). + /// \param NumLoops Number of affected loops /// \param AssociatedStmt The outermost associated loop. /// \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, Stmt *TransformedStmt, Stmt *PreInits); + static OMPReverseDirective *Create(const ASTContext &C, + SourceLocation StartLoc, + SourceLocation EndLoc, + Stmt *AssociatedStmt, unsigned NumLoops, + Stmt *TransformedStmt, Stmt *PreInits); /// Build an empty '#pragma omp reverse' AST node for deserialization. /// /// \param C Context of the AST. - /// \param NumClauses Number of clauses to allocate. - static OMPReverseDirective *CreateEmpty(const ASTContext &C); + /// \param NumLoops Number of associated loops to allocate + static OMPReverseDirective *CreateEmpty(const ASTContext &C, + unsigned NumLoops); /// Gets/sets the associated loops after the transformation, i.e. after /// de-sugaring. @@ -5857,7 +5864,7 @@ class OMPInterchangeDirective final : public OMPLoopTransformationDirective { : OMPLoopTransformationDirective(OMPInterchangeDirectiveClass, llvm::omp::OMPD_interchange, StartLoc, EndLoc, NumLoops) { - setNumGeneratedLoops(3 * NumLoops); + setNumGeneratedLoops(NumLoops); } void setPreInits(Stmt *PreInits) { diff --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp index 093e1f659916f..2eeb5e45ab511 100644 --- a/clang/lib/AST/StmtOpenMP.cpp +++ b/clang/lib/AST/StmtOpenMP.cpp @@ -471,18 +471,21 @@ OMPUnrollDirective *OMPUnrollDirective::CreateEmpty(const ASTContext &C, OMPReverseDirective * OMPReverseDirective::Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, Stmt *AssociatedStmt, - Stmt *TransformedStmt, Stmt *PreInits) { + unsigned NumLoops, Stmt *TransformedStmt, + Stmt *PreInits) { OMPReverseDirective *Dir = createDirective<OMPReverseDirective>( - C, {}, AssociatedStmt, TransformedStmtOffset + 1, StartLoc, EndLoc); + C, {}, AssociatedStmt, TransformedStmtOffset + 1, StartLoc, EndLoc, + NumLoops); Dir->setTransformedStmt(TransformedStmt); Dir->setPreInits(PreInits); return Dir; } -OMPReverseDirective *OMPReverseDirective::CreateEmpty(const ASTContext &C) { +OMPReverseDirective *OMPReverseDirective::CreateEmpty(const ASTContext &C, + unsigned NumLoops) { return createEmptyDirective<OMPReverseDirective>( C, /*NumClauses=*/0, /*HasAssociatedStmt=*/true, - TransformedStmtOffset + 1, SourceLocation(), SourceLocation()); + TransformedStmtOffset + 1, SourceLocation(), SourceLocation(), NumLoops); } OMPInterchangeDirective *OMPInterchangeDirective::Create( diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index d928b7ae2b4c2..00f4658180807 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -15140,7 +15140,7 @@ StmtResult SemaOpenMP::ActOnOpenMPReverseDirective(Stmt *AStmt, // instantiated. if (SemaRef.CurContext->isDependentContext()) return OMPReverseDirective::Create(Context, StartLoc, EndLoc, AStmt, - nullptr, nullptr); + NumLoops, nullptr, nullptr); assert(LoopHelpers.size() == NumLoops && "Expecting a single-dimensional loop iteration space"); @@ -15299,7 +15299,7 @@ StmtResult SemaOpenMP::ActOnOpenMPReverseDirective(Stmt *AStmt, ForStmt(Context, Init.get(), Cond.get(), nullptr, Incr.get(), ReversedBody, LoopHelper.Init->getBeginLoc(), LoopHelper.Init->getBeginLoc(), LoopHelper.Inc->getEndLoc()); - return OMPReverseDirective::Create(Context, StartLoc, EndLoc, AStmt, + return OMPReverseDirective::Create(Context, StartLoc, EndLoc, AStmt, NumLoops, ReversedFor, buildPreInits(Context, PreInits)); } diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp index 65102b64030c6..44cfb83ad2db4 100644 --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/clang/lib/Serialization/ASTReaderStmt.cpp @@ -3602,11 +3602,10 @@ Stmt *ASTReader::ReadStmtFromStream(ModuleFile &F) { } case STMT_OMP_REVERSE_DIRECTIVE: { - assert(Record[ASTStmtReader::NumStmtFields] == 1 && - "Reverse directive accepts only a single loop"); + unsigned NumLoops = Record[ASTStmtReader::NumStmtFields]; assert(Record[ASTStmtReader::NumStmtFields + 1] == 0 && "Reverse directive has no clauses"); - S = OMPReverseDirective::CreateEmpty(Context); + S = OMPReverseDirective::CreateEmpty(Context, NumLoops); break; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
