Merged main:d5cad0550626 into amd-gfx:1b9a5a807978

Local branch amd-gfx 1b9a5a8 Merged main:247cc265e74e into amd-gfx:8b39e87bc826
Remote branch main d5cad05 [lldb] Remve old CODE_OWNERS.txt

Author: SC llvm team

clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp

@@ -55,7 +55,9 @@ IncludeCleanerCheck::IncludeCleanerCheck(StringRef Name,
                                          ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       IgnoreHeaders(utils::options::parseStringList(
-          Options.getLocalOrGlobal("IgnoreHeaders", ""))) {
+          Options.getLocalOrGlobal("IgnoreHeaders", ""))),
+      DeduplicateFindings(
+          Options.getLocalOrGlobal("DeduplicateFindings", true)) {
   for (const auto &Header : IgnoreHeaders) {
     if (!llvm::Regex{Header}.isValid())
       configurationDiag("Invalid ignore headers regex '%0'") << Header;
@@ -69,6 +71,7 @@ IncludeCleanerCheck::IncludeCleanerCheck(StringRef Name,
 void IncludeCleanerCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IgnoreHeaders",
                 utils::options::serializeStringList(IgnoreHeaders));
+  Options.store(Opts, "DeduplicateFindings", DeduplicateFindings);
 }
 
 bool IncludeCleanerCheck::isLanguageVersionSupported(
@@ -114,12 +117,26 @@ void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
     // FIXME: Filter out implicit template specializations.
     MainFileDecls.push_back(D);
   }
+  llvm::DenseSet<include_cleaner::Symbol> SeenSymbols;
   // FIXME: Find a way to have less code duplication between include-cleaner
   // analysis implementation and the below code.
   walkUsed(MainFileDecls, RecordedPreprocessor.MacroReferences, &RecordedPI,
            *SM,
            [&](const include_cleaner::SymbolReference &Ref,
                llvm::ArrayRef<include_cleaner::Header> Providers) {
+             // Process each symbol once to reduce noise in the findings.
+             // Tidy checks are used in two different workflows:
+             // - Ones that show all the findings for a given file. For such
+             // workflows there is not much point in showing all the occurences,
+             // as one is enough to indicate the issue.
+             // - Ones that show only the findings on changed pieces. For such
+             // workflows it's useful to show findings on every reference of a
+             // symbol as otherwise tools might give incosistent results
+             // depending on the parts of the file being edited. But it should
+             // still help surface findings for "new violations" (i.e.
+             // dependency did not exist in the code at all before).
+             if (DeduplicateFindings && !SeenSymbols.insert(Ref.Target).second)
+               return;
              bool Satisfied = false;
              for (const include_cleaner::Header &H : Providers) {
                if (H.kind() == include_cleaner::Header::Physical &&

clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.h

@@ -44,6 +44,8 @@ class IncludeCleanerCheck : public ClangTidyCheck {
   include_cleaner::PragmaIncludes RecordedPI;
   HeaderSearch *HS;
   std::vector<StringRef> IgnoreHeaders;
+  // Whether emit only one finding per usage of a symbol.
+  const bool DeduplicateFindings;
   llvm::SmallVector<llvm::Regex> IgnoreHeadersRegex;
   bool shouldIgnore(const include_cleaner::Header &H);
 };

clang-tools-extra/docs/ReleaseNotes.rst

@@ -180,6 +180,10 @@ Changes in existing checks
   <clang-tidy/checks/llvm/namespace-comment>` check to provide fixes for
   ``inline`` namespaces in the same format as :program:`clang-format`.
 
+- Improved :doc:`misc-include-cleaner
+  <clang-tidy/checks/misc/include-cleaner>` check by adding option
+  `DeduplicateFindings` to output one finding per symbol occurence.
+
 - Improved :doc:`modernize-loop-convert
   <clang-tidy/checks/modernize/loop-convert>` to support for-loops with
   iterators initialized by free functions like ``begin``, ``end``, or ``size``.

clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst

@@ -33,3 +33,8 @@ Options
    files that match this regex as a suffix.  E.g., `foo/.*` disables
    insertion/removal for all headers under the directory `foo`. By default, no 
    headers will be ignored.
+
+.. option:: DeduplicateFindings
+
+   A boolean that controls whether the check should deduplicate findings for the
+   same symbol. Defaults to `true`.

clang-tools-extra/unittests/clang-tidy/IncludeCleanerTest.cpp

@@ -15,6 +15,8 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
+#include "llvm/Testing/Annotations/Annotations.h"
+#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <initializer_list>
 
@@ -108,6 +110,50 @@ int BazResult = baz();
                            )"}}));
 }
 
+TEST(IncludeCleanerCheckTest, DedupsMissingIncludes) {
+  llvm::Annotations Code(R"(
+#include "baz.h" // IWYU pragma: keep
+
+int BarResult1 = $diag1^bar();
+int BarResult2 = $diag2^bar();)");
+
+  {
+    std::vector<ClangTidyError> Errors;
+    runCheckOnCode<IncludeCleanerCheck>(Code.code(), &Errors, "file.cpp",
+                                        std::nullopt, ClangTidyOptions(),
+                                        {{"baz.h", R"(#pragma once
+                              #include "bar.h"
+                           )"},
+                                         {"bar.h", R"(#pragma once
+                              int bar();
+                           )"}});
+    ASSERT_THAT(Errors.size(), testing::Eq(1U));
+    EXPECT_EQ(Errors.front().Message.Message,
+              "no header providing \"bar\" is directly included");
+    EXPECT_EQ(Errors.front().Message.FileOffset, Code.point("diag1"));
+  }
+  {
+    std::vector<ClangTidyError> Errors;
+    ClangTidyOptions Opts;
+    Opts.CheckOptions.insert({"DeduplicateFindings", "false"});
+    runCheckOnCode<IncludeCleanerCheck>(Code.code(), &Errors, "file.cpp",
+                                        std::nullopt, Opts,
+                                        {{"baz.h", R"(#pragma once
+                              #include "bar.h"
+                           )"},
+                                         {"bar.h", R"(#pragma once
+                              int bar();
+                           )"}});
+    ASSERT_THAT(Errors.size(), testing::Eq(2U));
+    EXPECT_EQ(Errors.front().Message.Message,
+              "no header providing \"bar\" is directly included");
+    EXPECT_EQ(Errors.front().Message.FileOffset, Code.point("diag1"));
+    EXPECT_EQ(Errors.back().Message.Message,
+              "no header providing \"bar\" is directly included");
+    EXPECT_EQ(Errors.back().Message.FileOffset, Code.point("diag2"));
+  }
+}
+
 TEST(IncludeCleanerCheckTest, SuppressMissingIncludes) {
   const char *PreCode = R"(
 #include "bar.h"

clang/include/clang/AST/StmtOpenMP.h

@@ -281,6 +281,15 @@ class OMPExecutableDirective : public Stmt {
     return Data->getClauses();
   }
 
+  /// Was this directive mapped from an another directive?
+  /// e.g. 1) omp loop bind(parallel) is mapped to OMPD_for
+  ///      2) omp loop bind(teams) is mapped to OMPD_distribute
+  ///      3) omp loop bind(thread) is mapped to OMPD_simd
+  /// It was necessary to note it down in the Directive because of
+  /// clang::TreeTransform::TransformOMPExecutableDirective() pass in
+  /// the frontend.
+  OpenMPDirectiveKind PrevMappedDirective = llvm::omp::OMPD_unknown;
+
 protected:
   /// Data, associated with the directive.
   OMPChildren *Data = nullptr;
@@ -345,6 +354,10 @@ class OMPExecutableDirective : public Stmt {
     return Inst;
   }
 
+  void setMappedDirective(OpenMPDirectiveKind MappedDirective) {
+    PrevMappedDirective = MappedDirective;
+  }
+
 public:
   /// Iterates over expressions/statements used in the construct.
   class used_clauses_child_iterator
@@ -598,6 +611,8 @@ class OMPExecutableDirective : public Stmt {
            "Expected directive with the associated statement.");
     return Data->getRawStmt();
   }
+
+  OpenMPDirectiveKind getMappedDirective() const { return PrevMappedDirective; }
 };
 
 /// This represents '#pragma omp parallel' directive.
@@ -1604,7 +1619,8 @@ class OMPSimdDirective : public OMPLoopDirective {
                                   SourceLocation EndLoc, unsigned CollapsedNum,
                                   ArrayRef<OMPClause *> Clauses,
                                   Stmt *AssociatedStmt,
-                                  const HelperExprs &Exprs);
+                                  const HelperExprs &Exprs,
+                                  OpenMPDirectiveKind ParamPrevMappedDirective);
 
   /// Creates an empty directive with the place
   /// for \a NumClauses clauses.
@@ -1682,7 +1698,8 @@ class OMPForDirective : public OMPLoopDirective {
                                  SourceLocation EndLoc, unsigned CollapsedNum,
                                  ArrayRef<OMPClause *> Clauses,
                                  Stmt *AssociatedStmt, const HelperExprs &Exprs,
-                                 Expr *TaskRedRef, bool HasCancel);
+                                 Expr *TaskRedRef, bool HasCancel,
+                                 OpenMPDirectiveKind ParamPrevMappedDirective);
 
   /// Creates an empty directive with the place
   /// for \a NumClauses clauses.
@@ -4406,7 +4423,8 @@ class OMPDistributeDirective : public OMPLoopDirective {
   static OMPDistributeDirective *
   Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc,
          unsigned CollapsedNum, ArrayRef<OMPClause *> Clauses,
-         Stmt *AssociatedStmt, const HelperExprs &Exprs);
+         Stmt *AssociatedStmt, const HelperExprs &Exprs,
+         OpenMPDirectiveKind ParamPrevMappedDirective);
 
   /// Creates an empty directive with the place
   /// for \a NumClauses clauses.

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -9875,6 +9875,11 @@ def err_break_not_in_loop_or_switch : Error<
 def warn_loop_ctrl_binds_to_inner : Warning<
   "'%0' is bound to current loop, GCC binds it to the enclosing loop">,
   InGroup<GccCompat>;
+def err_omp_bind_required_on_loop : Error<
+  "expected 'bind' clause for 'loop' construct without an enclosing OpenMP "
+  "construct">;
+def err_omp_loop_reduction_clause : Error<
+  "'reduction' clause not allowed with '#pragma omp loop bind(teams)'">;
 def warn_break_binds_to_switch : Warning<
   "'break' is bound to loop, GCC binds it to switch">,
   InGroup<GccCompat>;

clang/include/clang/Sema/Sema.h

@@ -11185,6 +11185,23 @@ class Sema final {
   /// All `omp assumes` we encountered so far.
   SmallVector<AssumptionAttr *, 4> OMPAssumeGlobal;
 
+  /// OMPD_loop is mapped to OMPD_for, OMPD_distribute or OMPD_simd depending
+  /// on the parameter of the bind clause. In the methods for the
+  /// mapped directives, check the parameters of the lastprivate clause.
+  bool checkLastPrivateForMappedDirectives(ArrayRef<OMPClause *> Clauses);
+  /// Depending on the bind clause of OMPD_loop map the directive to new
+  /// directives.
+  ///    1) loop bind(parallel) --> OMPD_for
+  ///    2) loop bind(teams) --> OMPD_distribute
+  ///    3) loop bind(thread) --> OMPD_simd
+  /// This is being handled in Sema instead of Codegen because of the need for
+  /// rigorous semantic checking in the new mapped directives.
+  bool mapLoopConstruct(llvm::SmallVector<OMPClause *> &ClausesWithoutBind,
+                        ArrayRef<OMPClause *> Clauses,
+                        OpenMPBindClauseKind BindKind,
+                        OpenMPDirectiveKind &Kind,
+                        OpenMPDirectiveKind &PrevMappedDirective);
+
 public:
   /// The declarator \p D defines a function in the scope \p S which is nested
   /// in an `omp begin/end declare variant` scope. In this method we create a
@@ -11480,7 +11497,8 @@ class Sema final {
   StmtResult ActOnOpenMPExecutableDirective(
       OpenMPDirectiveKind Kind, const DeclarationNameInfo &DirName,
       OpenMPDirectiveKind CancelRegion, ArrayRef<OMPClause *> Clauses,
-      Stmt *AStmt, SourceLocation StartLoc, SourceLocation EndLoc);
+      Stmt *AStmt, SourceLocation StartLoc, SourceLocation EndLoc,
+      OpenMPDirectiveKind PrevMappedDirective = llvm::omp::OMPD_unknown);
   /// Called on well-formed '\#pragma omp parallel' after parsing
   /// of the  associated statement.
   StmtResult ActOnOpenMPParallelDirective(ArrayRef<OMPClause *> Clauses,

clang/lib/AST/StmtOpenMP.cpp

@@ -297,11 +297,10 @@ OMPParallelDirective *OMPParallelDirective::CreateEmpty(const ASTContext &C,
                                                     /*NumChildren=*/1);
 }
 
-OMPSimdDirective *
-OMPSimdDirective::Create(const ASTContext &C, SourceLocation StartLoc,
-                         SourceLocation EndLoc, unsigned CollapsedNum,
-                         ArrayRef<OMPClause *> Clauses, Stmt *AssociatedStmt,
-                         const HelperExprs &Exprs) {
+OMPSimdDirective *OMPSimdDirective::Create(
+    const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc,
+    unsigned CollapsedNum, ArrayRef<OMPClause *> Clauses, Stmt *AssociatedStmt,
+    const HelperExprs &Exprs, OpenMPDirectiveKind ParamPrevMappedDirective) {
   auto *Dir = createDirective<OMPSimdDirective>(
       C, Clauses, AssociatedStmt, numLoopChildren(CollapsedNum, OMPD_simd),
       StartLoc, EndLoc, CollapsedNum);
@@ -321,6 +320,7 @@ OMPSimdDirective::Create(const ASTContext &C, SourceLocation StartLoc,
   Dir->setDependentInits(Exprs.DependentInits);
   Dir->setFinalsConditions(Exprs.FinalsConditions);
   Dir->setPreInits(Exprs.PreInits);
+  Dir->setMappedDirective(ParamPrevMappedDirective);
   return Dir;
 }
 
@@ -336,7 +336,8 @@ OMPSimdDirective *OMPSimdDirective::CreateEmpty(const ASTContext &C,
 OMPForDirective *OMPForDirective::Create(
     const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc,
     unsigned CollapsedNum, ArrayRef<OMPClause *> Clauses, Stmt *AssociatedStmt,
-    const HelperExprs &Exprs, Expr *TaskRedRef, bool HasCancel) {
+    const HelperExprs &Exprs, Expr *TaskRedRef, bool HasCancel,
+    OpenMPDirectiveKind ParamPrevMappedDirective) {
   auto *Dir = createDirective<OMPForDirective>(
       C, Clauses, AssociatedStmt, numLoopChildren(CollapsedNum, OMPD_for) + 1,
       StartLoc, EndLoc, CollapsedNum);
@@ -366,6 +367,7 @@ OMPForDirective *OMPForDirective::Create(
   Dir->setPreInits(Exprs.PreInits);
   Dir->setTaskReductionRefExpr(TaskRedRef);
   Dir->setHasCancel(HasCancel);
+  Dir->setMappedDirective(ParamPrevMappedDirective);
   return Dir;
 }
 
@@ -1515,7 +1517,7 @@ OMPParallelMaskedTaskLoopSimdDirective::CreateEmpty(const ASTContext &C,
 OMPDistributeDirective *OMPDistributeDirective::Create(
     const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc,
     unsigned CollapsedNum, ArrayRef<OMPClause *> Clauses, Stmt *AssociatedStmt,
-    const HelperExprs &Exprs) {
+    const HelperExprs &Exprs, OpenMPDirectiveKind ParamPrevMappedDirective) {
   auto *Dir = createDirective<OMPDistributeDirective>(
       C, Clauses, AssociatedStmt,
       numLoopChildren(CollapsedNum, OMPD_distribute), StartLoc, EndLoc,
@@ -1544,6 +1546,7 @@ OMPDistributeDirective *OMPDistributeDirective::Create(
   Dir->setDependentInits(Exprs.DependentInits);
   Dir->setFinalsConditions(Exprs.FinalsConditions);
   Dir->setPreInits(Exprs.PreInits);
+  Dir->setMappedDirective(ParamPrevMappedDirective);
   return Dir;
 }