new altera unroll loops check

This lint check is a part of the FLOCL (FPGA Linters for OpenCL)
project out of the Synergy Lab at Virginia Tech.

FLOCL is a set of lint checks aimed at FPGA developers who write code
in OpenCL.

The altera unroll loops check finds inner loops that have not been
unrolled, as well as fully-unrolled loops that should be partially
unrolled due to unknown loop bounds or a large number of loop
iterations.

Based on the Altera SDK for OpenCL: Best Practices Guide.

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

@@ -12,6 +12,7 @@
 #include "KernelNameRestrictionCheck.h"
 #include "SingleWorkItemBarrierCheck.h"
 #include "StructPackAlignCheck.h"
+#include "UnrollLoopsCheck.h"
 
 using namespace clang::ast_matchers;
 
@@ -28,6 +29,7 @@ class AlteraModule : public ClangTidyModule {
         "altera-single-work-item-barrier");
     CheckFactories.registerCheck<StructPackAlignCheck>(
         "altera-struct-pack-align");
+    CheckFactories.registerCheck<UnrollLoopsCheck>("altera-unroll-loops");
   }
 };
 

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

@@ -8,6 +8,7 @@ add_clang_library(clangTidyAlteraModule
   KernelNameRestrictionCheck.cpp
   SingleWorkItemBarrierCheck.cpp
   StructPackAlignCheck.cpp
+  UnrollLoopsCheck.cpp
 
   LINK_LIBS
   clangTidy

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

@@ -0,0 +1,277 @@
+//===--- UnrollLoopsCheck.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 "UnrollLoopsCheck.h"
+#include "clang/AST/APValue.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/OperationKinds.h"
+#include "clang/AST/ParentMapContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include <math.h>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace altera {
+
+UnrollLoopsCheck::UnrollLoopsCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      MaxLoopIterations(Options.get("MaxLoopIterations", 100U)) {}
+
+void UnrollLoopsCheck::registerMatchers(MatchFinder *Finder) {
+  const auto HasLoopBound = hasDescendant(
+      varDecl(allOf(matchesName("__end*"),
+                    hasDescendant(integerLiteral().bind("cxx_loop_bound")))));
+  const auto CXXForRangeLoop =
+      cxxForRangeStmt(anyOf(HasLoopBound, unless(HasLoopBound)));
+  const auto AnyLoop = anyOf(forStmt(), whileStmt(), doStmt(), CXXForRangeLoop);
+  Finder->addMatcher(
+      stmt(allOf(AnyLoop, unless(hasDescendant(stmt(AnyLoop))))).bind("loop"),
+      this);
+}
+
+void UnrollLoopsCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *Loop = Result.Nodes.getNodeAs<Stmt>("loop");
+  const auto *CXXLoopBound =
+      Result.Nodes.getNodeAs<IntegerLiteral>("cxx_loop_bound");
+  const ASTContext *Context = Result.Context;
+  switch (unrollType(Loop, Result.Context)) {
+  case NotUnrolled:
+    diag(Loop->getBeginLoc(),
+         "kernel performance could be improved by unrolling this loop with a "
+         "'#pragma unroll' directive");
+    break;
+  case PartiallyUnrolled:
+    // Loop already partially unrolled, do nothing.
+    break;
+  case FullyUnrolled:
+    if (hasKnownBounds(Loop, CXXLoopBound, Context)) {
+      if (hasLargeNumIterations(Loop, CXXLoopBound, Context)) {
+        diag(Loop->getBeginLoc(),
+             "loop likely has a large number of iterations and thus "
+             "cannot be fully unrolled; to partially unroll this loop, use "
+             "the '#pragma unroll <num>' directive");
+        return;
+      }
+      return;
+    }
+    if (isa<WhileStmt, DoStmt>(Loop)) {
+      diag(Loop->getBeginLoc(),
+           "full unrolling requested, but loop bounds may not be known; to "
+           "partially unroll this loop, use the '#pragma unroll <num>' "
+           "directive",
+           DiagnosticIDs::Note);
+      break;
+    }
+    diag(Loop->getBeginLoc(),
+         "full unrolling requested, but loop bounds are not known; to "
+         "partially unroll this loop, use the '#pragma unroll <num>' "
+         "directive");
+    break;
+  }
+}
+
+enum UnrollLoopsCheck::UnrollType
+UnrollLoopsCheck::unrollType(const Stmt *Statement, ASTContext *Context) {
+  const DynTypedNodeList Parents = Context->getParents<Stmt>(*Statement);
+  for (const DynTypedNode &Parent : Parents) {
+    const auto *ParentStmt = Parent.get<AttributedStmt>();
+    if (!ParentStmt)
+      continue;
+    for (const Attr *Attribute : ParentStmt->getAttrs()) {
+      const auto *LoopHint = dyn_cast<LoopHintAttr>(Attribute);
+      if (!LoopHint)
+        continue;
+      switch (LoopHint->getState()) {
+      case LoopHintAttr::Numeric:
+        return PartiallyUnrolled;
+      case LoopHintAttr::Disable:
+        return NotUnrolled;
+      case LoopHintAttr::Full:
+        return FullyUnrolled;
+      case LoopHintAttr::Enable:
+        return FullyUnrolled;
+      case LoopHintAttr::AssumeSafety:
+        return NotUnrolled;
+      case LoopHintAttr::FixedWidth:
+        return NotUnrolled;
+      case LoopHintAttr::ScalableWidth:
+        return NotUnrolled;
+      }
+    }
+  }
+  return NotUnrolled;
+}
+
+bool UnrollLoopsCheck::hasKnownBounds(const Stmt *Statement,
+                                      const IntegerLiteral *CXXLoopBound,
+                                      const ASTContext *Context) {
+  if (isa<CXXForRangeStmt>(Statement))
+    return CXXLoopBound != nullptr;
+  // Too many possibilities in a while statement, so always recommend partial
+  // unrolling for these.
+  if (isa<WhileStmt, DoStmt>(Statement))
+    return false;
+  // The last loop type is a for loop.
+  const auto *ForLoop = dyn_cast<ForStmt>(Statement);
+  if (!ForLoop)
+    llvm_unreachable("Unknown loop");
+  const Stmt *Initializer = ForLoop->getInit();
+  const Expr *Conditional = ForLoop->getCond();
+  const Expr *Increment = ForLoop->getInc();
+  if (!Initializer || !Conditional || !Increment)
+    return false;
+  // If the loop variable value isn't known, loop bounds are unknown.
+  if (const auto *InitDeclStatement = dyn_cast<DeclStmt>(Initializer)) {
+    if (const auto *VariableDecl =
+            dyn_cast<VarDecl>(InitDeclStatement->getSingleDecl())) {
+      APValue *Evaluation = VariableDecl->evaluateValue();
+      if (!Evaluation || !Evaluation->hasValue())
+        return false;
+    }
+  }
+  // If increment is unary and not one of ++ and --, loop bounds are unknown.
+  if (const auto *Op = dyn_cast<UnaryOperator>(Increment))
+    if (!Op->isIncrementDecrementOp())
+      return false;
+
+  if (isa<BinaryOperator>(Conditional)) {
+    const auto *BinaryOp = dyn_cast<BinaryOperator>(Conditional);
+    const Expr *LHS = BinaryOp->getLHS();
+    const Expr *RHS = BinaryOp->getRHS();
+    // If both sides are value dependent or constant, loop bounds are unknown.
+    return LHS->isEvaluatable(*Context) != RHS->isEvaluatable(*Context);
+  }
+  return false; // If it's not a binary operator, loop bounds are unknown.
+}
+
+const Expr *UnrollLoopsCheck::getCondExpr(const Stmt *Statement) {
+  if (const auto *ForLoop = dyn_cast<ForStmt>(Statement))
+    return ForLoop->getCond();
+  if (const auto *WhileLoop = dyn_cast<WhileStmt>(Statement))
+    return WhileLoop->getCond();
+  if (const auto *DoWhileLoop = dyn_cast<DoStmt>(Statement))
+    return DoWhileLoop->getCond();
+  if (const auto *CXXRangeLoop = dyn_cast<CXXForRangeStmt>(Statement))
+    return CXXRangeLoop->getCond();
+  llvm_unreachable("Unknown loop");
+}
+
+bool UnrollLoopsCheck::hasLargeNumIterations(const Stmt *Statement,
+                                             const IntegerLiteral *CXXLoopBound,
+                                             const ASTContext *Context) {
+  // Because hasKnownBounds is called before this, if this is true, then
+  // CXXLoopBound is also matched.
+  if (isa<CXXForRangeStmt>(Statement)) {
+    assert(CXXLoopBound && "CXX ranged for loop has no loop bound");
+    return exprHasLargeNumIterations(CXXLoopBound, Context);
+  }
+  const auto *ForLoop = dyn_cast<ForStmt>(Statement);
+  assert(ForLoop && "Unknown loop");
+  const Stmt *Initializer = ForLoop->getInit();
+  const Expr *Conditional = ForLoop->getCond();
+  const Expr *Increment = ForLoop->getInc();
+  int InitValue;
+  // If the loop variable value isn't known, we can't know the loop bounds.
+  if (const auto *InitDeclStatement = dyn_cast<DeclStmt>(Initializer)) {
+    if (const auto *VariableDecl =
+            dyn_cast<VarDecl>(InitDeclStatement->getSingleDecl())) {
+      APValue *Evaluation = VariableDecl->evaluateValue();
+      if (!Evaluation || !Evaluation->isInt())
+        return true;
+      InitValue = Evaluation->getInt().getExtValue();
+    }
+  }
+  assert(isa<BinaryOperator>(Conditional) &&
+         "Conditional is not a binary operator");
+  int EndValue;
+  const auto *BinaryOp = dyn_cast<BinaryOperator>(Conditional);
+  if (!extractValue(EndValue, BinaryOp, Context))
+    return true;
+
+  double Iterations;
+
+  // If increment is unary and not one of ++, --, we can't know the loop bounds.
+  if (const auto *Op = dyn_cast<UnaryOperator>(Increment)) {
+    if (Op->isIncrementOp())
+      Iterations = EndValue - InitValue;
+    else if (Op->isDecrementOp())
+      Iterations = InitValue - EndValue;
+    else
+      llvm_unreachable("Unary operator neither increment nor decrement");
+  }
+
+  // If increment is binary and not one of +, -, *, /, we can't know the loop
+  // bounds.
+  if (const auto *Op = dyn_cast<BinaryOperator>(Increment)) {
+    int ConstantValue;
+    if (!extractValue(ConstantValue, Op, Context))
+      return true;
+    switch (Op->getOpcode()) {
+    case (BO_AddAssign):
+      Iterations = ceil(float(EndValue - InitValue) / ConstantValue);
+      break;
+    case (BO_SubAssign):
+      Iterations = ceil(float(InitValue - EndValue) / ConstantValue);
+      break;
+    case (BO_MulAssign):
+      Iterations = 1 + (log(EndValue) - log(InitValue)) / log(ConstantValue);
+      break;
+    case (BO_DivAssign):
+      Iterations = 1 + (log(InitValue) - log(EndValue)) / log(ConstantValue);
+      break;
+    default:
+      // All other operators are not handled; assume large bounds.
+      return true;
+    }
+  }
+  return Iterations > MaxLoopIterations;
+}
+
+bool UnrollLoopsCheck::extractValue(int &Value, const BinaryOperator *Op,
+                                    const ASTContext *Context) {
+  const Expr *LHS = Op->getLHS();
+  const Expr *RHS = Op->getRHS();
+  Expr::EvalResult Result;
+  if (LHS->isEvaluatable(*Context))
+    LHS->EvaluateAsRValue(Result, *Context);
+  else if (RHS->isEvaluatable(*Context))
+    RHS->EvaluateAsRValue(Result, *Context);
+  else
+    return false; // Cannot evalue either side.
+  if (!Result.Val.isInt())
+    return false; // Cannot check number of iterations, return false to be
+                  // safe.
+  Value = Result.Val.getInt().getExtValue();
+  return true;
+}
+
+bool UnrollLoopsCheck::exprHasLargeNumIterations(const Expr *Expression,
+                                                 const ASTContext *Context) {
+  Expr::EvalResult Result;
+  if (Expression->EvaluateAsRValue(Result, *Context)) {
+    if (!Result.Val.isInt())
+      return false; // Cannot check number of iterations, return false to be
+                    // safe.
+    // The following assumes values go from 0 to Val in increments of 1.
+    return Result.Val.getInt() > MaxLoopIterations;
+  }
+  // Cannot evaluate Expression as an r-value, so cannot check number of
+  // iterations.
+  return false;
+}
+
+void UnrollLoopsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "MaxLoopIterations", MaxLoopIterations);
+}
+
+} // namespace altera
+} // namespace tidy
+} // namespace clang

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

@@ -0,0 +1,78 @@
+//===--- UnrollLoopsCheck.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_UNROLLLOOPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_UNROLLLOOPSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace altera {
+
+/// Finds inner loops that have not been unrolled, as well as fully unrolled
+/// loops with unknown loop bounds or a large number of iterations.
+///
+/// Unrolling inner loops could improve the performance of OpenCL kernels.
+/// However, if they have unknown loop bounds or a large number of iterations,
+/// they cannot be fully unrolled, and should be partially unrolled.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/altera-unroll-loops.html
+class UnrollLoopsCheck : public ClangTidyCheck {
+public:
+  UnrollLoopsCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  /// Recommend partial unrolling if number of loop iterations is greater than
+  /// MaxLoopIterations.
+  const unsigned MaxLoopIterations;
+  /// The kind of unrolling, if any, applied to a given loop.
+  enum UnrollType {
+    // This loop has no #pragma unroll directive associated with it.
+    NotUnrolled,
+    // This loop has a #pragma unroll directive associated with it.
+    FullyUnrolled,
+    // This loop has a #pragma unroll <num> directive associated with it.
+    PartiallyUnrolled
+  };
+  /// Attempts to extract an integer value from either side of the
+  /// BinaryOperator. Returns true and saves the result to &value if successful,
+  /// returns false otherwise.
+  bool extractValue(int &Value, const BinaryOperator *Op,
+                    const ASTContext *Context);
+  /// Returns true if the given loop statement has a large number of iterations,
+  /// as determined by the integer value in the loop's condition expression,
+  /// if one exists.
+  bool hasLargeNumIterations(const Stmt *Statement,
+                             const IntegerLiteral *CXXLoopBound,
+                             const ASTContext *Context);
+  /// Checks one hand side of the binary operator to ascertain if the upper
+  /// bound on the number of loops is greater than max_loop_iterations or not.
+  /// If the expression is not evaluatable or not an integer, returns false.
+  bool exprHasLargeNumIterations(const Expr *Expression,
+                                 const ASTContext *Context);
+  /// Returns the type of unrolling, if any, associated with the given
+  /// statement.
+  enum UnrollType unrollType(const Stmt *Statement, ASTContext *Context);
+  /// Returns the condition expression within a given for statement. If there is
+  /// none, or if the Statement is not a loop, then returns a NULL pointer.
+  const Expr *getCondExpr(const Stmt *Statement);
+  /// Returns True if the loop statement has known bounds.
+  bool hasKnownBounds(const Stmt *Statement, const IntegerLiteral *CXXLoopBound,
+                      const ASTContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+};
+
+} // namespace altera
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_UNROLLLOOPSCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -83,6 +83,12 @@ New checks
   Finds ``pthread_setcanceltype`` function calls where a thread's cancellation
   type is set to asynchronous.
 
+- New :doc:`altera-unroll-loops
+  <clang-tidy/checks/altera-unroll-loops>` check.
+
+  Finds inner loops that have not been unrolled, as well as fully unrolled
+  loops with unknown loops bounds or a large number of iterations.
+
 - New :doc:`cppcoreguidelines-prefer-member-initializer
   <clang-tidy/checks/cppcoreguidelines-prefer-member-initializer>` check.