[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation direcrive and "looprange" clause (PR #139293)
@@ -3223,6 +3223,8 @@ LValue CodeGenFunction::EmitDeclRefLValue(const
DeclRefExpr *E) {
// No other cases for now.
} else {
+ llvm::dbgs() << "THE DAMN DECLREFEXPR HASN'T BEEN ENTERED IN
LOCALDECLMAP\n";
+ VD->dumpColor();
eZWALT wrote:
Oops, thanks!
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation direcrive and "looprange" clause (PR #139293)
@@ -5378,6 +5379,10 @@ class CodeGenFunction : public CodeGenTypeCache {
/// Set the address of a local variable.
void setAddrOfLocalVar(const VarDecl *VD, Address Addr) {
+if (LocalDeclMap.count(VD)) {
+ llvm::errs() << "Warning: VarDecl already exists in map: ";
+ VD->dumpColor();
+}
eZWALT wrote:
Oops, thanks!
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation direcrive and "looprange" clause (PR #139293)
@@ -5790,7 +5805,11 @@ class OMPReverseDirective final : public
OMPLoopTransformationDirective {
explicit OMPReverseDirective(SourceLocation StartLoc, SourceLocation EndLoc)
: OMPLoopTransformationDirective(OMPReverseDirectiveClass,
llvm::omp::OMPD_reverse, StartLoc,
- EndLoc, 1) {}
+ EndLoc, 1) {
+// Reverse produces a single top-level canonical loop nest
+setNumGeneratedLoops(1);
eZWALT wrote:
Okey do i make another pull request just for the loop corrections though?
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation direcrive and "looprange" clause (PR #139293)
@@ -962,6 +962,9 @@ class OMPLoopTransformationDirective : public
OMPLoopBasedDirective {
/// Number of loops generated by this loop transformation.
unsigned NumGeneratedLoops = 0;
+ /// Number of top level canonical loop nests generated by this loop
+ /// transformation
+ unsigned NumGeneratedLoopNests = 0;
eZWALT wrote:
Maybe the name is a bit unfortunate and could be improved, but they are 2
completely different fields conceptually. This top level loops are the ones
actually managed by loop Sequence constructs like fuse and the upcoming split.
A loop sequence contains loops which may contain several inner nestes loops,
but these should not be taken into account for performing fusion or splitting.
This was not taken into account originally due to all transformations having a
fixed number of generated top level nests (1). However fuse or split may
generate several loop nests with inner nested loops.
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation direcrive and "looprange" clause (PR #139293)
@@ -962,6 +962,9 @@ class OMPLoopTransformationDirective : public
OMPLoopBasedDirective {
/// Number of loops generated by this loop transformation.
unsigned NumGeneratedLoops = 0;
+ /// Number of top level canonical loop nests generated by this loop
+ /// transformation
+ unsigned NumGeneratedLoopNests = 0;
eZWALT wrote:
Note that unroll is an exception, it could have 0 or 1 but it coincides
perfectly with the original number of loops .
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation direcrive and "looprange" clause (PR #139293)
https://github.com/eZWALT edited https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation direcrive and "looprange" clause (PR #139293)
https://github.com/eZWALT edited https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation direcrive and "looprange" clause (PR #139293)
eZWALT wrote: I want to notify that the following week I'll be unavailable, so expect this patch to be updated on the 20th of May. Thanks for the feedback @alexey-bataev https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: It’s true that NumGeneratedLoops is used throughout the existing OpenMP loop transformation infrastructure. While in some cases its usage could potentially be replaced by NumGeneratedLoopNests (especially when only checking for values like 0 or 1), the two variables convey distinct semantic information. NumGeneratedLoops refers to the number of individual loops produced, while NumGeneratedLoopNests captures the structure of nested loops. For current and future transformations, having access to both could be important for representing complex constructs accurately. Removing NumGeneratedLoops would require changes across the loop transformations logic it's not clear the benefit would justify that cost, particularly given the potential utility of retaining this semantic distinction.I’m not 100% certain all current transformations depend on that level of detail, but I believe it’s valuable to preserve until proven otherwise. https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
https://github.com/eZWALT edited https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: > @alexey-bataev It’s true that NumGeneratedLoops is used throughout the > existing OpenMP loop transformation infrastructure. While in some cases its > usage could potentially be replaced by NumGeneratedLoopNests (especially when > only checking for values like 0 or 1), the two variables convey distinct > semantic information. > > NumGeneratedLoops refers to the number of individual loops produced, while > NumGeneratedLoopNests captures the structure of nested loops. For current and > future transformations, having access to both could be important for > representing complex constructs accurately. > > Removing NumGeneratedLoops would require changes across the loop > transformations logic it's not clear the benefit would justify that cost, > particularly given the potential utility of retaining this semantic > distinction.I’m not 100% certain all current transformations depend on that > level of detail, but I believe it’s valuable to preserve until proven > otherwise. I've identified a case where `NumGeneratedLoops` is necessary and cannot be replaced by `NumGeneratedLoopNests`: the `permutation` clause of the `interchange` directive, e.g., permutation(2,1,...). In this transformation, we’re not interested in the number of top-level loop nests, but rather in how many individual loops exist within a single top-level nest, and how to reorder them. Let me know if i have clarified your doubts or if you want more examples, sometimes this kind of details are somewhat difficult to explain easily. https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -14175,27 +14222,350 @@ bool SemaOpenMP::checkTransformableLoopNest(
return false;
},
[&OriginalInits](OMPLoopBasedDirective *Transform) {
-Stmt *DependentPreInits;
-if (auto *Dir = dyn_cast(Transform))
- DependentPreInits = Dir->getPreInits();
-else if (auto *Dir = dyn_cast(Transform))
- DependentPreInits = Dir->getPreInits();
-else if (auto *Dir = dyn_cast(Transform))
- DependentPreInits = Dir->getPreInits();
-else if (auto *Dir = dyn_cast(Transform))
- DependentPreInits = Dir->getPreInits();
-else if (auto *Dir = dyn_cast(Transform))
- DependentPreInits = Dir->getPreInits();
-else
- llvm_unreachable("Unhandled loop transformation");
-
-appendFlattenedStmtList(OriginalInits.back(), DependentPreInits);
+updatePreInits(Transform, OriginalInits);
});
assert(OriginalInits.back().empty() && "No preinit after innermost loop");
OriginalInits.pop_back();
return Result;
}
+// Counts the total number of nested loops, including the outermost loop (the
+// original loop). PRECONDITION of this visitor is that it must be invoked from
+// the original loop to be analyzed. The traversal is stop for Decl's and
+// Expr's given that they may contain inner loops that must not be counted.
+//
+// Example AST structure for the code:
+//
+// int main() {
+// #pragma omp fuse
+// {
+// for (int i = 0; i < 100; i++) {<-- Outer loop
+// []() {
+// for(int j = 0; j < 100; j++) {} <-- NOT A LOOP
+// };
+// for(int j = 0; j < 5; ++j) {}<-- Inner loop
+// }
+// for (int r = 0; i < 100; i++) {<-- Outer loop
+// struct LocalClass {
+// void bar() {
+// for(int j = 0; j < 100; j++) {} <-- NOT A LOOP
+// }
+// };
+// for(int k = 0; k < 10; ++k) {}<-- Inner loop
+// {x = 5; for(k = 0; k < 10; ++k) x += k; x}; <-- NOT A LOOP
+// }
+// }
+// }
+// Result: Loop 'i' contains 2 loops, Loop 'r' also contains 2 loops
+class NestedLoopCounterVisitor : public DynamicRecursiveASTVisitor {
+private:
+ unsigned NestedLoopCount = 0;
+
+public:
+ explicit NestedLoopCounterVisitor() {}
+
+ unsigned getNestedLoopCount() const { return NestedLoopCount; }
+
+ bool VisitForStmt(ForStmt *FS) override {
+++NestedLoopCount;
+return true;
+ }
+
+ bool VisitCXXForRangeStmt(CXXForRangeStmt *FRS) override {
+++NestedLoopCount;
+return true;
+ }
+
+ bool TraverseStmt(Stmt *S) override {
+if (!S)
+ return true;
+
+// Skip traversal of all expressions, including special cases like
+// LambdaExpr, StmtExpr, BlockExpr, and RequiresExpr. These expressions
+// may contain inner statements (and even loops), but they are not part
+// of the syntactic body of the surrounding loop structure.
+// Therefore must not be counted
+if (isa(S))
+ return true;
+
+// Only recurse into CompoundStmt (block {}) and loop bodies
+if (isa(S) || isa(S) || isa(S)) {
+ return DynamicRecursiveASTVisitor::TraverseStmt(S);
+}
+
+// Stop traversal of the rest of statements, that break perfect
+// loop nesting, such as control flow (IfStmt, SwitchStmt...)
+return true;
+ }
+
+ bool TraverseDecl(Decl *D) override {
+// Stop in the case of finding a declaration, it is not important
+// in order to find nested loops (Possible CXXRecordDecl, RecordDecl,
+// FunctionDecl...)
+return true;
+ }
+};
+
+bool SemaOpenMP::analyzeLoopSequence(
+Stmt *LoopSeqStmt, unsigned &LoopSeqSize, unsigned &NumLoops,
+SmallVectorImpl &LoopHelpers,
+SmallVectorImpl &ForStmts,
+SmallVectorImpl> &OriginalInits,
+SmallVectorImpl> &TransformsPreInits,
+SmallVectorImpl> &LoopSequencePreInits,
+SmallVectorImpl &LoopCategories, ASTContext &Context,
+OpenMPDirectiveKind Kind) {
+
+ VarsWithInheritedDSAType TmpDSA;
+ QualType BaseInductionVarType;
+ // Helper Lambda to handle storing initialization and body statements for
both
+ // ForStmt and CXXForRangeStmt and checks for any possible mismatch between
+ // induction variables types
+ auto storeLoopStatements = [&OriginalInits, &ForStmts, &BaseInductionVarType,
+ this, &Context](Stmt *LoopStmt) {
+if (auto *For = dyn_cast(LoopStmt)) {
+ OriginalInits.back().push_back(For->getInit());
+ ForStmts.push_back(For);
+ // Extract induction variable
+ if (auto *InitStmt = dyn_cast_or_null(For->getInit())) {
+if (auto *InitDecl = dyn_cast(InitStmt->getSingleDecl())) {
+ QualType InductionVarType = InitDecl->getType().getCanonicalType();
+
+ // Compare with first loop type
+ if (BaseInductionVarType.isNull()) {
+BaseInductionVarType = InductionVarT
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -11516,6 +11516,21 @@ def note_omp_implicit_dsa : Note< "implicitly determined as %0">; def err_omp_loop_var_dsa : Error< "loop iteration variable in the associated loop of 'omp %1' directive may not be %0, predetermined as %2">; +def warn_omp_different_loop_ind_var_types : Warning < + "loop sequence following '#pragma omp %0' contains induction variables of differing types: %1 and %2">, eZWALT wrote: How could it be? The iteration var type is dependent on the original iteration variable type, therefore making it possible for fusion to loop multiple loops with different induction variable types given that i dont emit an error but rather a warning. But i dont really see how NumIterations could be the limiting factor here, could you please explain me? https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -14175,27 +14222,350 @@ bool SemaOpenMP::checkTransformableLoopNest(
return false;
},
[&OriginalInits](OMPLoopBasedDirective *Transform) {
-Stmt *DependentPreInits;
-if (auto *Dir = dyn_cast(Transform))
- DependentPreInits = Dir->getPreInits();
-else if (auto *Dir = dyn_cast(Transform))
- DependentPreInits = Dir->getPreInits();
-else if (auto *Dir = dyn_cast(Transform))
- DependentPreInits = Dir->getPreInits();
-else if (auto *Dir = dyn_cast(Transform))
- DependentPreInits = Dir->getPreInits();
-else if (auto *Dir = dyn_cast(Transform))
- DependentPreInits = Dir->getPreInits();
-else
- llvm_unreachable("Unhandled loop transformation");
-
-appendFlattenedStmtList(OriginalInits.back(), DependentPreInits);
+updatePreInits(Transform, OriginalInits);
});
assert(OriginalInits.back().empty() && "No preinit after innermost loop");
OriginalInits.pop_back();
return Result;
}
+// Counts the total number of nested loops, including the outermost loop (the
+// original loop). PRECONDITION of this visitor is that it must be invoked from
+// the original loop to be analyzed. The traversal is stop for Decl's and
+// Expr's given that they may contain inner loops that must not be counted.
+//
+// Example AST structure for the code:
+//
+// int main() {
+// #pragma omp fuse
+// {
+// for (int i = 0; i < 100; i++) {<-- Outer loop
+// []() {
+// for(int j = 0; j < 100; j++) {} <-- NOT A LOOP
+// };
+// for(int j = 0; j < 5; ++j) {}<-- Inner loop
+// }
+// for (int r = 0; i < 100; i++) {<-- Outer loop
+// struct LocalClass {
+// void bar() {
+// for(int j = 0; j < 100; j++) {} <-- NOT A LOOP
+// }
+// };
+// for(int k = 0; k < 10; ++k) {}<-- Inner loop
+// {x = 5; for(k = 0; k < 10; ++k) x += k; x}; <-- NOT A LOOP
+// }
+// }
+// }
+// Result: Loop 'i' contains 2 loops, Loop 'r' also contains 2 loops
+class NestedLoopCounterVisitor : public DynamicRecursiveASTVisitor {
+private:
+ unsigned NestedLoopCount = 0;
+
+public:
+ explicit NestedLoopCounterVisitor() {}
+
+ unsigned getNestedLoopCount() const { return NestedLoopCount; }
+
+ bool VisitForStmt(ForStmt *FS) override {
+++NestedLoopCount;
+return true;
+ }
+
+ bool VisitCXXForRangeStmt(CXXForRangeStmt *FRS) override {
+++NestedLoopCount;
+return true;
+ }
+
+ bool TraverseStmt(Stmt *S) override {
+if (!S)
+ return true;
+
+// Skip traversal of all expressions, including special cases like
+// LambdaExpr, StmtExpr, BlockExpr, and RequiresExpr. These expressions
+// may contain inner statements (and even loops), but they are not part
+// of the syntactic body of the surrounding loop structure.
+// Therefore must not be counted
+if (isa(S))
+ return true;
+
+// Only recurse into CompoundStmt (block {}) and loop bodies
+if (isa(S) || isa(S) || isa(S)) {
+ return DynamicRecursiveASTVisitor::TraverseStmt(S);
+}
+
+// Stop traversal of the rest of statements, that break perfect
+// loop nesting, such as control flow (IfStmt, SwitchStmt...)
+return true;
+ }
+
+ bool TraverseDecl(Decl *D) override {
+// Stop in the case of finding a declaration, it is not important
+// in order to find nested loops (Possible CXXRecordDecl, RecordDecl,
+// FunctionDecl...)
+return true;
+ }
+};
+
+bool SemaOpenMP::analyzeLoopSequence(
+Stmt *LoopSeqStmt, unsigned &LoopSeqSize, unsigned &NumLoops,
+SmallVectorImpl &LoopHelpers,
+SmallVectorImpl &ForStmts,
+SmallVectorImpl> &OriginalInits,
+SmallVectorImpl> &TransformsPreInits,
+SmallVectorImpl> &LoopSequencePreInits,
+SmallVectorImpl &LoopCategories, ASTContext &Context,
+OpenMPDirectiveKind Kind) {
+
+ VarsWithInheritedDSAType TmpDSA;
+ QualType BaseInductionVarType;
+ // Helper Lambda to handle storing initialization and body statements for
both
+ // ForStmt and CXXForRangeStmt and checks for any possible mismatch between
+ // induction variables types
+ auto storeLoopStatements = [&OriginalInits, &ForStmts, &BaseInductionVarType,
+ this, &Context](Stmt *LoopStmt) {
+if (auto *For = dyn_cast(LoopStmt)) {
+ OriginalInits.back().push_back(For->getInit());
+ ForStmts.push_back(For);
+ // Extract induction variable
+ if (auto *InitStmt = dyn_cast_or_null(For->getInit())) {
+if (auto *InitDecl = dyn_cast(InitStmt->getSingleDecl())) {
+ QualType InductionVarType = InitDecl->getType().getCanonicalType();
+
+ // Compare with first loop type
+ if (BaseInductionVarType.isNull()) {
+BaseInductionVarType = InductionVarT
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -1480,6 +1493,108 @@ class SemaOpenMP : public SemaBase {
SmallVectorImpl &LoopHelpers,
Stmt *&Body, SmallVectorImpl> &OriginalInits);
+ /// @brief Categories of loops encountered during semantic OpenMP loop
+ /// analysis
+ ///
+ /// This enumeration identifies the structural category of a loop or sequence
+ /// of loops analyzed in the context of OpenMP transformations and
directives.
+ /// This categorization helps differentiate between original source loops
+ /// and the structures resulting from applying OpenMP loop transformations.
+ enum class OMPLoopCategory {
+
+/// @var OMPLoopCategory::RegularLoop
+/// Represents a standard canonical loop nest found in the
+/// original source code or an intact loop after transformations
+/// (i.e Post/Pre loops of a loopranged fusion)
+RegularLoop,
+
+/// @var OMPLoopCategory::TransformSingleLoop
+/// Represents the resulting loop structure when an OpenMP loop
+// transformation, generates a single, top-level loop
+TransformSingleLoop,
+
+/// @var OMPLoopCategory::TransformLoopSequence
+/// Represents the resulting loop structure when an OpenMP loop
+/// transformation
+/// generates a sequence of two or more canonical loop nests
+TransformLoopSequence
+ };
+
+ /// The main recursive process of `checkTransformableLoopSequence` that
+ /// performs grammatical parsing of a canonical loop sequence. It extracts
+ /// key information, such as the number of top-level loops, loop statements,
+ /// helper expressions, and other relevant loop-related data, all in a single
+ /// execution to avoid redundant traversals. This analysis flattens inner
+ /// Loop Sequences
+ ///
+ /// \param LoopSeqStmtThe AST of the original statement.
+ /// \param LoopSeqSize[out] Number of top level canonical loops.
+ /// \param NumLoops [out] Number of total canonical loops (nested too).
+ /// \param LoopHelpers[out] The multiple loop analyses results.
+ /// \param ForStmts [out] The multiple Stmt of each For loop.
+ /// \param OriginalInits [out] The raw original initialization statements
+ /// of each belonging to a loop of the loop sequence
+ /// \param TransformPreInits [out] The multiple collection of statements and
+ /// declarations that must have been executed/declared
+ /// before entering the loop (each belonging to a
+ /// particular loop transformation, nullptr otherwise)
+ /// \param LoopSequencePreInits [out] Additional general collection of loop
+ /// transformation related statements and declarations
+ /// not bounded to a particular loop that must be
+ /// executed before entering the loop transformation
+ /// \param LoopCategories [out] A sequence of OMPLoopCategory values,
+ /// one for each loop or loop transformation node
+ /// successfully analyzed.
+ /// \param Context
+ /// \param Kind The loop transformation directive kind.
+ /// \return Whether the original statement is both syntactically and
+ /// semantically correct according to OpenMP 6.0 canonical loop
+ /// sequence definition.
+ bool analyzeLoopSequence(
+ Stmt *LoopSeqStmt, unsigned &LoopSeqSize, unsigned &NumLoops,
+ SmallVectorImpl &LoopHelpers,
+ SmallVectorImpl &ForStmts,
+ SmallVectorImpl> &OriginalInits,
+ SmallVectorImpl> &TransformsPreInits,
+ SmallVectorImpl> &LoopSequencePreInits,
eZWALT wrote:
it is correct though, in order to be consistent with the rest of the codebase,
there are several instances of this SmallVector being propagated (In
checkTransformableLoopNest, ActOnOpenMPUnroll, ActOnOpenMPStripe...) therefore
i either modify all of these instances or i keep it as it is.
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -1151,6 +1151,106 @@ class OMPFullClause final : public
OMPNoChildClause {
static OMPFullClause *CreateEmpty(const ASTContext &C);
};
+/// This class represents the 'looprange' clause in the
+/// '#pragma omp fuse' directive
+///
+/// \code {c}
+/// #pragma omp fuse looprange(1,2)
+/// {
+/// for(int i = 0; i < 64; ++i)
+/// for(int j = 0; j < 256; j+=2)
+/// for(int k = 127; k >= 0; --k)
+/// \endcode
+class OMPLoopRangeClause final : public OMPClause {
+ friend class OMPClauseReader;
+
+ explicit OMPLoopRangeClause()
+ : OMPClause(llvm::omp::OMPC_looprange, {}, {}) {}
+
+ /// Location of '('
+ SourceLocation LParenLoc;
+
+ /// Location of 'first'
+ SourceLocation FirstLoc;
+
+ /// Location of 'count'
+ SourceLocation CountLoc;
+
+ /// Expr associated with 'first' argument
+ Expr *First = nullptr;
+
+ /// Expr associated with 'count' argument
+ Expr *Count = nullptr;
+
+ /// Set 'first'
+ void setFirst(Expr *First) { this->First = First; }
+
+ /// Set 'count'
+ void setCount(Expr *Count) { this->Count = Count; }
+
+ /// Set location of '('.
+ void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; }
+
+ /// Set location of 'first' argument
+ void setFirstLoc(SourceLocation Loc) { FirstLoc = Loc; }
+
+ /// Set location of 'count' argument
+ void setCountLoc(SourceLocation Loc) { CountLoc = Loc; }
+
+public:
+ /// Build an AST node for a 'looprange' clause
+ ///
+ /// \param StartLoc Starting location of the clause.
+ /// \param LParenLocLocation of '('.
+ /// \param ModifierLoc Modifier location.
+ /// \param
+ static OMPLoopRangeClause *
+ Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation
LParenLoc,
+ SourceLocation FirstLoc, SourceLocation CountLoc,
+ SourceLocation EndLoc, Expr *First, Expr *Count);
+
+ /// Build an empty 'looprange' node for deserialization
+ ///
+ /// \param C Context of the AST.
+ static OMPLoopRangeClause *CreateEmpty(const ASTContext &C);
+
+ /// Returns the location of '('
+ SourceLocation getLParenLoc() const { return LParenLoc; }
+
+ /// Returns the location of 'first'
+ SourceLocation getFirstLoc() const { return FirstLoc; }
+
+ /// Returns the location of 'count'
+ SourceLocation getCountLoc() const { return CountLoc; }
+
+ /// Returns the argument 'first' or nullptr if not set
+ Expr *getFirst() const { return cast_or_null(First); }
+
+ /// Returns the argument 'count' or nullptr if not set
+ Expr *getCount() const { return cast_or_null(Count); }
+
+ child_range children() {
+return child_range(reinterpret_cast(&First),
+ reinterpret_cast(&Count) + 1);
eZWALT wrote:
Well spotted, thank you!
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
eZWALT wrote: Gentle-ping, I'm not sure if GitHub has notified you of the comments :) @alexey-bataev https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -962,6 +962,9 @@ class OMPLoopTransformationDirective : public
OMPLoopBasedDirective {
/// Number of loops generated by this loop transformation.
unsigned NumGeneratedLoops = 0;
+ /// Number of top level canonical loop nests generated by this loop
+ /// transformation
+ unsigned NumGeneratedLoopNests = 0;
eZWALT wrote:
This distinction is indeed important and actively used in `SemaOpenMP.cpp`
file, particularly within the `AnalyzeLoopSequence` function (starting at line
14284). For example, it's referenced in lines 14344 and 14364 to differentiate
between specific loop transformations.
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -11516,6 +11516,21 @@ def note_omp_implicit_dsa : Note< "implicitly determined as %0">; def err_omp_loop_var_dsa : Error< "loop iteration variable in the associated loop of 'omp %1' directive may not be %0, predetermined as %2">; +def warn_omp_different_loop_ind_var_types : Warning < + "loop sequence following '#pragma omp %0' contains induction variables of differing types: %1 and %2">, + InGroup; +def err_omp_not_canonical_loop : Error < + "loop after '#pragma omp %0' is not in canonical form">; +def err_omp_not_a_loop_sequence : Error < + "statement after '#pragma omp %0' must be a loop sequence containing canonical loops or loop-generating constructs">; +def err_omp_empty_loop_sequence : Error < + "loop sequence after '#pragma omp %0' must contain at least 1 canonical loop or loop-generating construct">; +def err_omp_invalid_looprange : Error < + "loop range in '#pragma omp %0' exceeds the number of available loops: " + "range end '%1' is greater than the total number of loops '%2'">; eZWALT wrote: The two errors serve different purposes: 1. The first is triggered when the loop sequence inside a fusion construct (full or ranged) contains no loops. 2. The second is specific to loopranged fusion and reports when the specified range exceeds the number of available loops. While the first case is technically a subset of the second, they occur in different contexts. Keeping both improves clarity and helps users better understand the issue. That said, I’m open to refactoring if you'd prefer a single, more general diagnostic, though it may reduce the precision of the error messages. https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
https://github.com/eZWALT created
https://github.com/llvm/llvm-project/pull/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.
>From affda91204c1aacdab8ebd0966a27e93feec6db3 Mon Sep 17 00:00:00 2001
From: eZWALT
Date: Mon, 19 May 2025 10:49:10 +
Subject: [PATCH] Correct the number of generated loops
---
clang/include/clang/AST/StmtOpenMP.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/AST/StmtOpenMP.h
b/clang/include/clang/AST/StmtOpenMP.h
index 736bcabbad1f7..7ded194dd6eb2 100644
--- a/clang/include/clang/AST/StmtOpenMP.h
+++ b/clang/include/clang/AST/StmtOpenMP.h
@@ -5790,7 +5790,9 @@ class OMPReverseDirective final : public
OMPLoopTransformationDirective {
explicit OMPReverseDirective(SourceLocation StartLoc, SourceLocation EndLoc)
: OMPLoopTransformationDirective(OMPReverseDirectiveClass,
llvm::omp::OMPD_reverse, StartLoc,
- EndLoc, 1) {}
+ EndLoc, 1) {
+setNumGeneratedLoops(1);
+ }
void setPreInits(Stmt *PreInits) {
Data->getChildren()[PreInitsOffset] = PreInits;
@@ -5857,7 +5859,7 @@ class OMPInterchangeDirective final : public
OMPLoopTransformationDirective {
: OMPLoopTransformationDirective(OMPInterchangeDirectiveClass,
llvm::omp::OMPD_interchange, StartLoc,
EndLoc, NumLoops) {
-setNumGeneratedLoops(3 * NumLoops);
+setNumGeneratedLoops(NumLoops);
}
void setPreInits(Stmt *PreInits) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: > > AnalyzeLoopSequence > > Could you point, where this is located? Of course, [See line 14315 in `SemaOpenMP.cpp`](https://github.com/eZWALT/llvm-project/blob/main/clang/lib/Sema/SemaOpenMP.cpp#L14315) https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: > > AnalyzeLoopSequence > > Could you point, where this is located? Although now that i think about it, this function is not inside this PR but rather on the PR #139293, but the dependency is clear. https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: gentle ping @alexey-bataev https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
eZWALT wrote: gentle ping @alexey-bataev https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
https://github.com/eZWALT updated
https://github.com/llvm/llvm-project/pull/140532
>From affda91204c1aacdab8ebd0966a27e93feec6db3 Mon Sep 17 00:00:00 2001
From: eZWALT
Date: Mon, 19 May 2025 10:49:10 +
Subject: [PATCH] Correct the number of generated loops
---
clang/include/clang/AST/StmtOpenMP.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/AST/StmtOpenMP.h
b/clang/include/clang/AST/StmtOpenMP.h
index 736bcabbad1f7..7ded194dd6eb2 100644
--- a/clang/include/clang/AST/StmtOpenMP.h
+++ b/clang/include/clang/AST/StmtOpenMP.h
@@ -5790,7 +5790,9 @@ class OMPReverseDirective final : public
OMPLoopTransformationDirective {
explicit OMPReverseDirective(SourceLocation StartLoc, SourceLocation EndLoc)
: OMPLoopTransformationDirective(OMPReverseDirectiveClass,
llvm::omp::OMPD_reverse, StartLoc,
- EndLoc, 1) {}
+ EndLoc, 1) {
+setNumGeneratedLoops(1);
+ }
void setPreInits(Stmt *PreInits) {
Data->getChildren()[PreInitsOffset] = PreInits;
@@ -5857,7 +5859,7 @@ class OMPInterchangeDirective final : public
OMPLoopTransformationDirective {
: OMPLoopTransformationDirective(OMPInterchangeDirectiveClass,
llvm::omp::OMPD_interchange, StartLoc,
EndLoc, NumLoops) {
-setNumGeneratedLoops(3 * NumLoops);
+setNumGeneratedLoops(NumLoops);
}
void setPreInits(Stmt *PreInits) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: After conducting an examination of the directive handling logic, I can confidently state that the number of generated loops (`NumGeneratedLoops`) does not affect the semantic checks for the majority of transformations. This is because values are usually hardcoded in the `ActOnXXX` semantic handlers. For example: - In the case of the `'reverse'` directive, the number of loops (`NumLoops`) is hardcoded to `1`, meaning it remains unaffected by any external loop count logic. - For the `'interchange'` directive, the number of loops is also explicitly set using the following logic: ``` size_t NumLoops = PermutationClause ? PermutationClause->getNumLoops() : 2; ``` These values are passed into the `checkTransformableLoopNest` function and are not accessed elsewhere in the codebase, except: - In `doForAllLoops`, where only the **presence** of loops is checked (i.e., `NumLoops == 0` or `> 0`), not the actual count, therefore this change won't break this conditional flow. - In the newly introduced analysis functions (as part of the `'fuse'` transformation: #139293), specifically within `checkTransformableLoopSequence`, where both `NumGeneratedLoops` and `NumGeneratedLoopNests` are read and actively utilized. https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
https://github.com/eZWALT edited https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
https://github.com/eZWALT edited https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -1480,6 +1493,108 @@ class SemaOpenMP : public SemaBase {
SmallVectorImpl &LoopHelpers,
Stmt *&Body, SmallVectorImpl> &OriginalInits);
+ /// @brief Categories of loops encountered during semantic OpenMP loop
+ /// analysis
+ ///
+ /// This enumeration identifies the structural category of a loop or sequence
+ /// of loops analyzed in the context of OpenMP transformations and
directives.
+ /// This categorization helps differentiate between original source loops
+ /// and the structures resulting from applying OpenMP loop transformations.
+ enum class OMPLoopCategory {
+
+/// @var OMPLoopCategory::RegularLoop
+/// Represents a standard canonical loop nest found in the
+/// original source code or an intact loop after transformations
+/// (i.e Post/Pre loops of a loopranged fusion)
+RegularLoop,
+
+/// @var OMPLoopCategory::TransformSingleLoop
+/// Represents the resulting loop structure when an OpenMP loop
+// transformation, generates a single, top-level loop
+TransformSingleLoop,
+
+/// @var OMPLoopCategory::TransformLoopSequence
+/// Represents the resulting loop structure when an OpenMP loop
+/// transformation
+/// generates a sequence of two or more canonical loop nests
+TransformLoopSequence
+ };
+
+ /// The main recursive process of `checkTransformableLoopSequence` that
+ /// performs grammatical parsing of a canonical loop sequence. It extracts
+ /// key information, such as the number of top-level loops, loop statements,
+ /// helper expressions, and other relevant loop-related data, all in a single
+ /// execution to avoid redundant traversals. This analysis flattens inner
+ /// Loop Sequences
+ ///
+ /// \param LoopSeqStmtThe AST of the original statement.
+ /// \param LoopSeqSize[out] Number of top level canonical loops.
+ /// \param NumLoops [out] Number of total canonical loops (nested too).
+ /// \param LoopHelpers[out] The multiple loop analyses results.
+ /// \param ForStmts [out] The multiple Stmt of each For loop.
+ /// \param OriginalInits [out] The raw original initialization statements
+ /// of each belonging to a loop of the loop sequence
+ /// \param TransformPreInits [out] The multiple collection of statements and
+ /// declarations that must have been executed/declared
+ /// before entering the loop (each belonging to a
+ /// particular loop transformation, nullptr otherwise)
+ /// \param LoopSequencePreInits [out] Additional general collection of loop
+ /// transformation related statements and declarations
+ /// not bounded to a particular loop that must be
+ /// executed before entering the loop transformation
+ /// \param LoopCategories [out] A sequence of OMPLoopCategory values,
+ /// one for each loop or loop transformation node
+ /// successfully analyzed.
+ /// \param Context
+ /// \param Kind The loop transformation directive kind.
+ /// \return Whether the original statement is both syntactically and
+ /// semantically correct according to OpenMP 6.0 canonical loop
+ /// sequence definition.
+ bool analyzeLoopSequence(
+ Stmt *LoopSeqStmt, unsigned &LoopSeqSize, unsigned &NumLoops,
+ SmallVectorImpl &LoopHelpers,
+ SmallVectorImpl &ForStmts,
+ SmallVectorImpl> &OriginalInits,
+ SmallVectorImpl> &TransformsPreInits,
+ SmallVectorImpl> &LoopSequencePreInits,
eZWALT wrote:
At the end i've decided to modify all of them and avoid this rigid fixed logic
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -0,0 +1,186 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -std=c++20 -fopenmp
-fopenmp-version=60 -fsyntax-only -Wuninitialized -verify %s
+
+void func() {
+
+// expected-error@+2 {{statement after '#pragma omp fuse' must be a loop
sequence containing canonical loops or loop-generating constructs}}
+#pragma omp fuse
+;
+
+// expected-error@+2 {{statement after '#pragma omp fuse' must be a for
loop}}
+#pragma omp fuse
+{int bar = 0;}
+
+// expected-error@+4 {{statement after '#pragma omp fuse' must be a for
loop}}
+#pragma omp fuse
+{
+for(int i = 0; i < 10; ++i);
+int x = 2;
+}
+
+// expected-error@+2 {{statement after '#pragma omp fuse' must be a loop
sequence containing canonical loops or loop-generating constructs}}
+#pragma omp fuse
+#pragma omp for
+for (int i = 0; i < 7; ++i)
+;
+
+{
+// expected-error@+2 {{expected statement}}
+#pragma omp fuse
+}
+
+// expected-warning@+1 {{extra tokens at the end of '#pragma omp fuse' are
ignored}}
+#pragma omp fuse foo
+{
+for (int i = 0; i < 7; ++i)
+;
+for(int j = 0; j < 100; ++j);
+
+}
+
+
+// expected-error@+1 {{unexpected OpenMP clause 'final' in directive
'#pragma omp fuse'}}
+#pragma omp fuse final(0)
+{
+for (int i = 0; i < 7; ++i)
+;
+for(int j = 0; j < 100; ++j);
+
+}
+
+//expected-error@+4 {{loop after '#pragma omp fuse' is not in canonical
form}}
eZWALT wrote:
Is not that i need it, is that is emitted independently from the 1st one. I've
simply added the first one, the 2nd is emitted probably from some
CheckOpenMPLoop or other constraint checking function. Nevertheless, it is nice
to have such detailed information althought i understand that in the tradeoff,
it can result a bit verbose.
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: > It would be good to try to find the cases that may reveal this issues before > committing the patch Yes, i'll go through loop transformations tests and notify you tomorrow if this is the case, but i'm pretty sure that these are not breaking changes for the same reason that i told you on the other PR, NumGeneratedLoops is almost only being used in the CheckTransformableLoopNest and well i will have to check both ActOnOMPReverse/InterchangeDirective https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: Before leaving i can attest that the regression tests have been passed twice :+1: https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: > Are there any tests that might be affected by this change? Yesterday I ran all the tests (check-clang-openmp and check-clang) and no change in the behaviour or incidence was found, although i'll re-execute them just as a sanity check. Just a remainder but this merge request should be merged firstly before #139293. https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: > > @alexey-bataev After conducting an examination of the directive handling > > logic, I can confidently state that the number of generated loops > > (`NumGeneratedLoops`) does not affect the semantic checks for the majority > > of transformations. This is because values are usually hardcoded in the > > `ActOnXXX` semantic handlers. For example: > > > > * In the case of the `'reverse'` directive, the number of loops > > (`NumLoops`) is hardcoded to `1`, meaning it remains unaffected by any > > external loop count logic. > > * For the `'interchange'` directive, the number of loops is also explicitly > > set using the following logic: > > > > ``` > > size_t NumLoops = PermutationClause ? PermutationClause->getNumLoops() : 2; > > ``` > > > > > > > > > > > > > > > > > > > > > > > > These values are passed into the `checkTransformableLoopNest` function and > > are not accessed elsewhere in the codebase, except: > > > > * In `doForAllLoops`, where only the **presence** of loops is checked > > (i.e., `NumLoops == 0` or `> 0`), not the actual count, therefore this > > change won't break this conditional flow. > > * In the newly introduced analysis functions (as part of the `'fuse'` > > transformation: [[Clang][OpenMP][LoopTransformations] Add support for > > "#pragma omp fuse" loop transformation directive and "looprange" clause > > #139293](https://github.com/llvm/llvm-project/pull/139293)), specifically > > within `checkTransformableLoopSequence`, where both `NumGeneratedLoops` and > > `NumGeneratedLoopNests` are read and actively utilized. > > Can you remove these hardcoded values and use the stored value instead? > Otherwise, it is meaningless and should be removed But this rigidity stems from the `checkTransformableLoopNest`, which needs the number of loops to be specified beforehand. Changing this wouldn't make much sense. The `NumGeneratedLoops` information is only available after the creation of the `OMPLoopTransformation` AST nodes, but the number of loops must be known before that. Note that inferring this knowledge is trivial in the old scheme of loop transformations, since almost all have one top-level loop, or the loop count is specified by a clause , for example, `PermutationClause`. `OMPFuseDirective` is the only one where the number of loops (both top-level and nested) is dynamic, depending on the user code. Therefore, it is mandatory to do an analysis to gather the shape of the loop sequence. Probably I haven’t explained myself well enough, but I want to stress the difference: - `NumLoops` refers to the loops *expected* or known beforehand — which, in most directives, can be hardcoded. - `NumGeneratedLoops` is the total number of loops *after* transformation, stored as semantic information in the AST. - `NumGeneratedLoopNests` is the number of top-level loop nests, which is important for loop sequence transformations like `fuse` or `split`. **Summing up**: preserving this semantic information is important, and there’s no reason to change it right now. Feel free to digress, but I’m confident this is the right approach. https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: > What I see in the source code that it is used as a boolean flag. Can we > transform it to bool? There is no need to keep it integer Please could you cite the exact line? I'm not sure if you are refering to the logic inside checkTransformableLoopNest or not. https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: > > > What I see in the source code that it is used as a boolean flag. Can we > > > transform it to bool? There is no need to keep it integer > > > > > > Please could you cite the exact line? I'm not sure if you are refering to > > the logic inside checkTransformableLoopNest or not. > > I check getNumGeneratedLoops() function usage. I see that it is used only in > 2 linesб which can be replaced by just a boolean We could add a boolean function like 'AreThereGeneratedLoops()', but getGeneratedNumLoops() is also used to count the total loops inside 'AnalyzeLoopSequence', which feeds into NumGeneratedLoops in OMPFuseDirective. Changing its return type would break that. While we could remove NumGeneratedLoops out of OMPLoopTransformation AST nodes, it provides useful semantic flexibility for future transformations. There’s a tradeoff, but I believe keeping it does more good than harm. https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
eZWALT wrote: @alexey-bataev not sure what happened before with this build system, but now everything works as expected. Thanks for the fast replies and have a nice weekend! https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -11516,6 +11516,21 @@ def note_omp_implicit_dsa : Note< "implicitly determined as %0">; def err_omp_loop_var_dsa : Error< "loop iteration variable in the associated loop of 'omp %1' directive may not be %0, predetermined as %2">; +def warn_omp_different_loop_ind_var_types : Warning < + "loop sequence following '#pragma omp %0' contains induction variables of differing types: %1 and %2">, eZWALT wrote: Nothing actually, it was one of the 3 ideas i had to tackle this problem and if you prefer i will follow this approach (Using uint of 64 bits for example, however your comment left me thinking if NumIterations cause any trouble, i think it can't but i'm not 100% sure to be honest. https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
https://github.com/eZWALT edited https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: gentle ping :) https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: > > I originally kept the `NumGeneratedLoops` information consistent despite > > being partially subsumed by NumGeneratedLoopNests (Note that its not > > actually the same, it returns the number of generated loops in total adding > > nested loops, but due to the current usage of this semantic information I > > could argue they serve the same purpose) just in case this information was > > going to be used for future OpenMP Transformations or semantic logic maybe > > in OpenMP 7.0. If you both think this information about nested loops will > > never be used (Maybe if fusion gets a clause for multi-level fusion / > > fission could become relevant...), then I just remove it, but instead of > > storing a boolean value, a boolean function `hasGeneratedLoops` = > > `self->NumGeneratedLoops > 0` could be coded. > > After revising this topic thoroughly, I believe the most reasonable course > > of action would be to close this PR and keep `NumGeneratedLoops` as it is > > currently in this patch. Then, swap the semantic information of > > NumGeneratedLoops <-> `NumGeneratedLoopNests` and remove > > NumGeneratedLoopNests in patch #139293 . This would also enable me to > > remove unnecessary computations that were performed in > > `AnalyzeLoopSequence` to keep `NumGeneratedLoops` consistent, simplifying > > logic and removing this second variable. > > @alexey-bataev , @Meinersbur what do you think? Will this information ever > > be needed? > > Thank you! But before accepting the merge, does this mean that we discard > everything that i said about refactoring these variables and just keep it as > it is in both reviews @alexey-bataev ? Also, could you @Meinersbur specify > what exactly do you mean with a regression test for such thing? You mean like > incorporating some kind of logic which includes NumGeneratedLoops in all > LoopTransformation directives or just tile and reverse? Just want to be sure > before proceeding with #139293, in the aforementioned case it would not need > further revision. I will assume no answer means yes, thank you! https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
https://github.com/eZWALT updated
https://github.com/llvm/llvm-project/pull/140532
>From affda91204c1aacdab8ebd0966a27e93feec6db3 Mon Sep 17 00:00:00 2001
From: eZWALT
Date: Mon, 19 May 2025 10:49:10 +
Subject: [PATCH] Correct the number of generated loops
---
clang/include/clang/AST/StmtOpenMP.h | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/AST/StmtOpenMP.h
b/clang/include/clang/AST/StmtOpenMP.h
index 736bcabbad1f7..7ded194dd6eb2 100644
--- a/clang/include/clang/AST/StmtOpenMP.h
+++ b/clang/include/clang/AST/StmtOpenMP.h
@@ -5790,7 +5790,9 @@ class OMPReverseDirective final : public
OMPLoopTransformationDirective {
explicit OMPReverseDirective(SourceLocation StartLoc, SourceLocation EndLoc)
: OMPLoopTransformationDirective(OMPReverseDirectiveClass,
llvm::omp::OMPD_reverse, StartLoc,
- EndLoc, 1) {}
+ EndLoc, 1) {
+setNumGeneratedLoops(1);
+ }
void setPreInits(Stmt *PreInits) {
Data->getChildren()[PreInitsOffset] = PreInits;
@@ -5857,7 +5859,7 @@ class OMPInterchangeDirective final : public
OMPLoopTransformationDirective {
: OMPLoopTransformationDirective(OMPInterchangeDirectiveClass,
llvm::omp::OMPD_interchange, StartLoc,
EndLoc, NumLoops) {
-setNumGeneratedLoops(3 * NumLoops);
+setNumGeneratedLoops(NumLoops);
}
void setPreInits(Stmt *PreInits) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: Thank you so much @Meinersbur ! https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
eZWALT wrote: > Thanks for the work and sorry I could not have a look into it earlier Nono, thank you for your time and guidance! Between today and tomorrow i'll upload the updated version, thanks for taking the time to improve it and the nitpicks! https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -1143,6 +1143,97 @@ class OMPFullClause final : public
OMPNoChildClause {
static OMPFullClause *CreateEmpty(const ASTContext &C);
};
+/// This class represents the 'looprange' clause in the
+/// '#pragma omp fuse' directive
+///
+/// \code {c}
+/// #pragma omp fuse looprange(1,2)
+/// {
+/// for(int i = 0; i < 64; ++i)
+/// for(int j = 0; j < 256; j+=2)
+/// for(int k = 127; k >= 0; --k)
+/// \endcode
+class OMPLoopRangeClause final
+: public OMPClause,
+ private llvm::TrailingObjects {
+ friend class OMPClauseReader;
+ friend class llvm::TrailingObjects;
+
+ /// Location of '('
+ SourceLocation LParenLoc;
+
+ /// Location of first and count expressions
+ SourceLocation FirstLoc, CountLoc;
+
+ /// Number of looprange arguments (always 2: first, count)
+ unsigned NumArgs = 2;
+
+ /// Set the argument expressions.
+ void setArgs(ArrayRef Args) {
+assert(Args.size() == NumArgs && "Expected exactly 2 looprange arguments");
+std::copy(Args.begin(), Args.end(), getTrailingObjects());
+ }
+
+ /// Build an empty clause for deserialization.
+ explicit OMPLoopRangeClause()
+ : OMPClause(llvm::omp::OMPC_looprange, {}, {}), NumArgs(2) {}
+
+public:
+ /// Build a 'looprange' clause AST node.
+ static OMPLoopRangeClause *
+ Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation
LParenLoc,
+ SourceLocation FirstLoc, SourceLocation CountLoc,
+ SourceLocation EndLoc, ArrayRef Args);
+
+ /// Build an empty 'looprange' clause node.
+ static OMPLoopRangeClause *CreateEmpty(const ASTContext &C);
+
+ // Location getters/setters
+ SourceLocation getLParenLoc() const { return LParenLoc; }
+ SourceLocation getFirstLoc() const { return FirstLoc; }
+ SourceLocation getCountLoc() const { return CountLoc; }
+
+ void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; }
+ void setFirstLoc(SourceLocation Loc) { FirstLoc = Loc; }
+ void setCountLoc(SourceLocation Loc) { CountLoc = Loc; }
+
+ /// Get looprange arguments: first and count
+ Expr *getFirst() const { return getArgs()[0]; }
+ Expr *getCount() const { return getArgs()[1]; }
eZWALT wrote:
I did not notice this, thank you!
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -508,6 +512,43 @@ OMPInterchangeDirective::CreateEmpty(const ASTContext &C,
unsigned NumClauses,
SourceLocation(), SourceLocation(), NumLoops);
}
+OMPFuseDirective *OMPFuseDirective::Create(
+const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc,
+ArrayRef Clauses, unsigned NumLoops, unsigned NumLoopNests,
+Stmt *AssociatedStmt, Stmt *TransformedStmt, Stmt *PreInits) {
+
+ OMPFuseDirective *Dir = createDirective(
+ C, Clauses, AssociatedStmt, TransformedStmtOffset + 1, StartLoc, EndLoc,
+ NumLoops);
+ Dir->setTransformedStmt(TransformedStmt);
+ Dir->setPreInits(PreInits);
+ // The number of top level canonical nests could
+ // not match the total number of generated loops
+ // Example:
+ // Before fusion:
+ // for (int i = 0; i < N; ++i)
+ // for (int j = 0; j < M; ++j)
+ // A[i][j] = i + j;
+ //
+ // for (int k = 0; k < P; ++k)
+ // B[k] = k * 2;
+ // Here, NumLoopNests = 2, but NumLoops = 3.
eZWALT wrote:
Yes I guessed that probably this information would not be used... However i
wanted to be consistent with other loop transformations and avoid changing the
structure for loop nest only related transformations, so I did the visitor just
for the sake of consistency. I know it might not be the best solution in terms
of redundancy and extra lines of code, but i believe its more consistent with
the rest of loop transformations. But again, you are 100% correct, the total
number of loops does not really mater without the loop nest structure.
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -3256,9 +3256,8 @@ LValue CodeGenFunction::EmitDeclRefLValue(const
DeclRefExpr *E) {
var, ConvertTypeForMem(VD->getType()),
getContext().getDeclAlign(VD));
// No other cases for now.
-} else {
+} else
llvm_unreachable("DeclRefExpr for Decl not entered in LocalDeclMap?");
-}
eZWALT wrote:
This should not be in the pr thanks!
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -3256,9 +3256,8 @@ LValue CodeGenFunction::EmitDeclRefLValue(const
DeclRefExpr *E) {
var, ConvertTypeForMem(VD->getType()),
getContext().getDeclAlign(VD));
// No other cases for now.
-} else {
+} else
llvm_unreachable("DeclRefExpr for Decl not entered in LocalDeclMap?");
-}
eZWALT wrote:
> This shouldn't be inside the pr thanks!
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
https://github.com/eZWALT deleted https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
https://github.com/eZWALT edited https://github.com/llvm/llvm-project/pull/139293 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -1479,7 +1492,109 @@ class SemaOpenMP : public SemaBase {
bool checkTransformableLoopNest(
OpenMPDirectiveKind Kind, Stmt *AStmt, int NumLoops,
SmallVectorImpl &LoopHelpers,
- Stmt *&Body, SmallVectorImpl> &OriginalInits);
+ Stmt *&Body, SmallVectorImpl> &OriginalInits);
+
+ /// @brief Categories of loops encountered during semantic OpenMP loop
+ /// analysis
+ ///
+ /// This enumeration identifies the structural category of a loop or sequence
+ /// of loops analyzed in the context of OpenMP transformations and
directives.
+ /// This categorization helps differentiate between original source loops
+ /// and the structures resulting from applying OpenMP loop transformations.
+ enum class OMPLoopCategory {
+
+/// @var OMPLoopCategory::RegularLoop
+/// Represents a standard canonical loop nest found in the
+/// original source code or an intact loop after transformations
+/// (i.e Post/Pre loops of a loopranged fusion)
+RegularLoop,
+
+/// @var OMPLoopCategory::TransformSingleLoop
+/// Represents the resulting loop structure when an OpenMP loop
+// transformation, generates a single, top-level loop
+TransformSingleLoop,
+
+/// @var OMPLoopCategory::TransformLoopSequence
+/// Represents the resulting loop structure when an OpenMP loop
+/// transformation
+/// generates a sequence of two or more canonical loop nests
+TransformLoopSequence
+ };
+
+ /// The main recursive process of `checkTransformableLoopSequence` that
+ /// performs grammatical parsing of a canonical loop sequence. It extracts
+ /// key information, such as the number of top-level loops, loop statements,
+ /// helper expressions, and other relevant loop-related data, all in a single
+ /// execution to avoid redundant traversals. This analysis flattens inner
+ /// Loop Sequences
+ ///
+ /// \param LoopSeqStmtThe AST of the original statement.
+ /// \param LoopSeqSize[out] Number of top level canonical loops.
+ /// \param NumLoops [out] Number of total canonical loops (nested too).
+ /// \param LoopHelpers[out] The multiple loop analyses results.
+ /// \param ForStmts [out] The multiple Stmt of each For loop.
+ /// \param OriginalInits [out] The raw original initialization statements
+ /// of each belonging to a loop of the loop sequence
+ /// \param TransformPreInits [out] The multiple collection of statements and
+ /// declarations that must have been executed/declared
+ /// before entering the loop (each belonging to a
+ /// particular loop transformation, nullptr otherwise)
+ /// \param LoopSequencePreInits [out] Additional general collection of loop
+ /// transformation related statements and declarations
+ /// not bounded to a particular loop that must be
+ /// executed before entering the loop transformation
+ /// \param LoopCategories [out] A sequence of OMPLoopCategory values,
+ /// one for each loop or loop transformation node
+ /// successfully analyzed.
+ /// \param Context
+ /// \param Kind The loop transformation directive kind.
+ /// \return Whether the original statement is both syntactically and
+ /// semantically correct according to OpenMP 6.0 canonical loop
+ /// sequence definition.
+ bool analyzeLoopSequence(
+ Stmt *LoopSeqStmt, unsigned &LoopSeqSize, unsigned &NumLoops,
+ SmallVectorImpl &LoopHelpers,
+ SmallVectorImpl &ForStmts,
+ SmallVectorImpl> &OriginalInits,
+ SmallVectorImpl> &TransformsPreInits,
+ SmallVectorImpl> &LoopSequencePreInits,
eZWALT wrote:
It was originally implemented that way, but I changed it after @alexey-bataev’s
suggestion. At the time, I didn’t fully understand the tradeoff and followed
the recommendation a bit blindly. Could you clarify the reasoning behind the
change? If it has a significant impact, I’ll be happy to revert or adjust
accordingly.
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -14189,10 +14196,49 @@ StmtResult
SemaOpenMP::ActOnOpenMPTargetTeamsDistributeSimdDirective(
getASTContext(), StartLoc, EndLoc, NestedLoopCount, Clauses, AStmt, B);
}
+/// Overloaded base case function
+template static bool tryHandleAs(T *t, F &&) {
+ return false;
+}
+
+///
+/// Tries to recursively cast `t` to one of the given types and invokes `f` if
+/// successful.
+///
+/// @tparam Class The first type to check.
+/// @tparam Rest The remaining types to check.
+/// @tparam T The base type of `t`.
+/// @tparam F The callable type for the function to invoke upon a successful
+/// cast.
+/// @param t The object to be checked.
+/// @param f The function to invoke if `t` matches `Class`.
+/// @return `true` if `t` matched any type and `f` was called, otherwise
+/// `false`.
+template
+static bool tryHandleAs(T *t, F &&f) {
+ if (Class *c = dyn_cast(t)) {
+f(c);
+return true;
+ }
+ return tryHandleAs(t, std::forward(f));
+}
+
+/// Updates OriginalInits by checking Transform against loop transformation
+/// directives and appending their pre-inits if a match is found.
+static void updatePreInits(OMPLoopBasedDirective *Transform,
+ SmallVectorImpl> &PreInits) {
+ if (!tryHandleAs(
+ Transform, [&PreInits](auto *Dir) {
+appendFlattenedStmtList(PreInits.back(), Dir->getPreInits());
+ }))
+llvm_unreachable("Unhandled loop transformation");
+}
+
bool SemaOpenMP::checkTransformableLoopNest(
OpenMPDirectiveKind Kind, Stmt *AStmt, int NumLoops,
SmallVectorImpl &LoopHelpers,
-Stmt *&Body, SmallVectorImpl> &OriginalInits) {
eZWALT wrote:
The same i told you before in the other comment, i refactored it to match the
suggestion of alexey.
https://github.com/llvm/llvm-project/pull/139293
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -15499,6 +15836,496 @@ StmtResult
SemaOpenMP::ActOnOpenMPInterchangeDirective(
buildPreInits(Context, PreInits));
}
+StmtResult SemaOpenMP::ActOnOpenMPFuseDirective(ArrayRef Clauses,
+Stmt *AStmt,
+SourceLocation StartLoc,
+SourceLocation EndLoc) {
+
+ ASTContext &Context = getASTContext();
+ DeclContext *CurrContext = SemaRef.CurContext;
+ Scope *CurScope = SemaRef.getCurScope();
+ CaptureVars CopyTransformer(SemaRef);
+
+ // Ensure the structured block is not empty
+ if (!AStmt)
+return StmtError();
+
+ unsigned NumLoops = 1;
+ unsigned LoopSeqSize = 1;
+
+ // Defer transformation in dependent contexts
+ // The NumLoopNests argument is set to a placeholder 1 (even though
+ // using looprange fuse could yield up to 3 top level loop nests)
+ // because a dependent context could prevent determining its true value
+ if (CurrContext->isDependentContext()) {
+return OMPFuseDirective::Create(Context, StartLoc, EndLoc, Clauses,
+NumLoops, LoopSeqSize, AStmt, nullptr,
+nullptr);
+ }
+
+ // Validate that the potential loop sequence is transformable for fusion
+ // Also collect the HelperExprs, Loop Stmts, Inits, and Number of loops
+ SmallVector LoopHelpers;
+ SmallVector LoopStmts;
+ SmallVector> OriginalInits;
+ SmallVector> TransformsPreInits;
+ SmallVector> LoopSequencePreInits;
+ SmallVector LoopCategories;
+ if (!checkTransformableLoopSequence(OMPD_fuse, AStmt, LoopSeqSize, NumLoops,
+ LoopHelpers, LoopStmts, OriginalInits,
+ TransformsPreInits, LoopSequencePreInits,
+ LoopCategories, Context))
+return StmtError();
+
+ // Handle clauses, which can be any of the following: [looprange, apply]
+ const OMPLoopRangeClause *LRC =
+ OMPExecutableDirective::getSingleClause(Clauses);
+
+ // The clause arguments are invalidated if any error arises
+ // such as non-constant or non-positive arguments
+ if (LRC && (!LRC->getFirst() || !LRC->getCount()))
+return StmtError();
+
+ // Delayed semantic check of LoopRange constraint
+ // Evaluates the loop range arguments and returns the first and count values
+ auto EvaluateLoopRangeArguments = [&Context](Expr *First, Expr *Count,
+ uint64_t &FirstVal,
+ uint64_t &CountVal) {
+llvm::APSInt FirstInt = First->EvaluateKnownConstInt(Context);
+llvm::APSInt CountInt = Count->EvaluateKnownConstInt(Context);
+FirstVal = FirstInt.getZExtValue();
+CountVal = CountInt.getZExtValue();
+ };
+
+ // OpenMP [6.0, Restrictions]
+ // first + count - 1 must not evaluate to a value greater than the
+ // loop sequence length of the associated canonical loop sequence.
+ auto ValidLoopRange = [](uint64_t FirstVal, uint64_t CountVal,
+ unsigned NumLoops) -> bool {
+return FirstVal + CountVal - 1 <= NumLoops;
+ };
+ uint64_t FirstVal = 1, CountVal = 0, LastVal = LoopSeqSize;
+
+ // Validates the loop range after evaluating the semantic information
+ // and ensures that the range is valid for the given loop sequence size.
+ // Expressions are evaluated at compile time to obtain constant values.
+ if (LRC) {
+EvaluateLoopRangeArguments(LRC->getFirst(), LRC->getCount(), FirstVal,
+ CountVal);
+if (CountVal == 1)
+ SemaRef.Diag(LRC->getCountLoc(), diag::warn_omp_redundant_fusion)
+ << getOpenMPDirectiveName(OMPD_fuse);
+
+if (!ValidLoopRange(FirstVal, CountVal, LoopSeqSize)) {
+ SemaRef.Diag(LRC->getFirstLoc(), diag::err_omp_invalid_looprange)
+ << getOpenMPDirectiveName(OMPD_fuse) << (FirstVal + CountVal - 1)
+ << LoopSeqSize;
+ return StmtError();
+}
+
+LastVal = FirstVal + CountVal - 1;
+ }
+
+ // Complete fusion generates a single canonical loop nest
+ // However looprange clause generates several loop nests
+ unsigned NumLoopNests = LRC ? LoopSeqSize - CountVal + 1 : 1;
+
+ // Emit a warning for redundant loop fusion when the sequence contains only
+ // one loop.
+ if (LoopSeqSize == 1)
+SemaRef.Diag(AStmt->getBeginLoc(), diag::warn_omp_redundant_fusion)
+<< getOpenMPDirectiveName(OMPD_fuse);
+
+ assert(LoopHelpers.size() == LoopSeqSize &&
+ "Expecting loop iteration space dimensionality to match number of "
+ "affected loops");
+ assert(OriginalInits.size() == LoopSeqSize &&
+ "Expecting loop iteration space dimensionality to match number of "
+ "affected loops");
+
+ // Select the type with the largest bit width among all induction variables
+ QualType IVType = LoopHel
[clang] [flang] [llvm] [openmp] [Clang][OpenMP][LoopTransformations] Add support for "#pragma omp fuse" loop transformation directive and "looprange" clause (PR #139293)
@@ -15499,6 +15836,496 @@ StmtResult
SemaOpenMP::ActOnOpenMPInterchangeDirective(
buildPreInits(Context, PreInits));
}
+StmtResult SemaOpenMP::ActOnOpenMPFuseDirective(ArrayRef Clauses,
+Stmt *AStmt,
+SourceLocation StartLoc,
+SourceLocation EndLoc) {
+
+ ASTContext &Context = getASTContext();
+ DeclContext *CurrContext = SemaRef.CurContext;
+ Scope *CurScope = SemaRef.getCurScope();
+ CaptureVars CopyTransformer(SemaRef);
+
+ // Ensure the structured block is not empty
+ if (!AStmt)
+return StmtError();
+
+ unsigned NumLoops = 1;
+ unsigned LoopSeqSize = 1;
+
+ // Defer transformation in dependent contexts
+ // The NumLoopNests argument is set to a placeholder 1 (even though
+ // using looprange fuse could yield up to 3 top level loop nests)
+ // because a dependent context could prevent determining its true value
+ if (CurrContext->isDependentContext()) {
+return OMPFuseDirective::Create(Context, StartLoc, EndLoc, Clauses,
+NumLoops, LoopSeqSize, AStmt, nullptr,
+nullptr);
+ }
+
+ // Validate that the potential loop sequence is transformable for fusion
+ // Also collect the HelperExprs, Loop Stmts, Inits, and Number of loops
+ SmallVector LoopHelpers;
+ SmallVector LoopStmts;
+ SmallVector> OriginalInits;
+ SmallVector> TransformsPreInits;
+ SmallVector> LoopSequencePreInits;
+ SmallVector LoopCategories;
+ if (!checkTransformableLoopSequence(OMPD_fuse, AStmt, LoopSeqSize, NumLoops,
+ LoopHelpers, LoopStmts, OriginalInits,
+ TransformsPreInits, LoopSequencePreInits,
+ LoopCategories, Context))
+return StmtError();
+
+ // Handle clauses, which can be any of the following: [looprange, apply]
+ const OMPLoopRangeClause *LRC =
+ OMPExecutableDirective::getSingleClause(Clauses);
+
+ // The clause arguments are invalidated if any error arises
+ // such as non-constant or non-positive arguments
+ if (LRC && (!LRC->getFirst() || !LRC->getCount()))
+return StmtError();
+
+ // Delayed semantic check of LoopRange constraint
+ // Evaluates the loop range arguments and returns the first and count values
+ auto EvaluateLoopRangeArguments = [&Context](Expr *First, Expr *Count,
+ uint64_t &FirstVal,
+ uint64_t &CountVal) {
+llvm::APSInt FirstInt = First->EvaluateKnownConstInt(Context);
+llvm::APSInt CountInt = Count->EvaluateKnownConstInt(Context);
+FirstVal = FirstInt.getZExtValue();
+CountVal = CountInt.getZExtValue();
+ };
+
+ // OpenMP [6.0, Restrictions]
+ // first + count - 1 must not evaluate to a value greater than the
+ // loop sequence length of the associated canonical loop sequence.
+ auto ValidLoopRange = [](uint64_t FirstVal, uint64_t CountVal,
+ unsigned NumLoops) -> bool {
+return FirstVal + CountVal - 1 <= NumLoops;
+ };
+ uint64_t FirstVal = 1, CountVal = 0, LastVal = LoopSeqSize;
+
+ // Validates the loop range after evaluating the semantic information
+ // and ensures that the range is valid for the given loop sequence size.
+ // Expressions are evaluated at compile time to obtain constant values.
+ if (LRC) {
+EvaluateLoopRangeArguments(LRC->getFirst(), LRC->getCount(), FirstVal,
+ CountVal);
+if (CountVal == 1)
+ SemaRef.Diag(LRC->getCountLoc(), diag::warn_omp_redundant_fusion)
+ << getOpenMPDirectiveName(OMPD_fuse);
+
+if (!ValidLoopRange(FirstVal, CountVal, LoopSeqSize)) {
+ SemaRef.Diag(LRC->getFirstLoc(), diag::err_omp_invalid_looprange)
+ << getOpenMPDirectiveName(OMPD_fuse) << (FirstVal + CountVal - 1)
+ << LoopSeqSize;
+ return StmtError();
+}
+
+LastVal = FirstVal + CountVal - 1;
+ }
+
+ // Complete fusion generates a single canonical loop nest
+ // However looprange clause generates several loop nests
+ unsigned NumLoopNests = LRC ? LoopSeqSize - CountVal + 1 : 1;
+
+ // Emit a warning for redundant loop fusion when the sequence contains only
+ // one loop.
+ if (LoopSeqSize == 1)
+SemaRef.Diag(AStmt->getBeginLoc(), diag::warn_omp_redundant_fusion)
+<< getOpenMPDirectiveName(OMPD_fuse);
+
+ assert(LoopHelpers.size() == LoopSeqSize &&
+ "Expecting loop iteration space dimensionality to match number of "
+ "affected loops");
+ assert(OriginalInits.size() == LoopSeqSize &&
+ "Expecting loop iteration space dimensionality to match number of "
+ "affected loops");
+
+ // Select the type with the largest bit width among all induction variables
+ QualType IVType = LoopHel
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: I originally kept the `NumGeneratedLoops` information consistent despite being partially subsumed by NumGeneratedLoopNests (Note that its not actually the same, it returns the number of generated loops in total adding nested loops, but due to the current usage of this semantic information I could argue they serve the same purpose) just in case this information was going to be used for future OpenMP Transformations or semantic logic maybe in OpenMP 7.0. If you both think this information about nested loops will never be used (Maybe if fusion gets a clause for multi-level fusion / fission could become relevant...), then I just remove it, but instead of storing a boolean value, a boolean function `hasGeneratedLoops` = `self->NumGeneratedLoops > 0` could be coded. After revising this topic thoroughly, I believe the most reasonable course of action would be to close this PR and keep `NumGeneratedLoops` as it is currently in this patch. Then, swap the semantic information of NumGeneratedLoops <-> `NumGeneratedLoopNests` and remove NumGeneratedLoopNests. This would also enable me to remove unnecessary computations that were performed in `AnalyzeLoopSequence` to keep `NumGeneratedLoops` consistent, simplifying logic and removing this second variable. @alexey-bataev , @Meinersbur what do you think? Will this information ever be needed? https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: > I originally kept the `NumGeneratedLoops` information consistent despite > being partially subsumed by NumGeneratedLoopNests (Note that its not actually > the same, it returns the number of generated loops in total adding nested > loops, but due to the current usage of this semantic information I could > argue they serve the same purpose) just in case this information was going to > be used for future OpenMP Transformations or semantic logic maybe in OpenMP > 7.0. If you both think this information about nested loops will never be used > (Maybe if fusion gets a clause for multi-level fusion / fission could become > relevant...), then I just remove it, but instead of storing a boolean value, > a boolean function `hasGeneratedLoops` = `self->NumGeneratedLoops > 0` could > be coded. > > After revising this topic thoroughly, I believe the most reasonable course of > action would be to close this PR and keep `NumGeneratedLoops` as it is > currently in this patch. Then, swap the semantic information of > NumGeneratedLoops <-> `NumGeneratedLoopNests` and remove > NumGeneratedLoopNests in patch #139293 . This would also enable me to remove > unnecessary computations that were performed in `AnalyzeLoopSequence` to keep > `NumGeneratedLoops` consistent, simplifying logic and removing this second > variable. > > @alexey-bataev , @Meinersbur what do you think? Will this information ever be > needed? Thank you! But before accepting the merge, does this mean that we discard everything that i said about refactoring these variables and just keep it as it is in both reviews? Also, could you @Meinersbur specify what exactly do you mean with a regression test for such thing? You mean like incorporating some kind of logic which includes NumGeneratedLoops in all LoopTransformation directives or just tile and reverse? https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
eZWALT wrote: gentle ping @alexey-bataev @Meinersbur ;) https://github.com/llvm/llvm-project/pull/140532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
