Address basic PR feedback

Author: eZWALT

clang/include/clang/AST/OpenMPClause.h

@@ -1159,82 +1159,73 @@ class OMPFullClause final : public OMPNoChildClause<llvm::omp::OMPC_full> {
 ///   for(int j = 0; j < 256; j+=2)
 ///   for(int k = 127; k >= 0; --k)
 /// \endcode
-class OMPLoopRangeClause final : public OMPClause {
+class OMPLoopRangeClause final
+    : public OMPClause,
+      private llvm::TrailingObjects<OMPLoopRangeClause, Expr *> {
   friend class OMPClauseReader;
-
-  explicit OMPLoopRangeClause()
-      : OMPClause(llvm::omp::OMPC_looprange, {}, {}) {}
+  friend class llvm::TrailingObjects<OMPLoopRangeClause, Expr *>;
 
   /// 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; }
+  /// Location of first and count expressions
+  SourceLocation FirstLoc, CountLoc;
 
-  /// Set 'count'
-  void setCount(Expr *Count) { this->Count = Count; }
+  /// Number of looprange arguments (always 2: first, count)
+  unsigned NumArgs = 2;
 
-  /// Set location of '('.
-  void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; }
-
-  /// Set location of 'first' argument
-  void setFirstLoc(SourceLocation Loc) { FirstLoc = Loc; }
+  /// Set the argument expressions.
+  void setArgs(ArrayRef<Expr *> Args) {
+    assert(Args.size() == NumArgs && "Expected exactly 2 looprange arguments");
+    std::copy(Args.begin(), Args.end(), getTrailingObjects<Expr *>());
+  }
 
-  /// Set location of 'count' argument
-  void setCountLoc(SourceLocation Loc) { CountLoc = Loc; }
+  /// Build an empty clause for deserialization.
+  explicit OMPLoopRangeClause()
+      : OMPClause(llvm::omp::OMPC_looprange, {}, {}), NumArgs(2) {}
 
 public:
-  /// Build an AST node for a 'looprange' clause
-  ///
-  /// \param StartLoc     Starting location of the clause.
-  /// \param LParenLoc    Location of '('.
-  /// \param ModifierLoc  Modifier location.
-  /// \param
+  /// Build a 'looprange' clause AST node.
   static OMPLoopRangeClause *
   Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation LParenLoc,
          SourceLocation FirstLoc, SourceLocation CountLoc,
-         SourceLocation EndLoc, Expr *First, Expr *Count);
+         SourceLocation EndLoc, ArrayRef<Expr *> Args);
 
-  /// Build an empty 'looprange' node for deserialization
-  ///
-  /// \param C      Context of the AST.
+  /// Build an empty 'looprange' clause node.
   static OMPLoopRangeClause *CreateEmpty(const ASTContext &C);
 
-  /// Returns the location of '('
+  // Location getters/setters
   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<Expr>(First); }
+  void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; }
+  void setFirstLoc(SourceLocation Loc) { FirstLoc = Loc; }
+  void setCountLoc(SourceLocation Loc) { CountLoc = Loc; }
 
-  /// Returns the argument 'count' or nullptr if not set
-  Expr *getCount() const { return cast_or_null<Expr>(Count); }
+  /// Get looprange arguments: first and count
+  Expr *getFirst() const { return getArgs()[0]; }
+  Expr *getCount() const { return getArgs()[1]; }
 
-  child_range children() {
-    return child_range(reinterpret_cast<Stmt **>(&First),
-                       reinterpret_cast<Stmt **>(&Count) + 1);
+  /// Set looprange arguments: first and count
+  void setFirst(Expr *E) { getArgs()[0] = E; }
+  void setCount(Expr *E) { getArgs()[1] = E; }
+
+  MutableArrayRef<Expr *> getArgs() {
+    return MutableArrayRef<Expr *>(getTrailingObjects<Expr *>(), NumArgs);
+  }
+  ArrayRef<Expr *> getArgs() const {
+    return ArrayRef<Expr *>(getTrailingObjects<Expr *>(), NumArgs);
   }
 
+  child_range children() {
+    return child_range(reinterpret_cast<Stmt **>(getArgs().begin()),
+                       reinterpret_cast<Stmt **>(getArgs().end()));
+  }
   const_child_range children() const {
-    auto Children = const_cast<OMPLoopRangeClause *>(this)->children();
-    return const_child_range(Children.begin(), Children.end());
+    auto AR = getArgs();
+    return const_child_range(reinterpret_cast<Stmt *const *>(AR.begin()),
+                             reinterpret_cast<Stmt *const *>(AR.end()));
   }
 
   child_range used_children() {

clang/include/clang/AST/StmtOpenMP.h

@@ -5882,7 +5882,7 @@ class OMPInterchangeDirective final : public OMPLoopTransformationDirective {
                                        EndLoc, NumLoops) {
     // Interchange produces a single top-level canonical loop
     // nest, with the exact same amount of total loops
-    setNumGeneratedLoops(NumLoops);
+    setNumGeneratedLoops(3 * NumLoops);
     setNumGeneratedLoopNests(1);
   }
 

clang/include/clang/Sema/SemaOpenMP.h

@@ -1492,7 +1492,7 @@ class SemaOpenMP : public SemaBase {
   bool checkTransformableLoopNest(
       OpenMPDirectiveKind Kind, Stmt *AStmt, int NumLoops,
       SmallVectorImpl<OMPLoopBasedDirective::HelperExprs> &LoopHelpers,
-      Stmt *&Body, SmallVectorImpl<SmallVector<Stmt *, 0>> &OriginalInits);
+      Stmt *&Body, SmallVectorImpl<SmallVector<Stmt *>> &OriginalInits);
 
   /// @brief Categories of loops encountered during semantic OpenMP loop
   /// analysis
@@ -1555,9 +1555,9 @@ class SemaOpenMP : public SemaBase {
       Stmt *LoopSeqStmt, unsigned &LoopSeqSize, unsigned &NumLoops,
       SmallVectorImpl<OMPLoopBasedDirective::HelperExprs> &LoopHelpers,
       SmallVectorImpl<Stmt *> &ForStmts,
-      SmallVectorImpl<SmallVector<Stmt *, 0>> &OriginalInits,
-      SmallVectorImpl<SmallVector<Stmt *, 0>> &TransformsPreInits,
-      SmallVectorImpl<SmallVector<Stmt *, 0>> &LoopSequencePreInits,
+      SmallVectorImpl<SmallVector<Stmt *>> &OriginalInits,
+      SmallVectorImpl<SmallVector<Stmt *>> &TransformsPreInits,
+      SmallVectorImpl<SmallVector<Stmt *>> &LoopSequencePreInits,
       SmallVectorImpl<OMPLoopCategory> &LoopCategories, ASTContext &Context,
       OpenMPDirectiveKind Kind);
 
@@ -1591,9 +1591,9 @@ class SemaOpenMP : public SemaBase {
       unsigned &NumLoops,
       SmallVectorImpl<OMPLoopBasedDirective::HelperExprs> &LoopHelpers,
       SmallVectorImpl<Stmt *> &ForStmts,
-      SmallVectorImpl<SmallVector<Stmt *, 0>> &OriginalInits,
-      SmallVectorImpl<SmallVector<Stmt *, 0>> &TransformsPreInits,
-      SmallVectorImpl<SmallVector<Stmt *, 0>> &LoopSequencePreInits,
+      SmallVectorImpl<SmallVector<Stmt *>> &OriginalInits,
+      SmallVectorImpl<SmallVector<Stmt *>> &TransformsPreInits,
+      SmallVectorImpl<SmallVector<Stmt *>> &LoopSequencePreInits,
       SmallVectorImpl<OMPLoopCategory> &LoopCategories, ASTContext &Context);
 
   /// Helper to keep information about the current `omp begin/end declare

clang/lib/AST/OpenMPClause.cpp

@@ -1022,22 +1022,25 @@ OMPPartialClause *OMPPartialClause::CreateEmpty(const ASTContext &C) {
 
 OMPLoopRangeClause *
 OMPLoopRangeClause::Create(const ASTContext &C, SourceLocation StartLoc,
-                           SourceLocation LParenLoc, SourceLocation EndLoc,
-                           SourceLocation FirstLoc, SourceLocation CountLoc,
-                           Expr *First, Expr *Count) {
+                           SourceLocation LParenLoc, SourceLocation FirstLoc,
+                           SourceLocation CountLoc, SourceLocation EndLoc,
+                           ArrayRef<Expr *> Args) {
+
+  assert(Args.size() == 2 &&
+         "looprange clause must have exactly two arguments");
   OMPLoopRangeClause *Clause = CreateEmpty(C);
   Clause->setLocStart(StartLoc);
   Clause->setLParenLoc(LParenLoc);
-  Clause->setLocEnd(EndLoc);
   Clause->setFirstLoc(FirstLoc);
   Clause->setCountLoc(CountLoc);
-  Clause->setFirst(First);
-  Clause->setCount(Count);
+  Clause->setLocEnd(EndLoc);
+  Clause->setArgs(Args);
   return Clause;
 }
 
 OMPLoopRangeClause *OMPLoopRangeClause::CreateEmpty(const ASTContext &C) {
-  return new (C) OMPLoopRangeClause();
+  void *Mem = C.Allocate(totalSizeToAlloc<Expr *>(2));
+  return new (Mem) OMPLoopRangeClause();
 }
 
 OMPAllocateClause *OMPAllocateClause::Create(

clang/lib/CodeGen/CGExpr.cpp

@@ -3256,11 +3256,8 @@ LValue CodeGenFunction::EmitDeclRefLValue(const DeclRefExpr *E) {
           var, ConvertTypeForMem(VD->getType()), getContext().getDeclAlign(VD));
 
     // No other cases for now.
-    } else {
-      llvm::dbgs() << "THE DAMN DECLREFEXPR HASN'T BEEN ENTERED IN LOCALDECLMAP\n";
-      VD->dumpColor();
+    } else
       llvm_unreachable("DeclRefExpr for Decl not entered in LocalDeclMap?");
-    }
 
     // Handle threadlocal function locals.
     if (VD->getTLSKind() != VarDecl::TLS_None)

clang/lib/CodeGen/CodeGenFunction.h

@@ -5372,10 +5372,6 @@ 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(); 
-    }
     assert(!LocalDeclMap.count(VD) && "Decl already exists in LocalDeclMap!");
     LocalDeclMap.insert({VD, Addr});
   }