[Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives

[eZWALT]

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.

@alexey-bataev , @Meinersbur

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Walter J.T.V (eZWALT)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/140532.diff

1 Files Affected:

• (modified) clang/include/clang/AST/StmtOpenMP.h (+4-2)

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) {

[alexey-bataev]

Do we need the number of generated loops at all? Is it used anywhere? Maybe it worth it to remove it?

[eZWALT]

@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.