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
