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

Aaron Ballman aaron.ballman at gmail.com
Mon Jul 14 08:36:21 PDT 2014


On Wed, Jul 9, 2014 at 10:35 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
> Sorry I generated the patch incorrectly. Here is the correct patch.
>
> I also fixed the parsing off-the-end bug using the EoF terminator that was
> suggestsd before and added test for it.
>
> Let me know what you think,

Sorry about the delayed review; comments below.

> Index: include/clang/AST/Attr.h
> ===================================================================
> --- include/clang/AST/Attr.h (revision 212669)
> +++ 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 212669)
> +++ include/clang/Basic/Attr.td (working copy)
> @@ -1765,11 +1765,11 @@
>  }
>
>  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">];
> @@ -1780,7 +1780,8 @@
>                             "unroll", "unroll_count"],
>                            ["Vectorize", "VectorizeWidth", "Interleave", "InterleaveCount",
>                             "Unroll", "UnrollCount"]>,
> -              DefaultIntArgument<"Value", 1>];
> +              DefaultBoolArgument<"Enabled", 0>,
> +              ExprArgument<"Value">];
>
>    let AdditionalMembers = [{
>    static StringRef getOptionName(int Option) {
> @@ -1795,19 +1796,17 @@
>      llvm_unreachable("Unhandled LoopHint option.");
>    }
>
> -  static StringRef getValueName(int Value) {
> -    if (Value)
> -      return "enable";
> -    return "disable";
> +  static StringRef 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;
> +      value->printPretty(OS, 0, Policy);
>      else
> -      OS << getValueName(value);
> +      OS << getEnabledName(enabled);
>      OS << ")\n";
>    }
>    }];
> Index: include/clang/Basic/Diagnostic.h
> ===================================================================
> --- include/clang/Basic/Diagnostic.h (revision 212669)
> +++ include/clang/Basic/Diagnostic.h (working copy)
> @@ -18,10 +18,12 @@
>  #include "clang/Basic/DiagnosticIDs.h"
>  #include "clang/Basic/DiagnosticOptions.h"
>  #include "clang/Basic/SourceLocation.h"
> +#include "llvm/ADT/APInt.h"
>  #include "llvm/ADT/ArrayRef.h"
>  #include "llvm/ADT/DenseMap.h"
>  #include "llvm/ADT/IntrusiveRefCntPtr.h"
>  #include "llvm/ADT/iterator_range.h"
> +#include "llvm/Support/raw_ostream.h"
>  #include <list>
>  #include <vector>
>
> @@ -1059,6 +1061,15 @@
>    return DB;
>  }
>
> +inline const DiagnosticBuilder &operator<<(const DiagnosticBuilder &DB,
> +                                           const llvm::APInt &I) {
> +  std::string Value;
> +  llvm::raw_string_ostream OS(Value);
> +  OS << '\'' << I << '\'';
> +  DB.AddString(OS.str());
> +  return DB;
> +}
> +

I don't think this is a good API for the diagnostic builder -- an
APInt is a pretty generic datatype, and I don't think it makes sense
to force all callers to have it quoted. What's more, this doesn't give
callers the chance to interpret the APInt as signed or unsigned (it is
always signed with this code), or use a specific base to print as.

I would have callers determine the proper way to print APInt objects.
If needed, you could make the diagnostic builder know how to handle
64-bit values (this would also be used to correct other callers of the
diagnostic engine where APInt::getZExtValue() and friends get cast to
unsigned/signed). If you go this route, I think that should be a
separate patch.

>  // Adds a DeclContext to the diagnostic. The enable_if template magic is here
>  // so that we only match those arguments that are (statically) DeclContexts;
>  // other arguments that derive from DeclContext (e.g., RecordDecls) will not
> Index: include/clang/Basic/DiagnosticGroups.td
> ===================================================================
> --- include/clang/Basic/DiagnosticGroups.td (revision 212669)
> +++ 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 212669)
> +++ include/clang/Basic/DiagnosticParseKinds.td (working copy)
> @@ -913,6 +913,8 @@
>    "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_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 212669)
> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
> @@ -542,10 +542,10 @@
>    "#pragma visibility pop with no matching #pragma visibility push">;
>  def note_surrounding_namespace_starts_here : Note<
>    "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_invalid_argument : Error<
> +  "invalid argument %0; expected a positive integer value">;
> +def err_pragma_loop_invalid_expression : Error<
> +  "invalid expression; expected an integer constant expression">;
>  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/Parse/Parser.h
> ===================================================================
> --- include/clang/Parse/Parser.h (revision 212669)
> +++ include/clang/Parse/Parser.h (working copy)
> @@ -522,8 +522,8 @@
>    StmtResult HandlePragmaCaptured();
>
>    /// \brief Handle the annotation token produced for
> -  /// #pragma vectorize...
> -  LoopHint HandlePragmaLoopHint();
> +  /// #pragma clang loop...
> +  bool HandlePragmaLoopHint(LoopHint &Hint);
>
>    /// GetLookAheadToken - This peeks ahead N tokens and returns that token
>    /// without consuming any tokens.  LookAhead(0) returns 'Tok', LookAhead(1)
> Index: include/clang/Sema/LoopHint.h
> ===================================================================
> --- include/clang/Sema/LoopHint.h (revision 212669)
> +++ 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 212669)
> +++ include/clang/Sema/Sema.h (working copy)
> @@ -3442,6 +3442,9 @@
>                                   PredefinedExpr::IdentType IT);
>    ExprResult ActOnPredefinedExpr(SourceLocation Loc, tok::TokenKind Kind);
>    ExprResult ActOnIntegerConstant(SourceLocation Loc, uint64_t Val);
> +
> +  bool CheckLoopHintExpr(Expr *E, SourceLocation Loc, bool AllowValueDependent);
> +
>    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 212669)
> +++ lib/CodeGen/CGStmt.cpp (working copy)
> @@ -559,7 +559,6 @@
>        continue;
>
>      LoopHintAttr::OptionType Option = LH->getOption();
> -    int ValueInt = LH->getValue();
>
>      const char *MetadataName;
>      switch (Option) {
> @@ -579,18 +578,30 @@
>        break;
>      }
>
> +    bool Enabled = LH->getEnabled();
> +    Expr *ValueExpr = LH->getValue();
> +    int ValueInt = 1;
> +
> +    if (ValueExpr) {
> +      llvm::APSInt ValueAPS =
> +          ValueExpr->EvaluateKnownConstInt(CGM.getContext());
> +      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.
> @@ -601,7 +612,7 @@
>        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);
> Index: lib/Parse/ParsePragma.cpp
> ===================================================================
> --- lib/Parse/ParsePragma.cpp (revision 212669)
> +++ lib/Parse/ParsePragma.cpp (working copy)
> @@ -601,36 +601,68 @@
>
>  struct PragmaLoopHintInfo {
>    Token Loop;
> -  Token Value;
>    Token Option;
> +  Token *Args;
> +  size_t ArgSize;
>  };
>
> -LoopHint Parser::HandlePragmaLoopHint() {
> +bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
>    assert(Tok.is(tok::annot_pragma_loop_hint));
>    PragmaLoopHintInfo *Info =
>        static_cast<PragmaLoopHintInfo *>(Tok.getAnnotationValue());
>
> -  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->Args;
> +  size_t ArgSize = Info->ArgSize;
> +  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);
>
> -  return Hint;
> +  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 false;
> +    }
> +
> +    Hint.EnabledLoc =
> +        IdentifierLoc::create(Actions.Context, EnabledLoc, EnabledInfo);
> +  } else {
> +    PP.EnterTokenStream(Args, ArgSize, /*DisableMacroExpansion=*/false,
> +                        /*OwnsTokens=*/false);
> +
> +    ConsumeToken(); // The annotation token.
> +
> +    ExprResult R = ParseConstantExpression();
> +
> +    TryConsumeToken(tok::eof); //Consume the constant expression terminator.

Why is the result of this call being ignored? Also, should have a
space after the //.

> +
> +    if (Actions.CheckLoopHintExpr(R.get(), Args[0].getLocation(),
> +                                  /*AllowValueDependent=*/true))
> +      return false;
> +
> +    // Argument is a constant expression with an integer type.
> +    Hint.ValueExpr = R.get();
> +  }
> +
> +  return true;
>  }
>
>  // #pragma GCC visibility comes in two variants:
> @@ -1707,19 +1739,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 +1765,19 @@
>      auto *Info = new (PP.getPreprocessorAllocator()) PragmaLoopHintInfo;
>      Info->Loop = Loop;
>      Info->Option = Option;
> -    Info->Value = Value;
>
> +    Token EoF;
> +    EoF.startToken();
> +    EoF.setKind(tok::eof);
> +    EoF.setLocation(Loop.getLocation());
> +    ValueList.push_back(EoF); // Terminates expression for parsing.
> +
> +    Token *TokenArray = (Token *)PP.getPreprocessorAllocator().Allocate(
> +        ValueList.size() * sizeof(Token), llvm::alignOf<Token>());
> +    std::copy(ValueList.begin(), ValueList.end(), TokenArray);
> +    Info->Args = TokenArray;
> +    Info->ArgSize = ValueList.size();
> +
>      // Generate the vectorization hint token.
>      Token LoopHintTok;
>      LoopHintTok.startToken();
> Index: lib/Parse/ParseStmt.cpp
> ===================================================================
> --- lib/Parse/ParseStmt.cpp (revision 212669)
> +++ lib/Parse/ParseStmt.cpp (working copy)
> @@ -1822,13 +1822,12 @@
>
>    // Get vectorize hints and consume annotated token.
>    while (Tok.is(tok::annot_pragma_loop_hint)) {
> -    LoopHint Hint = HandlePragmaLoopHint();
> -    ConsumeToken();
> +    LoopHint Hint;
>
> -    if (!Hint.LoopLoc || !Hint.OptionLoc || !Hint.ValueLoc)
> +    if (!HandlePragmaLoopHint(Hint))
>        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 212669)
> +++ lib/Sema/SemaExpr.cpp (working copy)
> @@ -3046,6 +3046,41 @@
>    return FloatingLiteral::Create(S.Context, Val, isExact, Ty, Loc);
>  }
>
> +bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc,
> +                             bool AllowValueDependent) {
> +  if (!E) {
> +    Diag(Loc, diag::err_pragma_loop_invalid_expression);
> +    return true;
> +  }
> +
> +  if (E->isValueDependent()) {
> +    if (AllowValueDependent)
> +      return false;
> +    Diag(E->getExprLoc(), diag::err_pragma_loop_invalid_expression);
> +    return true;
> +  }
> +
> +  QualType QT = E->getType();
> +  if (!QT->isIntegerType() || QT->isBooleanType() || QT->isCharType()) {
> +    Diag(E->getExprLoc(), diag::err_pragma_loop_invalid_argument) << QT;
> +    return true;
> +  }
> +
> +  llvm::APSInt ValueAPS;
> +  ExprResult R = VerifyIntegerConstantExpression(
> +      E, &ValueAPS, diag::err_pragma_loop_invalid_expression);
> +
> +  if (!R.isUsable())
> +    return true;
> +
> +  if (!ValueAPS.isStrictlyPositive() || ValueAPS.getBitWidth() > 32) {
> +    Diag(E->getExprLoc(), diag::err_pragma_loop_invalid_argument) << ValueAPS;
> +    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 212669)
> +++ 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,24 @@
>            .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.CheckLoopHintExpr(ValueExpr, St->getLocStart(),
> +                            /*AllowAllowValueDependent=*/true))
>        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 +107,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 +123,6 @@
>        continue;
>
>      int Option = LH->getOption();
> -    int ValueInt = LH->getValue();
> -
>      int Category;
>      switch (Option) {
>      case LoopHintAttr::Vectorize:
> @@ -154,30 +141,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 +178,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 212669)
> +++ 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) {
> +  Expr *TransformedExpr = getDerived().TransformExpr(LH->getValue()).get();
> +
> +  if (TransformedExpr == LH->getValue())
> +    return LH;
> +
> +  // Generate error if there is a problem with the value.
> +  if (getSema().CheckLoopHintExpr(TransformedExpr, LH->getLocation(),
> +                                  /*AllowValueDependent=*/false))
> +    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 212669)
> +++ 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.

Comment does not match the code.

> +#define ATTR(X)
> +#define PRAGMA_ATTR(X)                                                         \
> +  case attr::X:                                                                \
> +    return getDerived().Transform##X##Attr(cast<X##Attr>(R));
> +#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 auto *I?

> +    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 212669)
> +++ test/CodeGen/pragma-loop.cpp (working copy)
> @@ -28,11 +28,15 @@
>    } while (i < Length);
>  }
>
> +// Test for scoped enumerations.
> +enum Tuner { Interleave = 4,
> +             Unroll = 8 };
> +
>  // Verify for loop is recognized after sequence of pragma clang loop directives.
>  void for_test(int *List, int Length) {
>  #pragma clang loop interleave(enable)
> -#pragma clang loop interleave_count(4)
> -#pragma clang loop unroll_count(8)
> +#pragma clang loop interleave_count(Tuner::Interleave)
> +#pragma clang loop unroll_count(Tuner::Unroll)
>    for (int i = 0; i < Length; i++) {
>      // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_3:.*]]
>      List[i] = i * 2;
> @@ -74,28 +78,68 @@
>    }
>  }
>
> +// Verify constant expressions are handled correctly.
> +void for_contant_expression_test(int *List, int Length) {
> +#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) {
> -
>  #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;
>    }
>  }
>
>  // Verify define is resolved correctly when template is used.
> -template <typename A>
> +template <typename A, typename T>
>  void for_template_define_test(A *List, int Length, A Value) {
> -#pragma clang loop vectorize_width(VECWIDTH) interleave_count(INTCOUNT)
> -#pragma clang loop unroll_count(UNROLLCOUNT)
> +  const T VWidth = VECWIDTH;
> +  const T ICount = INTCOUNT;
> +  const T UCount = UNROLLCOUNT;
> +#pragma clang loop vectorize_width(VWidth) interleave_count(ICount)
> +#pragma clang loop unroll_count(UCount)
>    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) {
> +#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
> @@ -105,7 +149,8 @@
>    double Value = 10;
>
>    for_template_test<double>(List, Length, Value);
> -  for_template_define_test<double>(List, Length, Value);
> +  for_template_define_test<double, int>(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 +169,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.cpp
> ===================================================================
> --- test/Misc/ast-print-pragmas.cpp (revision 212669)
> +++ 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 212669)
> +++ 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 212669)
> +++ test/Parser/pragma-loop.cpp (working copy)
> @@ -3,6 +3,81 @@
>  // 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 '-1'; 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 '0'; 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 '-1'; 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 '0'; 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 'char'; expected a positive integer value}} */ #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 'bool'; expected a positive integer value}} */ #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 expression; expected an integer constant expression}} expected-error {{use of undeclared identifier 'Vec'}} */ #pragma clang loop vectorize_width(Vec) interleave_count(I)
> +  /* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{use of undeclared identifier 'Int'}} */ #pragma clang loop vectorize_width(V) interleave_count(Int)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +template <typename T>
> +void test_type_template_vectorize(int *List, int Length) {
> +  const T Value = -1;
> +  /* expected-error {{invalid argument '-1'; expected a positive integer value}} */ #pragma clang loop vectorize_width(Value)
> +  for (int i = 0; i < Length; i++) {
> +    List[i] = i;
> +  }
> +}
> +
> +void BadFunctionDecl();
> +
>  void test(int *List, int Length) {
>    int i = 0;
>
> @@ -43,6 +118,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
> @@ -64,28 +141,42 @@
>  /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
>  /* 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 '0'; expected a positive integer value}} */ #pragma clang loop vectorize_width(0)
> +/* expected-error {{invalid argument '0'; expected a positive integer value}} */ #pragma clang loop interleave_count(0)
> +/* expected-error {{invalid argument '0'; expected a positive integer value}} */ #pragma clang loop unroll_count(0)
> +
> +/* expected-error {{invalid expression; expected an integer constant expression}} */ /* expected-note {{division by zero}} */ #pragma clang loop vectorize_width(10 / 0)
> +/* expected-error {{invalid argument '0'; expected a positive integer value}} */ #pragma clang loop interleave_count(10 / 5 - 2)
>    while (i-5 < Length) {
>      List[i] = i;
>    }
>
> -/* 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)
> +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);
> +
> +/* expected-note {{in instantiation of function template specialization}} */ test_nontype_template_char<'A'>(List, Length); // Loop hint arg cannot be a char.
> +/* expected-note {{in instantiation of function template specialization}} */ test_nontype_template_bool<true>(List, Length);  // Or a bool.
> +/* expected-note {{in instantiation of function template specialization}} */ test_type_template_vectorize<int>(List, Length); // Or a template type.
> +
> +/* expected-error {{invalid argument '3000000000'; expected a positive integer value}} */ #pragma clang loop vectorize_width(3000000000)
> +/* expected-error {{invalid argument '3000000000'; expected a positive integer value}} */ #pragma clang loop interleave_count(3000000000)
> +/* expected-error {{invalid argument '3000000000'; 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)
> +/* expected-warning {{extra tokens at end of '#pragma clang loop'}} */ #pragma clang loop vectorize_width(1 +) 1
> +/* expected-warning {{extra tokens at end of '#pragma clang loop'}} */ #pragma clang loop vectorize_width(1) +1
> +
> +/* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop vectorize_width(badvalue)
> +/* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop interleave_count(badvalue)
> +/* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop unroll_count(badvalue)
>    while (i-6 < Length) {
>      List[i] = i;
>    }
> @@ -102,9 +193,9 @@
>  /* 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 expression; expected an integer constant expression}} expected-error {{type name requires a specifier or qualifier}} expected-error {{expected expression}} */ #pragma clang loop vectorize_width(^)
> +/* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{expected expression}} expected-error {{expected expression}} */ #pragma clang loop interleave_count(/)
> +/* expected-error {{invalid expression; expected an integer constant expression}} expected-error {{expected expression}} expected-error {{expected expression}} */ #pragma clang loop unroll_count(==)
>    while (i-8 < Length) {
>      List[i] = i;
>    }
> Index: utils/TableGen/ClangAttrEmitter.cpp
> ===================================================================
> --- utils/TableGen/ClangAttrEmitter.cpp (revision 212669)
> +++ utils/TableGen/ClangAttrEmitter.cpp (working copy)
> @@ -1606,8 +1606,20 @@
>    }
>  }
>
> +// Determines if an attribute has a Pragma spelling.
> +bool AttrHasPragmaSpelling(Record *R) {

Should be a const Record *, and the function should be static.

> +  std::vector<FlattenedSpelling> Spellings = GetFlattenedSpellings(*R);
> +  for (const auto &SI : Spellings) {
> +    std::string Variety = SI.variety();
> +    if (Variety == "Pragma") {
> +      return true;
> +    }
> +  }
> +
> +  return false;

Implementation could be replaced with stock STL functionality:

  return std::find_if(Spellings.begin(), Spellings.end(),
                      [](const FlattenedSpelling &S) {
           return S.variety() == "Pragma";
         }) != Spellings.end();

> +}
> +
>  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))

This isn't correct -- it should be possible to have an attribute with
a pragma spelling and allow it to be an inheritable/noninheritable
attribute at the same time. The spelling should not affect the type
category for the attribute.

>        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



More information about the cfe-commits mailing list