Merged master:8427885e2781 into amd-gfx:300befc3bdb2

Local branch amd-gfx 300befc [AMDGPU] Avoid calling expandUnalignedLoad/Store and spilling when possible
Remote branch master 8427885 Temporairly revert "Thread safety analysis: Consider global variables in scope" & followup

clang-tools-extra/clang-tidy/CMakeLists.txt

@@ -46,6 +46,7 @@ endif()
 # If you add a check, also add it to ClangTidyForceLinker.h in this directory.
 add_subdirectory(android)
 add_subdirectory(abseil)
+add_subdirectory(altera)
 add_subdirectory(boost)
 add_subdirectory(bugprone)
 add_subdirectory(cert)
@@ -71,6 +72,7 @@ add_subdirectory(zircon)
 set(ALL_CLANG_TIDY_CHECKS
   clangTidyAndroidModule
   clangTidyAbseilModule
+  clangTidyAlteraModule
   clangTidyBoostModule
   clangTidyBugproneModule
   clangTidyCERTModule

clang-tools-extra/clang-tidy/ClangTidyForceLinker.h

@@ -20,6 +20,11 @@ extern volatile int AbseilModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED AbseilModuleAnchorDestination =
     AbseilModuleAnchorSource;
 
+// This anchor is used to force the linker to link the AlteraModule.
+extern volatile int AlteraModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED AlteraModuleAnchorDestination =
+    AlteraModuleAnchorSource;
+
 // This anchor is used to force the linker to link the AndroidModule.
 extern volatile int AndroidModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED AndroidModuleAnchorDestination =

clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp

@@ -0,0 +1,39 @@
+//===--- AlteraTidyModule.cpp - clang-tidy --------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "StructPackAlignCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace altera {
+
+class AlteraModule : public ClangTidyModule {
+public:
+  void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<StructPackAlignCheck>(
+        "altera-struct-pack-align");
+  }
+};
+
+} // namespace altera
+
+// Register the AlteraTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<altera::AlteraModule>
+    X("altera-module", "Adds Altera FPGA OpenCL lint checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the AlteraModule.
+volatile int AlteraModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/altera/CMakeLists.txt

@@ -0,0 +1,22 @@
+set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
+  support
+  )
+
+add_clang_library(clangTidyAlteraModule
+  AlteraTidyModule.cpp
+  StructPackAlignCheck.cpp
+
+  LINK_LIBS
+  clangTidy
+  clangTidyUtils
+  )
+
+clang_target_link_libraries(clangTidyAlteraModule
+  PRIVATE
+  clangAnalysis
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  )

clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.cpp

@@ -0,0 +1,144 @@
+//===--- StructPackAlignCheck.cpp - clang-tidy ----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "StructPackAlignCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecordLayout.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <math.h>
+#include <sstream>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace altera {
+
+void StructPackAlignCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(recordDecl(isStruct(), isDefinition(),
+                                unless(isExpansionInSystemHeader()))
+                         .bind("struct"),
+                     this);
+}
+
+CharUnits
+StructPackAlignCheck::computeRecommendedAlignment(CharUnits MinByteSize) {
+  CharUnits NewAlign = CharUnits::fromQuantity(1);
+  if (!MinByteSize.isPowerOfTwo()) {
+    int MSB = (int)MinByteSize.getQuantity();
+    for (; MSB > 0; MSB /= 2) {
+      NewAlign = NewAlign.alignTo(
+          CharUnits::fromQuantity(((int)NewAlign.getQuantity()) * 2));
+      // Abort if the computed alignment meets the maximum configured alignment.
+      if (NewAlign.getQuantity() >= MaxConfiguredAlignment)
+        break;
+    }
+  } else {
+    NewAlign = MinByteSize;
+  }
+  return NewAlign;
+}
+
+void StructPackAlignCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Struct = Result.Nodes.getNodeAs<RecordDecl>("struct");
+
+  // Do not trigger on templated struct declarations because the packing and
+  // alignment requirements are unknown.
+  if (Struct->isTemplated())
+     return;
+
+  // Get sizing info for the struct.
+  llvm::SmallVector<std::pair<unsigned int, unsigned int>, 10> FieldSizes;
+  unsigned int TotalBitSize = 0;
+  for (const FieldDecl *StructField : Struct->fields()) {
+    // For each StructField, record how big it is (in bits).
+    // Would be good to use a pair of <offset, size> to advise a better
+    // packing order.
+    unsigned int StructFieldWidth =
+        (unsigned int)Result.Context
+            ->getTypeInfo(StructField->getType().getTypePtr())
+            .Width;
+    FieldSizes.emplace_back(StructFieldWidth, StructField->getFieldIndex());
+    // FIXME: Recommend a reorganization of the struct (sort by StructField
+    // size, largest to smallest).
+    TotalBitSize += StructFieldWidth;
+  }
+
+  uint64_t CharSize = Result.Context->getCharWidth();
+  CharUnits CurrSize = Result.Context->getASTRecordLayout(Struct).getSize();
+  CharUnits MinByteSize =
+      CharUnits::fromQuantity(ceil((float)TotalBitSize / CharSize));
+  CharUnits MaxAlign = CharUnits::fromQuantity(
+      ceil((float)Struct->getMaxAlignment() / CharSize));
+  CharUnits CurrAlign =
+      Result.Context->getASTRecordLayout(Struct).getAlignment();
+  CharUnits NewAlign = computeRecommendedAlignment(MinByteSize);
+
+  bool IsPacked = Struct->hasAttr<PackedAttr>();
+  bool NeedsPacking = (MinByteSize < CurrSize) && (MaxAlign != NewAlign) &&
+                      (CurrSize != NewAlign);
+  bool NeedsAlignment = CurrAlign.getQuantity() != NewAlign.getQuantity();
+
+  if (!NeedsAlignment && !NeedsPacking)
+    return;
+
+  // If it's using much more space than it needs, suggest packing.
+  // (Do not suggest packing if it is currently explicitly aligned to what the
+  // minimum byte size would suggest as the new alignment.)
+  if (NeedsPacking && !IsPacked) {
+    diag(Struct->getLocation(),
+         "accessing fields in struct %0 is inefficient due to padding; only "
+         "needs %1 bytes but is using %2 bytes")
+        << Struct << (int)MinByteSize.getQuantity()
+        << (int)CurrSize.getQuantity()
+        << FixItHint::CreateInsertion(Struct->getEndLoc().getLocWithOffset(1),
+                                      " __attribute__((packed))");
+    diag(Struct->getLocation(),
+         "use \"__attribute__((packed))\" to reduce the amount of padding "
+         "applied to struct %0",
+         DiagnosticIDs::Note)
+        << Struct;
+  }
+
+  FixItHint FixIt;
+  AlignedAttr *Attribute = Struct->getAttr<AlignedAttr>();
+  std::string NewAlignQuantity = std::to_string((int)NewAlign.getQuantity());
+  if (Attribute) {
+    std::ostringstream FixItString;
+    FixItString << "aligned(" << NewAlignQuantity << ")";
+    FixIt =
+        FixItHint::CreateReplacement(Attribute->getRange(), FixItString.str());
+  } else {
+    std::ostringstream FixItString;
+    FixItString << " __attribute__((aligned(" << NewAlignQuantity << ")))";
+    FixIt = FixItHint::CreateInsertion(Struct->getEndLoc().getLocWithOffset(1),
+                                       FixItString.str());
+  }
+
+  // And suggest the minimum power-of-two alignment for the struct as a whole
+  // (with and without packing).
+  if (NeedsAlignment) {
+    diag(Struct->getLocation(),
+         "accessing fields in struct %0 is inefficient due to poor alignment; "
+         "currently aligned to %1 bytes, but recommended alignment is %2 bytes")
+        << Struct << (int)CurrAlign.getQuantity() << NewAlignQuantity << FixIt;
+
+    diag(Struct->getLocation(),
+         "use \"__attribute__((aligned(%0)))\" to align struct %1 to %0 bytes",
+         DiagnosticIDs::Note)
+        << NewAlignQuantity << Struct;
+  }
+}
+
+void StructPackAlignCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "MaxConfiguredAlignment", MaxConfiguredAlignment);
+}
+
+} // namespace altera
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/altera/StructPackAlignCheck.h

@@ -0,0 +1,41 @@
+//===--- StructPackAlignCheck.h - clang-tidy --------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_STRUCTPACKALIGNCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_STRUCTPACKALIGNCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace altera {
+
+/// Finds structs that are inefficiently packed or aligned, and recommends
+/// packing and/or aligning of said structs as needed.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/altera-struct-pack-align.html
+class StructPackAlignCheck : public ClangTidyCheck {
+public:
+  StructPackAlignCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context),
+    MaxConfiguredAlignment(Options.get("MaxConfiguredAlignment", 128)) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  const unsigned MaxConfiguredAlignment;
+  CharUnits computeRecommendedAlignment(CharUnits MinByteSize);
+};
+
+} // namespace altera
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_STRUCTPACKALIGNCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -67,6 +67,27 @@ The improvements are...
 Improvements to clang-tidy
 --------------------------
 
+New modules
+^^^^^^^^^^^
+
+- New :doc:`altera <clang-tidy/modules/altera>` module.
+
+  Includes checks related to OpenCL for FPGA coding guidelines, based on the
+  `Altera SDK for OpenCL: Best Practices Guide
+  <https://www.altera.com/en_US/pdfs/literature/hb/opencl-sdk/aocl_optimization_guide.pdf>`_.
+
+New checks
+^^^^^^^^^^
+
+- New :doc:`altera-struct-pack-align
+  <clang-tidy/checks/altera-struct-pack-align>` check.
+
+  Finds structs that are inefficiently packed or aligned, and recommends
+  packing and/or aligning of said structs as needed.
+
+- New :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc
+  <clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc>` check.
+
 - New :doc:`bugprone-redundant-branch-condition
   <clang-tidy/checks/bugprone-redundant-branch-condition>` check.
 

clang-tools-extra/docs/clang-tidy/checks/altera-struct-pack-align.rst

@@ -0,0 +1,54 @@
+.. title:: clang-tidy - altera-struct-pack-align
+
+altera-struct-pack-align
+========================
+
+Finds structs that are inefficiently packed or aligned, and recommends
+packing and/or aligning of said structs as needed.
+
+Structs that are not packed take up more space than they should, and accessing
+structs that are not well aligned is inefficient.
+
+Fix-its are provided to fix both of these issues by inserting and/or amending
+relevant struct attributes.
+
+Based on the `Altera SDK for OpenCL: Best Practices Guide
+<https://www.altera.com/en_US/pdfs/literature/hb/opencl-sdk/aocl_optimization_guide.pdf>`_.
+
+.. code-block:: c++
+
+  // The following struct is originally aligned to 4 bytes, and thus takes up
+  // 12 bytes of memory instead of 10. Packing the struct will make it use
+  // only 10 bytes of memory, and aligning it to 16 bytes will make it
+  // efficient to access.
+  struct example {
+    char a;    // 1 byte
+    double b;  // 8 bytes
+    char c;    // 1 byte
+  };
+
+  // The following struct is arranged in such a way that packing is not needed.
+  // However, it is aligned to 4 bytes instead of 8, and thus needs to be
+  // explicitly aligned.
+  struct implicitly_packed_example {
+    char a;  // 1 byte
+    char b;  // 1 byte
+    char c;  // 1 byte
+    char d;  // 1 byte
+    int e;   // 4 bytes
+  };
+
+  // The following struct is explicitly aligned and packed.
+  struct good_example {
+    char a;    // 1 byte
+    double b;  // 8 bytes
+    char c;    // 1 byte
+  } __attribute__((packed)) __attribute__((aligned(16));
+
+  // Explicitly aligning a struct to the wrong value will result in a warning.
+  // The following example should be aligned to 16 bytes, not 32.
+  struct badly_aligned_example {
+    char a;    // 1 byte
+    double b;  // 8 bytes
+    char c;    // 1 byte
+  } __attribute__((packed)) __attribute__((aligned(32)));

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -30,6 +30,7 @@ Clang-Tidy Checks
    `abseil-time-comparison <abseil-time-comparison.html>`_, "Yes"
    `abseil-time-subtraction <abseil-time-subtraction.html>`_, "Yes"
    `abseil-upgrade-duration-conversions <abseil-upgrade-duration-conversions.html>`_, "Yes"
+   `altera-struct-pack-align <altera-struct-pack-align.html>`_,
    `android-cloexec-accept <android-cloexec-accept.html>`_, "Yes"
    `android-cloexec-accept4 <android-cloexec-accept4.html>`_,
    `android-cloexec-creat <android-cloexec-creat.html>`_, "Yes"

clang-tools-extra/docs/clang-tidy/index.rst

@@ -58,6 +58,7 @@ There are currently the following groups of checks:
 Name prefix            Description
 ====================== =========================================================
 ``abseil-``            Checks related to Abseil library.
+``altera-``            Checks related to OpenCL programming for FPGAs.
 ``android-``           Checks related to Android.
 ``boost-``             Checks related to Boost library.
 ``bugprone-``          Checks that target bugprone code constructs.