[PATCH] Support constant expressions, including non-type template parameters, in pragma loop hints

Aaron Ballman aaron.ballman at gmail.com
Wed Jul 2 09:00:57 PDT 2014


Comments below (I tried not to duplicate other's comments, but agree
with the reviewers so far).

> Index: include/clang/AST/Attr.h
> ===================================================================
> --- include/clang/AST/Attr.h (revision 211958)
> +++ include/clang/AST/Attr.h (working copy)
> @@ -16,6 +16,7 @@
>
>  #include "clang/AST/AttrIterator.h"
>  #include "clang/AST/Decl.h"
> +#include "clang/AST/Expr.h"
>  #include "clang/AST/Type.h"
>  #include "clang/Basic/AttrKinds.h"
>  #include "clang/Basic/LLVM.h"
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td (revision 211958)
> +++ include/clang/Basic/Attr.td (working copy)
> @@ -1765,22 +1765,20 @@
>  }
>
>  def LoopHint : Attr {
> -  /// vectorize: vectorizes loop operations if 'value != 0'.
> +  /// vectorize: vectorizes loop operations if Enabled.
>    /// vectorize_width: vectorize loop operations with width 'value'.
> -  /// interleave: interleave multiple loop iterations if 'value != 0'.
> +  /// interleave: interleave multiple loop iterations if Enabled.
>    /// interleave_count: interleaves 'value' loop interations.
> -  /// unroll: unroll loop if 'value != 0'.
> +  /// unroll: unroll loop if Enabled.
>    /// unroll_count: unrolls loop 'value' times.
>
>    let Spellings = [Pragma<"clang", "loop">];
>
> -  /// State of the loop optimization specified by the spelling.
>    let Args = [EnumArgument<"Option", "OptionType",
> -                          ["vectorize", "vectorize_width", "interleave", "interleave_count",
> -                           "unroll", "unroll_count"],
> -                          ["Vectorize", "VectorizeWidth", "Interleave", "InterleaveCount",
> -                           "Unroll", "UnrollCount"]>,
> -              DefaultIntArgument<"Value", 1>];
> +                          ["vectorize", "vectorize_width", "interleave", "interleave_count", "unroll", "unroll_count"],
> +                          ["Vectorize", "VectorizeWidth", "Interleave", "InterleaveCount", "Unroll", "UnrollCount"]>,
> +              DefaultBoolArgument<"Enabled", 0>,
> +              ExprArgument<"Value", 0>];

You don't need the 0 for the ExprArgument (it already defaults to 0).

>    let AdditionalMembers = [{
>    static StringRef getOptionName(int Option) {
> @@ -1795,19 +1793,18 @@
>      llvm_unreachable("Unhandled LoopHint option.");
>    }
>
> -  static StringRef getValueName(int Value) {
> -    if (Value)
> +  static StringRef getEnabledName(bool Enabled) {
> +    if (Enabled)
>        return "enable";
>      return "disable";
>    }

Perhaps:
static const char *getEnabledName(bool Enabled) {
  return Enabled ? "enable" : "disable";
}

>    void printPrettyPragma(raw_ostream &OS, const PrintingPolicy &Policy) const {
>      OS << getOptionName(option) << "(";
> -    if (option == VectorizeWidth || option == InterleaveCount ||
> -        option == UnrollCount)
> -      OS << value;
> +    if (option == Vectorize || option == Interleave || option == Unroll)
> +      OS << getEnabledName(enabled);
>      else
> -      OS << getValueName(value);
> +      value->printPretty(OS, 0, Policy);
>      OS << ")\n";
>    }
>    }];
> Index: include/clang/Basic/DiagnosticGroups.td
> ===================================================================
> --- include/clang/Basic/DiagnosticGroups.td (revision 211958)
> +++ include/clang/Basic/DiagnosticGroups.td (working copy)
> @@ -708,3 +708,6 @@
>  // Instrumentation based profiling warnings.
>  def ProfileInstrOutOfDate : DiagGroup<"profile-instr-out-of-date">;
>  def ProfileInstrUnprofiled : DiagGroup<"profile-instr-unprofiled">;
> +
> +// #pragma loop directive based warnings.
> +def PragmaLoop : DiagGroup<"pragma-loop">;
> Index: include/clang/Basic/DiagnosticParseKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticParseKinds.td (revision 211958)
> +++ include/clang/Basic/DiagnosticParseKinds.td (working copy)
> @@ -913,6 +913,10 @@
>    "vectorize_width, interleave, interleave_count, unroll, or unroll_count">;
>  def err_pragma_loop_missing_argument : Error<
>    "missing argument to loop pragma %0">;
> +def err_pragma_loop_invalid_expression : Error<
> +  "invalid argument; expected a constant integer expression">;
> +def err_pragma_loop_invalid_keyword : Error<
> +  "invalid argument; expected 'enable' or 'disable'">;
>  } // end of Parse Issue category.
>
>  let CategoryName = "Modules Issue" in {
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td (revision 211958)
> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
> @@ -544,8 +544,6 @@
>    "surrounding namespace with visibility attribute starts here">;
>  def err_pragma_loop_invalid_value : Error<
>    "invalid argument; expected a positive integer value">;
> -def err_pragma_loop_invalid_keyword : Error<
> -  "invalid argument; expected 'enable' or 'disable'">;
>  def err_pragma_loop_compatibility : Error<
>    "%select{incompatible|duplicate}0 directives '%1(%2)' and '%3(%4)'">;
>  def err_pragma_loop_precedes_nonloop : Error<
> Index: include/clang/Sema/LoopHint.h
> ===================================================================
> --- include/clang/Sema/LoopHint.h (revision 211958)
> +++ include/clang/Sema/LoopHint.h (working copy)
> @@ -22,8 +22,12 @@
>    SourceRange Range;
>    Expr *ValueExpr;
>    IdentifierLoc *LoopLoc;
> -  IdentifierLoc *ValueLoc;
>    IdentifierLoc *OptionLoc;
> +  IdentifierLoc *EnabledLoc;
> +
> +  LoopHint()
> +      : ValueExpr(nullptr), LoopLoc(nullptr), OptionLoc(nullptr),
> +        EnabledLoc(nullptr) {}
>  };
>
>  } // end namespace clang
> Index: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h (revision 211958)
> +++ include/clang/Sema/Sema.h (working copy)
> @@ -3436,6 +3436,9 @@
>                                   PredefinedExpr::IdentType IT);
>    ExprResult ActOnPredefinedExpr(SourceLocation Loc, tok::TokenKind Kind);
>    ExprResult ActOnIntegerConstant(SourceLocation Loc, uint64_t Val);
> +
> +  bool DiagnoseLoopHintValue(Expr *ValueExpr);
> +
>    ExprResult ActOnNumericConstant(const Token &Tok, Scope *UDLScope = nullptr);
>    ExprResult ActOnCharacterConstant(const Token &Tok,
>                                      Scope *UDLScope = nullptr);
> Index: lib/CodeGen/CGStmt.cpp
> ===================================================================
> --- lib/CodeGen/CGStmt.cpp (revision 211958)
> +++ lib/CodeGen/CGStmt.cpp (working copy)
> @@ -550,7 +550,6 @@
>        continue;
>
>      LoopHintAttr::OptionType Option = LH->getOption();
> -    int ValueInt = LH->getValue();
>
>      const char *MetadataName;
>      switch (Option) {
> @@ -570,34 +569,46 @@
>        break;
>      }
>
> +    bool Enabled = LH->getEnabled();
> +    Expr *ValueExpr = LH->getValue();
> +    int ValueInt = 1;
> +
> +    if (ValueExpr) {
> +      llvm::APSInt ValueAPS;
> +      bool IsICE = ValueExpr->isIntegerConstantExpr(ValueAPS, CGM.getContext());
> +      (void)IsICE; // So release build do not warn about unused variables
> +      assert(IsICE &&
> +             "Loop hint argument is not an integer constant expression");
> +      ValueInt = ValueAPS.getSExtValue();
> +    }
> +
>      llvm::Value *Value;
>      llvm::MDString *Name;
> +
>      switch (Option) {
>      case LoopHintAttr::Vectorize:
>      case LoopHintAttr::Interleave:
> -      if (ValueInt == 1) {
> +      if (Enabled) {
>          // FIXME: In the future I will modifiy the behavior of the metadata
>          // so we can enable/disable vectorization and interleaving separately.
>          Name = llvm::MDString::get(Context, "llvm.loop.vectorize.enable");
>          Value = Builder.getTrue();
>          break;
>        }
> +
>        // Vectorization/interleaving is disabled, set width/count to 1.
>        ValueInt = 1;
>        // Fallthrough.
>      case LoopHintAttr::VectorizeWidth:
>      case LoopHintAttr::InterleaveCount:
> +    case LoopHintAttr::UnrollCount:
>        Name = llvm::MDString::get(Context, MetadataName);
>        Value = llvm::ConstantInt::get(Int32Ty, ValueInt);
>        break;
>      case LoopHintAttr::Unroll:
>        Name = llvm::MDString::get(Context, MetadataName);
> -      Value = (ValueInt == 0) ? Builder.getFalse() : Builder.getTrue();
> +      Value = Enabled ? Builder.getTrue() : Builder.getFalse();
>        break;
> -    case LoopHintAttr::UnrollCount:
> -      Name = llvm::MDString::get(Context, MetadataName);
> -      Value = llvm::ConstantInt::get(Int32Ty, ValueInt);
> -      break;
>      }
>
>      SmallVector<llvm::Value *, 2> OpValues;
> Index: lib/Parse/ParsePragma.cpp
> ===================================================================
> --- lib/Parse/ParsePragma.cpp (revision 211958)
> +++ lib/Parse/ParsePragma.cpp (working copy)
> @@ -601,8 +601,8 @@
>
>  struct PragmaLoopHintInfo {
>    Token Loop;
> -  Token Value;
>    Token Option;
> +  std::pair<Token *, size_t> Values;
>  };
>
>  LoopHint Parser::HandlePragmaLoopHint() {

Instead of requiring the caller to inspect all of the fields, why not
return a bool, and pass the LoopHint by reference?

> @@ -610,26 +610,69 @@
>    PragmaLoopHintInfo *Info =
>        static_cast<PragmaLoopHintInfo *>(Tok.getAnnotationValue());
>
> +  if (!Info)
> +    return LoopHint(); // Uninitialized loop hints are ignored.

How do we get into the situation where Info is null?

> +
>    LoopHint Hint;
>    Hint.LoopLoc =
>        IdentifierLoc::create(Actions.Context, Info->Loop.getLocation(),
>                              Info->Loop.getIdentifierInfo());
> -  Hint.OptionLoc =
> -      IdentifierLoc::create(Actions.Context, Info->Option.getLocation(),
> -                            Info->Option.getIdentifierInfo());
> -  Hint.ValueLoc =
> -      IdentifierLoc::create(Actions.Context, Info->Value.getLocation(),
> -                            Info->Value.getIdentifierInfo());
> +
> +  IdentifierInfo *OptionInfo = Info->Option.getIdentifierInfo();
> +  Hint.OptionLoc = IdentifierLoc::create(
> +      Actions.Context, Info->Option.getLocation(), OptionInfo);
> +
> +  Token *Args = Info->Values.first;
> +  size_t ArgSize = Info->Values.second;
> +  assert(ArgSize > 0 &&
> +         "LoopHintInfo::Values must contain at least one token.");
>    Hint.Range =
> -      SourceRange(Info->Option.getLocation(), Info->Value.getLocation());
> +      SourceRange(Info->Option.getLocation(), Args[ArgSize - 1].getLocation());
>
> -  // FIXME: We should allow non-type template parameters for the loop hint
> -  // value. See bug report #19610
> -  if (Info->Value.is(tok::numeric_constant))
> -    Hint.ValueExpr = Actions.ActOnNumericConstant(Info->Value).get();
> -  else
> -    Hint.ValueExpr = nullptr;
> +  bool EnabledOption = llvm::StringSwitch<bool>(OptionInfo->getName())
> +                           .Case("vectorize", true)
> +                           .Case("interleave", true)
> +                           .Case("unroll", true)
> +                           .Default(false);
>
> +  if (EnabledOption) {
> +    ConsumeToken(); // The annotation token.
> +    IdentifierInfo *EnabledInfo = Args[0].getIdentifierInfo();
> +    SourceLocation EnabledLoc = Args[0].getLocation();
> +    if (!EnabledInfo ||
> +        (!EnabledInfo->isStr("enable") && !EnabledInfo->isStr("disable"))) {
> +      Diag(EnabledLoc, diag::err_pragma_loop_invalid_keyword);
> +      return LoopHint(); // Uninitialized loop hints are ignored.
> +    }
> +
> +    Hint.EnabledLoc =
> +        IdentifierLoc::create(Actions.Context, EnabledLoc, EnabledInfo);
> +  } else {
> +    PP.EnterTokenStream(Args, ArgSize, false, false);
> +    ConsumeToken(); // The annotation token.
> +
> +    // Suppress diagnostic messages so we can report and invalid expression if
> +    // ParseConstantExpression fails.
> +    bool Suppress = Diags.getSuppressAllDiagnostics();
> +    Diags.setSuppressAllDiagnostics(true);
> +    ExprResult R = ParseConstantExpression();
> +    Diags.setSuppressAllDiagnostics(Suppress);
> +
> +    if (!R.isUsable()) {
> +      Diag(Args[0].getLocation(), diag::err_pragma_loop_invalid_expression);
> +      return LoopHint(); // Uninitialized loop hints are ignored.
> +    }
> +
> +    QualType QT = R.get()->getType();
> +    if (!QT->isIntegerType() || QT->isBooleanType() || QT->isCharType()) {

Are char and bool types truly that heinous? (I agree they would look
odd, but do they require restricting?). What about scoped
enumerations?

> +      Diag(Args[0].getLocation(), diag::err_pragma_loop_invalid_expression);
> +      return LoopHint(); // Uninitialized loop hints are ignored.
> +    }
> +
> +    // Argument is a constant expression with an integer type.
> +    Hint.ValueExpr = R.get();
> +  }
> +
>    return Hint;
>  }
>
> @@ -1668,6 +1711,7 @@
>  /// try to unroll the loop the number of times indicated by the value.
>  /// If unroll(enable) and unroll_count are both specified only
>  /// unroll_count takes effect.
> +///

Spurious comments.

>  void PragmaLoopHintHandler::HandlePragma(Preprocessor &PP,
>                                           PragmaIntroducerKind Introducer,
>                                           Token &Tok) {
> @@ -1707,19 +1751,21 @@
>        return;
>      }
>
> -    // FIXME: All tokens between '(' and ')' should be stored and parsed as a
> -    // constant expression.
> +    SmallVector<Token, 1> ValueList;
>      PP.Lex(Tok);
> -    if (Tok.is(tok::r_paren)) {
> +    while (Tok.isNot(tok::r_paren) && Tok.isNot(tok::eod)) {
> +      ValueList.push_back(Tok);
> +      PP.Lex(Tok);
> +    }
> +
> +    if (ValueList.empty()) {
>        // Nothing between the parentheses.
>        PP.Diag(Tok.getLocation(), diag::err_pragma_loop_missing_argument)
>            << OptionInfo;
>        return;
>      }
> -    Token Value = Tok;
>
>      // Read ')'
> -    PP.Lex(Tok);
>      if (Tok.isNot(tok::r_paren)) {
>        PP.Diag(Tok.getLocation(), diag::err_expected) << tok::r_paren;
>        return;
> @@ -1731,8 +1777,12 @@
>      auto *Info = new (PP.getPreprocessorAllocator()) PragmaLoopHintInfo;
>      Info->Loop = Loop;
>      Info->Option = Option;
> -    Info->Value = Value;
>
> +    Token *TokenArray = new Token[ValueList.size()];

What is responsible for freeing this memory?

> +    std::copy(ValueList.begin(), ValueList.end(), TokenArray);
> +    Info->Values = std::pair<Token *, size_t>(
> +        std::make_pair(TokenArray, ValueList.size()));
> +
>      // Generate the vectorization hint token.
>      Token LoopHintTok;
>      LoopHintTok.startToken();
> Index: lib/Parse/ParseStmt.cpp
> ===================================================================
> --- lib/Parse/ParseStmt.cpp (revision 211958)
> +++ lib/Parse/ParseStmt.cpp (working copy)
> @@ -1807,12 +1807,12 @@
>    // Get vectorize hints and consume annotated token.
>    while (Tok.is(tok::annot_pragma_loop_hint)) {
>      LoopHint Hint = HandlePragmaLoopHint();
> -    ConsumeToken();
>
> -    if (!Hint.LoopLoc || !Hint.OptionLoc || !Hint.ValueLoc)
> +    if (!Hint.LoopLoc || !Hint.OptionLoc ||
> +        !(Hint.EnabledLoc || Hint.ValueExpr))
>        continue;
>
> -    ArgsUnion ArgHints[] = {Hint.OptionLoc, Hint.ValueLoc,
> +    ArgsUnion ArgHints[] = {Hint.OptionLoc, Hint.EnabledLoc,
>                              ArgsUnion(Hint.ValueExpr)};
>      TempAttrs.addNew(Hint.LoopLoc->Ident, Hint.Range, nullptr,
>                       Hint.LoopLoc->Loc, ArgHints, 3, AttributeList::AS_Pragma);
> Index: lib/Sema/SemaExpr.cpp
> ===================================================================
> --- lib/Sema/SemaExpr.cpp (revision 211958)
> +++ lib/Sema/SemaExpr.cpp (working copy)
> @@ -3046,6 +3046,23 @@
>    return FloatingLiteral::Create(S.Context, Val, isExact, Ty, Loc);
>  }
>
> +bool Sema::DiagnoseLoopHintValue(Expr *ValueExpr) {

This should take a const Expr *.

> +  assert(ValueExpr && "Attribute must have a value value expression.");
> +
> +  if (ValueExpr->isValueDependent())
> +    return false;
> +
> +  int ValueInt;
> +  llvm::APSInt ValueAPS;
> +  if (!ValueExpr->isIntegerConstantExpr(ValueAPS, getASTContext()) ||
> +      (ValueInt = ValueAPS.getSExtValue()) < 1) {
> +    Diag(ValueExpr->getExprLoc(), diag::err_pragma_loop_invalid_value);
> +    return true;
> +  }
> +
> +  return false;
> +}
> +
>  ExprResult Sema::ActOnNumericConstant(const Token &Tok, Scope *UDLScope) {
>    // Fast path for a single digit (which is quite common).  A single digit
>    // cannot have a trigraph, escaped newline, radix prefix, or suffix.
> Index: lib/Sema/SemaStmtAttr.cpp
> ===================================================================
> --- lib/Sema/SemaStmtAttr.cpp (revision 211958)
> +++ lib/Sema/SemaStmtAttr.cpp (working copy)
> @@ -18,6 +18,7 @@
>  #include "clang/Sema/Lookup.h"
>  #include "clang/Sema/LoopHint.h"
>  #include "clang/Sema/ScopeInfo.h"
> +#include "clang/Sema/Sema.h"
>  #include "llvm/ADT/StringExtras.h"
>
>  using namespace clang;
> @@ -54,12 +55,12 @@
>    }
>
>    IdentifierLoc *OptionLoc = A.getArgAsIdent(0);
> -  IdentifierInfo *OptionInfo = OptionLoc->Ident;
> -  IdentifierLoc *ValueLoc = A.getArgAsIdent(1);
> -  IdentifierInfo *ValueInfo = ValueLoc->Ident;
> +  IdentifierLoc *EnabledLoc = A.getArgAsIdent(1);
>    Expr *ValueExpr = A.getArgAsExpr(2);
>
> -  assert(OptionInfo && "Attribute must have valid option info.");
> +  assert(OptionLoc && OptionLoc->Ident &&
> +         "Attribute must have valid option info.");
> +  IdentifierInfo *OptionInfo = OptionLoc->Ident;
>
>    LoopHintAttr::OptionType Option =
>        llvm::StringSwitch<LoopHintAttr::OptionType>(OptionInfo->getName())
> @@ -71,36 +72,23 @@
>            .Case("unroll_count", LoopHintAttr::UnrollCount)
>            .Default(LoopHintAttr::Vectorize);
>
> -  int ValueInt;
> +  bool Enabled = true;
>    if (Option == LoopHintAttr::Vectorize || Option == LoopHintAttr::Interleave ||
>        Option == LoopHintAttr::Unroll) {
> -    if (!ValueInfo) {
> -      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword);
> -      return nullptr;
> -    }
> -    if (ValueInfo->isStr("disable"))
> -      ValueInt = 0;
> -    else if (ValueInfo->isStr("enable"))
> -      ValueInt = 1;
> -    else {
> -      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword);
> -      return nullptr;
> -    }
> +    assert(EnabledLoc && EnabledLoc->Ident &&
> +           "Attribute must have a valid enable info.");
> +    IdentifierInfo *EnabledInfo = EnabledLoc->Ident;
> +    if (EnabledInfo && EnabledInfo->isStr("disable"))
> +      Enabled = false;
>    } else if (Option == LoopHintAttr::VectorizeWidth ||
>               Option == LoopHintAttr::InterleaveCount ||
>               Option == LoopHintAttr::UnrollCount) {
> -    // FIXME: We should support template parameters for the loop hint value.
> -    // See bug report #19610.
> -    llvm::APSInt ValueAPS;
> -    if (!ValueExpr || !ValueExpr->isIntegerConstantExpr(ValueAPS, S.Context) ||
> -        (ValueInt = ValueAPS.getSExtValue()) < 1) {
> -      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_value);
> +    assert(ValueExpr && "Attribute must have a valid value expression.");
> +    if (S.DiagnoseLoopHintValue(ValueExpr))
>        return nullptr;
> -    }
> -  } else
> -    llvm_unreachable("Unknown loop hint option");
> +  }
>
> -  return LoopHintAttr::CreateImplicit(S.Context, Option, ValueInt,
> +  return LoopHintAttr::CreateImplicit(S.Context, Option, Enabled, ValueExpr,
>                                        A.getRange());
>  }
>
> @@ -118,13 +106,13 @@
>      bool EnabledIsSet;
>      bool ValueIsSet;
>      bool Enabled;
> -    int Value;
> +    Expr *Value;
>    } Options[] = {{LoopHintAttr::Vectorize, LoopHintAttr::VectorizeWidth, false,
> -                  false, false, 0},
> +                  false, false, nullptr},
>                   {LoopHintAttr::Interleave, LoopHintAttr::InterleaveCount,
> -                  false, false, false, 0},
> +                  false, false, false, nullptr},
>                   {LoopHintAttr::Unroll, LoopHintAttr::UnrollCount, false, false,
> -                  false, 0}};
> +                  false, nullptr}};
>
>    for (const auto *I : Attrs) {
>      const LoopHintAttr *LH = dyn_cast<LoopHintAttr>(I);
> @@ -134,8 +122,6 @@
>        continue;
>
>      int Option = LH->getOption();
> -    int ValueInt = LH->getValue();
> -
>      int Category;
>      switch (Option) {
>      case LoopHintAttr::Vectorize:
> @@ -154,30 +140,34 @@
>
>      auto &CategoryState = Options[Category];
>      SourceLocation ValueLoc = LH->getRange().getEnd();
> +
>      if (Option == LoopHintAttr::Vectorize ||
>          Option == LoopHintAttr::Interleave || Option == LoopHintAttr::Unroll) {
> +      bool Enabled = LH->getEnabled();
>        // Enable|disable hint.  For example, vectorize(enable).
>        if (CategoryState.EnabledIsSet) {
>          // Cannot specify enable/disable state twice.
>          S.Diag(ValueLoc, diag::err_pragma_loop_compatibility)
>              << /*Duplicate=*/true << LoopHintAttr::getOptionName(Option)
> -            << LoopHintAttr::getValueName(CategoryState.Enabled)
> +            << LoopHintAttr::getEnabledName(CategoryState.Enabled)
>              << LoopHintAttr::getOptionName(Option)
> -            << LoopHintAttr::getValueName(ValueInt);
> +            << LoopHintAttr::getEnabledName(Enabled);
>        }
> +
>        CategoryState.EnabledIsSet = true;
> -      CategoryState.Enabled = ValueInt;
> +      CategoryState.Enabled = Enabled;
>      } else {
> +      Expr *Value = LH->getValue();
>        // Numeric hint.  For example, unroll_count(8).
>        if (CategoryState.ValueIsSet) {
>          // Cannot specify numeric hint twice.
>          S.Diag(ValueLoc, diag::err_pragma_loop_compatibility)
>              << /*Duplicate=*/true << LoopHintAttr::getOptionName(Option)
>              << CategoryState.Value << LoopHintAttr::getOptionName(Option)
> -            << ValueInt;
> +            << Value;
>        }
>        CategoryState.ValueIsSet = true;
> -      CategoryState.Value = ValueInt;
> +      CategoryState.Value = Value;
>      }
>
>      if (CategoryState.EnabledIsSet && !CategoryState.Enabled &&
> @@ -187,7 +177,7 @@
>        S.Diag(ValueLoc, diag::err_pragma_loop_compatibility)
>            << /*Duplicate=*/false
>            << LoopHintAttr::getOptionName(CategoryState.EnableOptionId)
> -          << LoopHintAttr::getValueName(CategoryState.Enabled)
> +          << LoopHintAttr::getEnabledName(CategoryState.Enabled)
>            << LoopHintAttr::getOptionName(CategoryState.NumericOptionId)
>            << CategoryState.Value;
>      }
> Index: lib/Sema/SemaTemplateInstantiate.cpp
> ===================================================================
> --- lib/Sema/SemaTemplateInstantiate.cpp (revision 211958)
> +++ lib/Sema/SemaTemplateInstantiate.cpp (working copy)
> @@ -767,6 +767,8 @@
>                            QualType ObjectType = QualType(),
>                            NamedDecl *FirstQualifierInScope = nullptr);
>
> +    const LoopHintAttr *TransformLoopHintAttr(const LoopHintAttr *LH);
> +
>      ExprResult TransformPredefinedExpr(PredefinedExpr *E);
>      ExprResult TransformDeclRefExpr(DeclRefExpr *E);
>      ExprResult TransformCXXDefaultArgExpr(CXXDefaultArgExpr *E);
> @@ -1127,6 +1129,25 @@
>    return transformNonTypeTemplateParmRef(NTTP, E->getLocation(), Arg);
>  }
>
> +const LoopHintAttr *
> +TemplateInstantiator::TransformLoopHintAttr(const LoopHintAttr *LH) {
> +  // Only transform non-type template parameters.
> +  if (!LH->getValue() || !LH->getValue()->isValueDependent())
> +    return LH;
> +
> +  Expr *TransformedExpr = getDerived().TransformExpr(LH->getValue()).get();
> +
> +  // Generate error if there is a problem with the value.
> +  if (getSema().DiagnoseLoopHintValue(TransformedExpr))
> +    return LH;
> +
> +  // Create new LoopHintValueAttr with integral expression in place of the
> +  // non-type template parameter.
> +  return LoopHintAttr::CreateImplicit(getSema().Context, LH->getOption(),
> +                                      LH->getEnabled(), TransformedExpr,
> +                                      LH->getRange());
> +}
> +
>  ExprResult TemplateInstantiator::transformNonTypeTemplateParmRef(
>                                                   NonTypeTemplateParmDecl *parm,
>                                                   SourceLocation loc,
> Index: lib/Sema/TreeTransform.h
> ===================================================================
> --- lib/Sema/TreeTransform.h (revision 211958)
> +++ lib/Sema/TreeTransform.h (working copy)
> @@ -327,6 +327,26 @@
>    /// \returns the transformed OpenMP clause.
>    OMPClause *TransformOMPClause(OMPClause *S);
>
> +  /// \brief Transform the given attribute.
> +  ///
> +  /// By default, this routine transforms a statement by delegating to the
> +  /// appropriate TransformXXXAttr function to transform a specific kind
> +  /// of attribute. Subclasses may override this function to transform
> +  /// attributed statements using some other mechanism.
> +  ///
> +  /// \returns the transformed attribute
> +  const Attr *TransformAttr(const Attr *S);
> +
> +/// \brief Transform the specified attribute.
> +///
> +/// Subclasses should override the desired attribute transformions.
> +///
> +/// \returns the transformed attribute.
> +#define ATTR(X)
> +#define PRAGMA_ATTR(X)                                                         \
> +  const X##Attr *Transform##X##Attr(const X##Attr *R) { return R; }
> +#include "clang/Basic/AttrList.inc"
> +
>    /// \brief Transform the given expression.
>    ///
>    /// By default, this routine transforms an expression by delegating to the
> @@ -5490,19 +5510,43 @@
>                                         SubStmt.get());
>  }
>
> -template<typename Derived>
> -StmtResult
> -TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S) {
> +template <typename Derived>
> +const Attr *TreeTransform<Derived>::TransformAttr(const Attr *R) {
> +  if (!R)
> +    return R;
> +
> +  switch (R->getKind()) {
> +// Transform expressions by calling TransformExpr.
> +#define ATTR(X)
> +#define PRAGMA_ATTR(X)                                                         \
> +  case attr::X:                                                                \
> +    return getDerived().Transform##X##Attr(dyn_cast<X##Attr>(R));

I think this should use cast<> instead of dyn_cast.

> +#include "clang/Basic/AttrList.inc"
> +  default:
> +    return R;
> +  }
> +}
> +
> +template <typename Derived>
> +StmtResult TreeTransform<Derived>::TransformAttributedStmt(AttributedStmt *S) {
> +  bool AttrsChanged = false;
> +  SmallVector<const Attr *, 1> Attrs;
> +
> +  // Visit attributes and keep track if any are transformed.
> +  for (auto *I : S->getAttrs()) {
> +    const Attr *R = getDerived().TransformAttr(I);
> +    AttrsChanged |= (I != R);
> +    Attrs.push_back(R);
> +  }
> +
>    StmtResult SubStmt = getDerived().TransformStmt(S->getSubStmt());
>    if (SubStmt.isInvalid())
>      return StmtError();
>
> -  // TODO: transform attributes
> -  if (SubStmt.get() == S->getSubStmt() /* && attrs are the same */)
> +  if (SubStmt.get() == S->getSubStmt() && !AttrsChanged)
>      return S;
>
> -  return getDerived().RebuildAttributedStmt(S->getAttrLoc(),
> -                                            S->getAttrs(),
> +  return getDerived().RebuildAttributedStmt(S->getAttrLoc(), Attrs,
>                                              SubStmt.get());
>  }
>
> Index: test/CodeGen/pragma-loop.cpp
> ===================================================================
> --- test/CodeGen/pragma-loop.cpp (revision 211958)
> +++ test/CodeGen/pragma-loop.cpp (working copy)
> @@ -2,7 +2,7 @@
>
>  // Verify while loop is recognized after sequence of pragma clang loop directives.
>  void while_test(int *List, int Length) {
> -  // CHECK: define {{.*}} @_Z10while_test
> +  // CHECK: define {{.*}}while_test
>    int i = 0;
>
>  #pragma clang loop vectorize(enable)
> @@ -18,6 +18,7 @@
>
>  // Verify do loop is recognized after multi-option pragma clang loop directive.
>  void do_test(int *List, int Length) {
> +  // CHECK: define {{.*}}do_test
>    int i = 0;
>
>  #pragma clang loop vectorize_width(8) interleave_count(4) unroll(disable)
> @@ -30,6 +31,7 @@
>
>  // Verify for loop is recognized after sequence of pragma clang loop directives.
>  void for_test(int *List, int Length) {
> +// CHECK: define {{.*}}for_test
>  #pragma clang loop interleave(enable)
>  #pragma clang loop interleave_count(4)
>  #pragma clang loop unroll_count(8)
> @@ -42,6 +44,7 @@
>  // Verify c++11 for range loop is recognized after
>  // sequence of pragma clang loop directives.
>  void for_range_test() {
> +  // CHECK: define {{.*}}for_range_test
>    double List[100];
>
>  #pragma clang loop vectorize_width(2) interleave_count(2)
> @@ -53,6 +56,7 @@
>
>  // Verify disable pragma clang loop directive generates correct metadata
>  void disable_test(int *List, int Length) {
> +// CHECK: define {{.*}}disable_test
>  #pragma clang loop vectorize(disable) unroll(disable)
>    for (int i = 0; i < Length; i++) {
>      // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_5:.*]]
> @@ -64,8 +68,9 @@
>  #define INTCOUNT 2
>  #define UNROLLCOUNT 8
>
> -// Verify defines are correctly resolved in pragma clang loop directive
> +// Verify defines are correctly resolved in pragma clang loop directive.
>  void for_define_test(int *List, int Length, int Value) {
> +// CHECK: define {{.*}}for_define_test
>  #pragma clang loop vectorize_width(VECWIDTH) interleave_count(INTCOUNT)
>  #pragma clang loop unroll_count(UNROLLCOUNT)
>    for (int i = 0; i < Length; i++) {
> @@ -74,13 +79,29 @@
>    }
>  }
>
> +// Verify constant expressions are handled correctly.
> +void for_contant_expression_test(int *List, int Length) {
> +// CHECK: define {{.*}}for_contant_expression_test
> +#pragma clang loop vectorize_width(1 + 4)
> +  for (int i = 0; i < Length; i++) {
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_7:.*]]
> +    List[i] = i;
> +  }
> +
> +#pragma clang loop vectorize_width(3 + VECWIDTH)
> +  for (int i = 0; i < Length; i++) {
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_8:.*]]
> +    List[i] += i;
> +  }
> +}
> +
>  // Verify metadata is generated when template is used.
>  template <typename A>
>  void for_template_test(A *List, int Length, A Value) {
> -
> +// CHECK: define {{.*}}for_template_test
>  #pragma clang loop vectorize_width(8) interleave_count(8) unroll_count(8)
>    for (int i = 0; i < Length; i++) {
> -    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_7:.*]]
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_9:.*]]
>      List[i] = i * Value;
>    }
>  }
> @@ -88,14 +109,39 @@
>  // Verify define is resolved correctly when template is used.
>  template <typename A>
>  void for_template_define_test(A *List, int Length, A Value) {
> +// CHECK: define {{.*}}for_template_define_test
>  #pragma clang loop vectorize_width(VECWIDTH) interleave_count(INTCOUNT)
>  #pragma clang loop unroll_count(UNROLLCOUNT)
>    for (int i = 0; i < Length; i++) {
> -    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_8:.*]]
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_10:.*]]
>      List[i] = i * Value;
>    }
>  }
>
> +// Verify templates and constant expressions are handled correctly.
> +template <typename A, int V, int I, int U>
> +void for_template_constant_expression_test(A *List, int Length) {
> +// CHECK: define {{.*}}for_template_constant_expression_test
> +#pragma clang loop vectorize_width(V) interleave_count(I) unroll_count(U)
> +  for (int i = 0; i < Length; i++) {
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_11:.*]]
> +    List[i] = i;
> +  }
> +
> +#pragma clang loop vectorize_width(V * 2 + VECWIDTH) interleave_count(I * 2 + INTCOUNT) unroll_count(U * 2 + UNROLLCOUNT)
> +  for (int i = 0; i < Length; i++) {
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_12:.*]]
> +    List[i] += i;
> +  }
> +
> +  const int Scale = 4;
> +#pragma clang loop vectorize_width(Scale *V) interleave_count(Scale *I) unroll_count(Scale *U)
> +  for (int i = 0; i < Length; i++) {
> +    // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_13:.*]]
> +    List[i] += i;
> +  }
> +}
> +
>  #undef VECWIDTH
>  #undef INTCOUNT
>  #undef UNROLLCOUNT
> @@ -106,6 +152,7 @@
>
>    for_template_test<double>(List, Length, Value);
>    for_template_define_test<double>(List, Length, Value);
> +  for_template_constant_expression_test<double, 2, 4, 8>(List, Length);
>  }
>
>  // CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata ![[UNROLLENABLE_1:.*]], metadata ![[WIDTH_4:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[INTENABLE_1:.*]]}
> @@ -124,6 +171,17 @@
>  // CHECK: ![[LOOP_5]] = metadata !{metadata ![[LOOP_5]], metadata ![[UNROLLENABLE_0:.*]], metadata ![[WIDTH_1:.*]]}
>  // CHECK: ![[WIDTH_1]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 1}
>  // CHECK: ![[LOOP_6]] = metadata !{metadata ![[LOOP_6]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_2:.*]], metadata ![[WIDTH_2:.*]]}
> -// CHECK: ![[LOOP_7]] = metadata !{metadata ![[LOOP_7]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_8:.*]], metadata ![[WIDTH_8:.*]]}
> +// CHECK: ![[LOOP_7]] = metadata !{metadata ![[LOOP_7]], metadata ![[WIDTH_5:.*]]}
> +// CHECK: ![[WIDTH_5]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 5}
> +// CHECK: ![[LOOP_8]] = metadata !{metadata ![[LOOP_8]], metadata ![[WIDTH_5:.*]]}
> +// CHECK: ![[LOOP_9]] = metadata !{metadata ![[LOOP_9]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_8:.*]], metadata ![[WIDTH_8:.*]]}
>  // CHECK: ![[INTERLEAVE_8]] = metadata !{metadata !"llvm.loop.vectorize.unroll", i32 8}
> -// CHECK: ![[LOOP_8]] = metadata !{metadata ![[LOOP_8]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_2:.*]], metadata ![[WIDTH_2:.*]]}
> +// CHECK: ![[LOOP_10]] = metadata !{metadata ![[LOOP_10]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_2:.*]], metadata ![[WIDTH_2:.*]]}
> +// CHECK: ![[LOOP_11]] = metadata !{metadata ![[LOOP_11]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[WIDTH_2:.*]]}
> +// CHECK: ![[LOOP_12]] = metadata !{metadata ![[LOOP_12]], metadata ![[UNROLL_24:.*]], metadata ![[INTERLEAVE_10:.*]], metadata ![[WIDTH_6:.*]]}
> +// CHECK: ![[UNROLL_24]] = metadata !{metadata !"llvm.loop.unroll.count", i32 24}
> +// CHECK: ![[INTERLEAVE_10]] = metadata !{metadata !"llvm.loop.vectorize.unroll", i32 10}
> +// CHECK: ![[WIDTH_6]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 6}
> +// CHECK: ![[LOOP_13]] = metadata !{metadata ![[LOOP_13]], metadata ![[UNROLL_32:.*]], metadata ![[INTERLEAVE_16:.*]], metadata ![[WIDTH_8:.*]]}
> +// CHECK: ![[UNROLL_32]] = metadata !{metadata !"llvm.loop.unroll.count", i32 32}
> +// CHECK: ![[INTERLEAVE_16]] = metadata !{metadata !"llvm.loop.vectorize.unroll", i32 16}
> Index: test/Misc/ast-print-pragmas-xfail.cpp
> ===================================================================
> --- test/Misc/ast-print-pragmas-xfail.cpp (revision 211958)
> +++ test/Misc/ast-print-pragmas-xfail.cpp (working copy)
> @@ -5,15 +5,15 @@
>
>  void run1(int *List, int Length) {
>    int i = 0;
> -// CEHCK: #pragma loop vectorize(4)
> -// CHECK-NEXT: #pragma loop interleave(8)
> -// CHECK-NEXT: #pragma loop vectorize(enable)
> -// CHECK-NEXT: #pragma loop interleave(enable)
> -#pragma loop vectorize(4)
> -#pragma loop interleave(8)
> -#pragma loop vectorize(enable)
> -#pragma loop interleave(enable)
> -// CHECK-NEXT: while (i < Length)
> +// CEHCK: #pragma clang loop vectorize(4)
> +// CHECK-NEXT: #pragma clang loop interleave(8)
> +// CHECK-NEXT: #pragma clang loop vectorize(enable)
> +// CHECK-NEXT: #pragma clang loop interleave(enable)
> +#pragma clang loop vectorize(4)
> +#pragma clang loop interleave(8)
> +#pragma clang loop vectorize(enable)
> +#pragma clang loop interleave(enable)
> +  // CHECK-NEXT: while (i < Length)
>    while (i < Length) {
>      List[i] = i;
>      i++;
> Index: test/Misc/ast-print-pragmas.cpp
> ===================================================================
> --- test/Misc/ast-print-pragmas.cpp (revision 211958)
> +++ test/Misc/ast-print-pragmas.cpp (working copy)
> @@ -38,3 +38,18 @@
>      i++;
>    }
>  }
> +
> +template <int V, int I>
> +void test_nontype_template_param(int *List, int Length) {
> +#pragma clang loop vectorize_width(V) interleave_count(I)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +// CHECK: #pragma clang loop interleave_count(I)
> +// CHECK: #pragma clang loop vectorize_width(V)
> +
> +void test_templates(int *List, int Length) {
> +  test_nontype_template_param<2, 4>(List, Length);
> +}
> Index: test/PCH/pragma-loop.cpp
> ===================================================================
> --- test/PCH/pragma-loop.cpp (revision 211958)
> +++ test/PCH/pragma-loop.cpp (working copy)
> @@ -13,6 +13,8 @@
>  // CHECK: #pragma clang loop unroll(enable)
>  // CHECK: #pragma clang loop interleave(enable)
>  // CHECK: #pragma clang loop vectorize(disable)
> +// CHECK: #pragma clang loop interleave_count(I)
> +// CHECK: #pragma clang loop vectorize_width(V)
>
>  #ifndef HEADER
>  #define HEADER
> @@ -51,6 +53,15 @@
>        i++;
>      }
>    }
> +
> +  template <int V, int I>
> +  inline void run4(int *List, int Length) {
> +#pragma clang loop vectorize_width(V)
> +#pragma clang loop interleave_count(I)
> +    for (int i = 0; i < Length; i++) {
> +      List[i] = i;
> +    }
> +  }
>  };
>
>  #else
> @@ -63,6 +74,7 @@
>    pt.run1(List, 100);
>    pt.run2(List, 100);
>    pt.run3(List, 100);
> +  pt.run4<2, 4>(List, 100);
>  }
>
>  #endif
> Index: test/Parser/pragma-loop.cpp
> ===================================================================
> --- test/Parser/pragma-loop.cpp (revision 211958)
> +++ test/Parser/pragma-loop.cpp (working copy)
> @@ -3,6 +3,72 @@
>  // Note that this puts the expected lines before the directives to work around
>  // limitations in the -verify mode.
>
> +template <int V, int I>
> +void test_nontype_template_param(int *List, int Length) {
> +#pragma clang loop vectorize_width(V) interleave_count(I)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +
> +#pragma clang loop vectorize_width(V + 4) interleave_count(I + 4)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +template <int V>
> +void test_nontype_template_vectorize(int *List, int Length) {
> +  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(V)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +
> +  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(V / 2)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] += i;
> +  }
> +}
> +
> +template <int I>
> +void test_nontype_template_interleave(int *List, int Length) {
> +  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(I)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +
> +  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(2 % I)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +template <char V>
> +void test_nontype_template_char(int *List, int Length) {
> +  /* expected-error {{invalid argument; expected a constant integer expression}} */ #pragma clang loop vectorize_width(V)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +template <bool V>
> +void test_nontype_template_bool(int *List, int Length) {
> +  /* expected-error {{invalid argument; expected a constant integer expression}} */ #pragma clang loop vectorize_width(V)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +template <int V, int I>
> +void test_nontype_template_badarg(int *List, int Length) {
> +  /* expected-error {{invalid argument; expected a constant integer expression}} */ #pragma clang loop vectorize_width(Vec) interleave_count(I)
> +  /* expected-error {{invalid argument; expected a constant integer expression}} */ #pragma clang loop vectorize_width(V) interleave_count(Int)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +void BadFunctionDecl();
> +
>  void test(int *List, int Length) {
>    int i = 0;
>
> @@ -43,6 +109,8 @@
>      VList[j] = List[j];
>    }
>
> +  test_nontype_template_param<4, 8>(List, Length);
> +
>  /* expected-error {{expected '('}} */ #pragma clang loop vectorize
>  /* expected-error {{expected '('}} */ #pragma clang loop interleave
>  /* expected-error {{expected '('}} */ #pragma clang loop unroll
> @@ -65,46 +133,43 @@
>  /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
>  /* expected-warning {{extra tokens at end of '#pragma clang loop'}} */ #pragma clang loop vectorize(enable) ,
>
> -  while (i-4 < Length) {
> -    List[i] = i;
> -  }
> -
>  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(0)
>  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(0)
>  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(0)
> +
> +/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(10 / 0)
> +/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(10 / 5 - 2)
>    while (i-5 < Length) {
>      List[i] = i;
>    }
>
> +test_nontype_template_vectorize<4>(List, Length);
> +/* expected-note {{in instantiation of function template specialization}} */ test_nontype_template_vectorize<-1>(List, Length);
> +test_nontype_template_interleave<8>(List, Length);
> +/* expected-note {{in instantiation of function template specialization}} */ test_nontype_template_interleave<-1>(List, Length);
> +test_nontype_template_char<'A'>(List, Length);
> +test_nontype_template_bool<true>(List, Length);
> +
>  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(3000000000)
>  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(3000000000)
>  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(3000000000)
> -  while (i-6 < Length) {
> -    List[i] = i;
> -  }
>
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(badvalue)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(badvalue)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(badvalue)
> -  while (i-6 < Length) {
> -    List[i] = i;
> -  }
> +/* expected-error {{invalid argument; expected a constant integer expression}} */ #pragma clang loop vectorize_width(badvalue)
> +/* expected-error {{invalid argument; expected a constant integer expression}} */ #pragma clang loop interleave_count(badvalue)
> +/* expected-error {{invalid argument; expected a constant integer expression}} */ #pragma clang loop unroll_count(badvalue)
>
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop vectorize(badidentifier)
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop interleave(badidentifier)
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop unroll(badidentifier)
> -  while (i-7 < Length) {
> -    List[i] = i;
> -  }
>
>  // PR20069 - Loop pragma arguments that are not identifiers or numeric
>  // constants crash FE.
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop vectorize(()
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop interleave(*)
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop unroll(=)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(^)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(/)
> -/* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(==)
> +/* expected-error {{invalid argument; expected a constant integer expression}} */ #pragma clang loop vectorize_width(^)
> +/* expected-error {{invalid argument; expected a constant integer expression}} */ #pragma clang loop interleave_count(/)
> +/* expected-error {{invalid argument; expected a constant integer expression}} */ #pragma clang loop unroll_count(==)
>    while (i-8 < Length) {
>      List[i] = i;
>    }
> Index: utils/TableGen/ClangAttrEmitter.cpp
> ===================================================================
> --- utils/TableGen/ClangAttrEmitter.cpp (revision 211958)
> +++ utils/TableGen/ClangAttrEmitter.cpp (working copy)
> @@ -1606,8 +1606,20 @@
>    }
>  }
>
> +// Determines if an attribute has a Pragma spelling.
> +bool AttrHasPragmaSpelling(Record *R) {
> +  std::vector<FlattenedSpelling> Spellings = GetFlattenedSpellings(*R);
> +  for (const auto &SI : Spellings) {
> +    std::string Variety = SI.variety();
> +    if (Variety == "Pragma") {
> +      return true;
> +    }
> +  }
> +
> +  return false;
> +}
> +
>  namespace clang {
> -
>  // Emits the enumeration list for attributes.
>  void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS) {
>    emitSourceFileHeader("List of all attributes that Clang recognizes", OS);
> @@ -1633,15 +1645,25 @@
>          " INHERITABLE_PARAM_ATTR(NAME)\n";
>    OS << "#endif\n\n";
>
> +  OS << "#ifndef PRAGMA_ATTR\n";
> +  OS << "#define PRAGMA_ATTR(NAME) ATTR(NAME)\n";
> +  OS << "#endif\n\n";
> +
> +  OS << "#ifndef LAST_PRAGMA_ATTR\n";
> +  OS << "#define LAST_PRAGMA_ATTR(NAME) PRAGMA_ATTR(NAME)\n";
> +  OS << "#endif\n\n";
> +
>    Record *InhClass = Records.getClass("InheritableAttr");
>    Record *InhParamClass = Records.getClass("InheritableParamAttr");
> -  std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr"),
> -                       NonInhAttrs, InhAttrs, InhParamAttrs;
> +  std::vector<Record *> Attrs = Records.getAllDerivedDefinitions("Attr"),
> +                        NonInhAttrs, InhAttrs, InhParamAttrs, PragmaAttrs;
>    for (auto *Attr : Attrs) {
>      if (!Attr->getValueAsBit("ASTNode"))
>        continue;
> -
> -    if (Attr->isSubClassOf(InhParamClass))
> +
> +    if (AttrHasPragmaSpelling(Attr))
> +      PragmaAttrs.push_back(Attr);
> +    else if (Attr->isSubClassOf(InhParamClass))
>        InhParamAttrs.push_back(Attr);
>      else if (Attr->isSubClassOf(InhClass))
>        InhAttrs.push_back(Attr);
> @@ -1649,6 +1671,7 @@
>        NonInhAttrs.push_back(Attr);
>    }
>
> +  EmitAttrList(OS, "PRAGMA_ATTR", PragmaAttrs);
>    EmitAttrList(OS, "INHERITABLE_PARAM_ATTR", InhParamAttrs);
>    EmitAttrList(OS, "INHERITABLE_ATTR", InhAttrs);
>    EmitAttrList(OS, "ATTR", NonInhAttrs);
> @@ -1657,6 +1680,8 @@
>    OS << "#undef INHERITABLE_ATTR\n";
>    OS << "#undef LAST_INHERITABLE_ATTR\n";
>    OS << "#undef LAST_INHERITABLE_PARAM_ATTR\n";
> +  OS << "#undef LAST_PRAGMA_ATTR\n";
> +  OS << "#undef PRAGMA_ATTR\n";
>    OS << "#undef ATTR\n";
>  }
>
>

~Aaron

On Fri, Jun 27, 2014 at 5:11 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
> Hi Hal, Aaron, Richard,
>
> This patch adds support for constant expressions, including non-type template parameters, in pragma loop hints.
>
> Support for constant expressions is done using ParseConstantExpression() in the pragma parser, modifying the LoopHintAttr to hold an Expr, and transforming the LoopHintAttr expression during template instantiation.
>
> Previously it was suggested that I also split the LoopHintAttr into an Attr for enable/disable loop hints and an Attr for loop hints with an expression. I gave it a try but it caused a lot of duplication without much of a reduction in the complexity of loop hint handling. I am willing to give it another look if you think it would be better in the long run.
>
> Tyler
>
>




More information about the cfe-commits mailing list