[clang] [Clang][OpenMP][LoopTransformations] Fix incorrect number of generated loops for Tile and Reverse directives (PR #140532)
Michael Kruse via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 16 07:05:27 PDT 2025
Meinersbur wrote:
> 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.
Whatever the refactoring will be, this I think this patch is useful by itself. Even if the refactoring will delete all this code, the commit can be cherry-picked for those who do not want the refactoring. I don't think we should work forever on obvious fixes in case we can find something nicer.
With the reproducer I meant the `setNumGeneratedLoops(1);` for the reverse directive. It might make a difference in `OMPLoopBasedDirective::doForAllLoop()` which might think there are no further loops to iterate into (`if (NumGeneratedLoops == 0) {`) when in fact there is. This requires multiple nesting of loop transformations which might be difficult to find a reproducer for. Since I think this fix is rather obvious, I would prioritize fixing it over spending a lot of time finding a regression test.
https://github.com/llvm/llvm-project/pull/140532
More information about the cfe-commits
mailing list