Address reviewers comments

Author: rofirrim

clang/include/clang/Sema/SemaOpenMP.h

@@ -1503,12 +1503,12 @@ class SemaOpenMP : public SemaBase {
   /// Holds the result of the analysis of a (possibly canonical) loop.
   struct LoopAnalysis {
     /// The analyzed loop or loop transformation.
-    Stmt *AStmt;
+    Stmt *AStmt = nullptr;
     /// Loop analyses results.
     OMPLoopBasedDirective::HelperExprs HelperExprs;
-    /// The for-statement of the loop. ForStmt equals AStmt only when the latter
-    /// is a canonical loop (i.e. not a loop transformation).
-    Stmt *ForStmt;
+    /// The for-statement of the loop. TheForStmt equals AStmt only when the
+    /// latter is a canonical loop (i.e. not a loop transformation).
+    Stmt *TheForStmt = nullptr;
     /// Initialization statements before transformations.
     SmallVector<Stmt *> OriginalInits;
     /// Initialization statements required after transformation of this loop.
@@ -1521,7 +1521,7 @@ class SemaOpenMP : public SemaBase {
 
     // Convenience functions used when building LoopSequenceAnalysis.
     static bool isRegularLoop(Stmt *S) {
-      return isa<clang::ForStmt, CXXForRangeStmt>(S);
+      return isa<ForStmt, CXXForRangeStmt>(S);
     }
     static bool isLoopTransformation(Stmt *S) {
       return isa<OMPLoopTransformationDirective>(S);

clang/lib/AST/StmtOpenMP.cpp

@@ -371,22 +371,18 @@ OMPForDirective *OMPForDirective::Create(
 }
 
 Stmt *OMPLoopTransformationDirective::getTransformedStmt() const {
-  if (auto *D = dyn_cast<OMPCanonicalLoopNestTransformationDirective>(S)) {
+  if (auto *D = dyn_cast<OMPCanonicalLoopNestTransformationDirective>(S))
     return D->getTransformedStmt();
-  }
-  if (auto *D = dyn_cast<OMPCanonicalLoopSequenceTransformationDirective>(S)) {
+  if (auto *D = dyn_cast<OMPCanonicalLoopSequenceTransformationDirective>(S))
     return D->getTransformedStmt();
-  }
   llvm_unreachable("unexpected object type");
 }
 
 Stmt *OMPLoopTransformationDirective::getPreInits() const {
-  if (auto *D = dyn_cast<OMPCanonicalLoopNestTransformationDirective>(S)) {
+  if (auto *D = dyn_cast<OMPCanonicalLoopNestTransformationDirective>(S))
     return D->getPreInits();
-  }
-  if (auto *D = dyn_cast<OMPCanonicalLoopSequenceTransformationDirective>(S)) {
+  if (auto *D = dyn_cast<OMPCanonicalLoopSequenceTransformationDirective>(S))
     return D->getPreInits();
-  }
   llvm_unreachable("unexpected object type");
 }
 

clang/lib/Sema/SemaOpenMP.cpp

@@ -14564,11 +14564,11 @@ bool SemaOpenMP::analyzeLoopSequence(Stmt *LoopSeqStmt,
   auto StoreLoopStatements = [](LoopAnalysis &Analysis, Stmt *LoopStmt) {
     if (auto *For = dyn_cast<ForStmt>(LoopStmt)) {
       Analysis.OriginalInits.push_back(For->getInit());
-      Analysis.ForStmt = For;
+      Analysis.TheForStmt = For;
     } else {
       auto *CXXFor = cast<CXXForRangeStmt>(LoopStmt);
       Analysis.OriginalInits.push_back(CXXFor->getBeginStmt());
-      Analysis.ForStmt = CXXFor;
+      Analysis.TheForStmt = CXXFor;
     }
   };
 
@@ -16019,7 +16019,9 @@ StmtResult SemaOpenMP::ActOnOpenMPFuseDirective(ArrayRef<OMPClause *> Clauses,
 
   // SeqAnalysis.LoopSeqSize exists mostly to handle dependent contexts,
   // otherwise it must be the same as SeqAnalysis.Loops.size().
-  assert(SeqAnalysis.LoopSeqSize == SeqAnalysis.Loops.size());
+  assert(SeqAnalysis.LoopSeqSize == SeqAnalysis.Loops.size() &&
+         "Inconsistent size of the loop sequence and the number of loops "
+         "found in the sequence");
 
   // Handle clauses, which can be any of the following: [looprange, apply]
   const OMPLoopRangeClause *LRC =
@@ -16084,7 +16086,7 @@ StmtResult SemaOpenMP::ActOnOpenMPFuseDirective(ArrayRef<OMPClause *> Clauses,
   // Select the type with the largest bit width among all induction variables
   QualType IVType =
       SeqAnalysis.Loops[FirstVal - 1].HelperExprs.IterationVarRef->getType();
-  for (unsigned I = FirstVal; I < LastVal; ++I) {
+  for (unsigned I : llvm::seq<unsigned>(FirstVal, LastVal)) {
     QualType CurrentIVType =
         SeqAnalysis.Loops[I].HelperExprs.IterationVarRef->getType();
     if (Context.getTypeSize(CurrentIVType) > Context.getTypeSize(IVType)) {
@@ -16146,9 +16148,7 @@ StmtResult SemaOpenMP::ActOnOpenMPFuseDirective(ArrayRef<OMPClause *> Clauses,
   //  original loop. The preinit structure must ensure that hidden variables
   //  like '.omp.fuse.max' are still properly handled.
   // Transformations that apply this concept: Loopranged Fuse, Split
-  if (!SeqAnalysis.LoopSequencePreInits.empty()) {
-    llvm::append_range(PreInits, SeqAnalysis.LoopSequencePreInits);
-  }
+  llvm::append_range(PreInits, SeqAnalysis.LoopSequencePreInits);
 
   // Process each single loop to generate and collect declarations
   // and statements for all helper expressions related to
@@ -16170,19 +16170,18 @@ StmtResult SemaOpenMP::ActOnOpenMPFuseDirective(ArrayRef<OMPClause *> Clauses,
   for (unsigned int I = FirstVal - 1, J = 0; I < LastVal; ++I, ++J) {
     if (SeqAnalysis.Loops[I].isRegularLoop()) {
       addLoopPreInits(Context, SeqAnalysis.Loops[I].HelperExprs,
-                      SeqAnalysis.Loops[I].ForStmt,
+                      SeqAnalysis.Loops[I].TheForStmt,
                       SeqAnalysis.Loops[I].OriginalInits, PreInits);
     } else if (SeqAnalysis.Loops[I].isLoopTransformation()) {
       // For transformed loops, insert both pre-inits and original inits.
       // Order matters: pre-inits may define variables used in the original
       // inits such as upper bounds...
       SmallVector<Stmt *> &TransformPreInit =
           SeqAnalysis.Loops[TransformIndex++].TransformsPreInits;
-      if (!TransformPreInit.empty())
-        llvm::append_range(PreInits, TransformPreInit);
+      llvm::append_range(PreInits, TransformPreInit);
 
       addLoopPreInits(Context, SeqAnalysis.Loops[I].HelperExprs,
-                      SeqAnalysis.Loops[I].ForStmt,
+                      SeqAnalysis.Loops[I].TheForStmt,
                       SeqAnalysis.Loops[I].OriginalInits, PreInits);
     }
     auto [UBVD, UBDStmt] =
@@ -16270,7 +16269,7 @@ StmtResult SemaOpenMP::ActOnOpenMPFuseDirective(ArrayRef<OMPClause *> Clauses,
   //   omp.fuse.max = max(omp.temp1, omp.temp0)
 
   ExprResult MaxExpr;
-  // I is the true
+  // I is the range of loops in the sequence that we fuse.
   for (unsigned I = FirstVal - 1, J = 0; I < LastVal; ++I, ++J) {
     DeclRefExpr *NIRef = MakeVarDeclRef(NIVarDecls[J]);
     QualType NITy = NIRef->getType();
@@ -16382,13 +16381,13 @@ StmtResult SemaOpenMP::ActOnOpenMPFuseDirective(ArrayRef<OMPClause *> Clauses,
 
     // If the loop is a CXXForRangeStmt then the iterator variable is needed
     if (auto *SourceCXXFor =
-            dyn_cast<CXXForRangeStmt>(SeqAnalysis.Loops[I].ForStmt))
+            dyn_cast<CXXForRangeStmt>(SeqAnalysis.Loops[I].TheForStmt))
       BodyStmts.push_back(SourceCXXFor->getLoopVarStmt());
 
     Stmt *Body =
-        (isa<ForStmt>(SeqAnalysis.Loops[I].ForStmt))
-            ? cast<ForStmt>(SeqAnalysis.Loops[I].ForStmt)->getBody()
-            : cast<CXXForRangeStmt>(SeqAnalysis.Loops[I].ForStmt)->getBody();
+        (isa<ForStmt>(SeqAnalysis.Loops[I].TheForStmt))
+            ? cast<ForStmt>(SeqAnalysis.Loops[I].TheForStmt)->getBody()
+            : cast<CXXForRangeStmt>(SeqAnalysis.Loops[I].TheForStmt)->getBody();
     BodyStmts.push_back(Body);
 
     CompoundStmt *CombinedBody =
@@ -16457,7 +16456,7 @@ StmtResult SemaOpenMP::ActOnOpenMPFuseDirective(ArrayRef<OMPClause *> Clauses,
           llvm::append_range(PreInits, TransformPreInit);
       }
 
-      FinalLoops.push_back(SeqAnalysis.Loops[I].ForStmt);
+      FinalLoops.push_back(SeqAnalysis.Loops[I].TheForStmt);
     }
 
     FinalLoops.insert(FinalLoops.begin() + (FirstVal - 1), FusedForStmt);