[PATCH] Support constant expressions, including non-type template parameters, in pragma loop hints
Aaron Ballman
aaron at aaronballman.com
Wed Jul 30 07:34:40 PDT 2014
On Tue, Jul 29, 2014 at 6:15 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
> Hi Aaron,
>
> I’ve cut another piece off the constant expression patch. This piece adds
> the state variable to the loop hint attribute. Would you be able tor review
> it?
>
> Parts of this patch may seem a little overkill, they are necessary to
> support constant expressions. Specifically, the way the argument to a loop
> hint pragma is handled. The single token argument is replaced by an array of
> tokens which either give a keyword (single token) or a constant expression
> (multiple tokens). We need to have these cases separated out to pass the
> hint to the SemaStmtAttr handler. Consequently validation of the keywords
> must move from SemaStmtAttr handler to the loop hint annotation token
> handler. It makes the token handler a bit more complicated and the
> SemaStmtAttr a bit simpler.
>
> A couple of new tests are included as well.
I don't see any new tests in the patch file, did those get
accidentally left out?
Comments on the patch below.
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td (revision 214227)
> +++ include/clang/Basic/Attr.td (working copy)
> @@ -1784,13 +1784,19 @@
> }
>
> def LoopHint : Attr {
> - /// vectorize: vectorizes loop operations if 'value != 0'.
> - /// vectorize_width: vectorize loop operations with width 'value'.
> - /// interleave: interleave multiple loop iterations if 'value != 0'.
> - /// interleave_count: interleaves 'value' loop interations.
> - /// unroll: fully unroll loop if 'value != 0'.
> - /// unroll_count: unrolls loop 'value' times.
> + /// #pragma clang loop <option> directive
> + /// vectorize: vectorizes loop operations if State == Enable.
> + /// vectorize_width: vectorize loop operations with width 'Value'.
> + /// interleave: interleave multiple loop iterations if State == Enable.
> + /// interleave_count: interleaves 'Value' loop interations.
> + /// unroll: fully unroll loop if State == Enable.
> + /// unroll_count: unrolls loop 'Value' times.
>
> + /// #pragma unroll <argument> directive
> + /// <no arg>: fully unrolls loop.
> + /// boolean: fully unrolls loop if State == Enable.
> + /// expression: unrolls loop 'Value' times.
> +
> let Spellings = [Pragma<"clang", "loop">, Pragma<"", "unroll">,
> Pragma<"", "nounroll">];
>
> @@ -1800,6 +1806,9 @@
> "unroll", "unroll_count"],
> ["Vectorize", "VectorizeWidth", "Interleave", "InterleaveCount",
> "Unroll", "UnrollCount"]>,
> + EnumArgument<"State", "StateType",
The name StateType isn't particularly descriptive. How about LoopHintState?
> + ["default", "enable", "disable"],
> + ["Default", "Enable", "Disable"]>,
This isn't really a "default" so much as "it's not Enabled or
Disabled", correct? Perhaps UsesValue instead? I know that's not quite
accurate either since it'll be used for #pragma unroll with no
arguments...
> DefaultIntArgument<"Value", 1>];
>
> let AdditionalMembers = [{
> @@ -1841,9 +1850,9 @@
> if (option == VectorizeWidth || option == InterleaveCount ||
> option == UnrollCount)
> OS << value;
> - else if (value < 0)
> + else if (state == Default)
> return "";
> - else if (value > 0)
> + else if (state == Enable)
> OS << (option == Unroll ? "full" : "enable");
> else
> OS << "disable";
> Index: include/clang/Basic/DiagnosticParseKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticParseKinds.td (revision 214227)
> +++ include/clang/Basic/DiagnosticParseKinds.td (working copy)
> @@ -917,10 +917,16 @@
> def err_omp_expected_identifier_for_critical : Error<
> "expected identifier specifying the name of the 'omp critical' directive">;
>
> +// Pragma support.
> +def err_pragma_invalid_keyword : Error<
> + "%select{invalid|missing}0 argument; expected '%select{enable|full}1' or 'disable'">;
> +
> // Pragma loop support.
> def err_pragma_loop_invalid_option : Error<
> "%select{invalid|missing}0 option%select{ %1|}0; expected vectorize, "
> "vectorize_width, interleave, interleave_count, unroll, or unroll_count">;
> +def err_pragma_loop_numeric_value : Error<
> + "invalid argument; expected a positive integer value">;
>
> // Pragma unroll support.
> def warn_pragma_unroll_cuda_value_in_parens : Warning<
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td (revision 214227)
> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
> @@ -541,8 +541,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 '%0' or 'disable'">;
> def err_pragma_loop_compatibility : Error<
> "%select{incompatible|duplicate}0 directives '%1' and '%2'">;
> def err_pragma_loop_precedes_nonloop : Error<
> Index: include/clang/Parse/Parser.h
> ===================================================================
> --- include/clang/Parse/Parser.h (revision 214227)
> +++ include/clang/Parse/Parser.h (working copy)
> @@ -525,7 +525,7 @@
>
> /// \brief Handle the annotation token produced for
> /// #pragma clang loop and #pragma unroll.
> - LoopHint HandlePragmaLoopHint();
> + bool HandlePragmaLoopHint(LoopHint &);
Please name the parameter.
>
> /// 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 214227)
> +++ include/clang/Sema/LoopHint.h (working copy)
> @@ -29,11 +29,15 @@
> // "#pragma unroll" and "#pragma nounroll" cases, this is identical to
> // PragmaNameLoc.
> IdentifierLoc *OptionLoc;
> - // Identifier for the hint argument. If null, then the hint has no argument
> - // such as for "#pragma unroll".
> - IdentifierLoc *ValueLoc;
> + // Identifier for the hint state argument. If null, then the state is
> + // default value such as for "#pragma unroll".
> + IdentifierLoc *StateLoc;
> // Expression for the hint argument if it exists, null otherwise.
> Expr *ValueExpr;
> +
> + LoopHint()
> + : PragmaNameLoc(nullptr), OptionLoc(nullptr), StateLoc(nullptr),
> + ValueExpr(nullptr) {}
> };
>
> } // end namespace clang
> Index: lib/CodeGen/CGStmt.cpp
> ===================================================================
> --- lib/CodeGen/CGStmt.cpp (revision 214227)
> +++ lib/CodeGen/CGStmt.cpp (working copy)
> @@ -588,6 +588,7 @@
> continue;
>
> LoopHintAttr::OptionType Option = LH->getOption();
> + int State = LH->getState();
Declaration shouldn't be an int; should be StateType (or whatever it
gets renamed to).
> int ValueInt = LH->getValue();
>
> const char *MetadataName;
> @@ -602,8 +603,8 @@
> break;
> case LoopHintAttr::Unroll:
> // With the unroll loop hint, a non-zero value indicates full unrolling.
> - MetadataName =
> - ValueInt == 0 ? "llvm.loop.unroll.disable" : "llvm.loop.unroll.full";
> + MetadataName = State == LoopHintAttr::Disable ? "llvm.loop.unroll.disable"
> + : "llvm.loop.unroll.full";
> break;
> case LoopHintAttr::UnrollCount:
> MetadataName = "llvm.loop.unroll.count";
> @@ -614,7 +615,7 @@
> switch (Option) {
> case LoopHintAttr::Vectorize:
> case LoopHintAttr::Interleave:
> - if (ValueInt == 1) {
> + if (State != LoopHintAttr::Disable) {
> // 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");
> Index: lib/Parse/ParsePragma.cpp
> ===================================================================
> --- lib/Parse/ParsePragma.cpp (revision 214227)
> +++ lib/Parse/ParsePragma.cpp (working copy)
> @@ -722,38 +722,62 @@
> bool HasValue;
> };
>
> -LoopHint Parser::HandlePragmaLoopHint() {
> +bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
> assert(Tok.is(tok::annot_pragma_loop_hint));
> PragmaLoopHintInfo *Info =
> static_cast<PragmaLoopHintInfo *>(Tok.getAnnotationValue());
> + ConsumeToken(); // The annotation token.
>
> - LoopHint Hint;
> - Hint.PragmaNameLoc =
> - IdentifierLoc::create(Actions.Context, Info->PragmaName.getLocation(),
> - Info->PragmaName.getIdentifierInfo());
> - Hint.OptionLoc =
> - IdentifierLoc::create(Actions.Context, Info->Option.getLocation(),
> - Info->Option.getIdentifierInfo());
> - if (Info->HasValue) {
> - Hint.Range =
> - SourceRange(Info->Option.getLocation(), Info->Value.getLocation());
> - Hint.ValueLoc =
> - IdentifierLoc::create(Actions.Context, Info->Value.getLocation(),
> - Info->Value.getIdentifierInfo());
> + IdentifierInfo *PragmaNameInfo = Info->PragmaName.getIdentifierInfo();
> + Hint.PragmaNameLoc = IdentifierLoc::create(
> + Actions.Context, Info->PragmaName.getLocation(), PragmaNameInfo);
>
> + IdentifierInfo *OptionInfo = Info->Option.getIdentifierInfo();
> + Hint.OptionLoc = IdentifierLoc::create(
> + Actions.Context, Info->Option.getLocation(), OptionInfo);
> +
> + bool PragmaUnroll = PragmaNameInfo->getName() == "unroll";
> + bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll";
> + if (!Info->HasValue && (PragmaUnroll | PragmaNoUnroll)) {
Please use || here instead of |.
> + Hint.Range = SourceRange(Info->PragmaName.getLocation());
Pretty sure you don't need the explicit constructor call here.
> + return true;
> + }
> +
> + bool StateOption = false;
> + if (OptionInfo) // Pragma unroll does not specify an option.
It doesn't always specify an option, but it sometimes does (otherwise
there wouldn't be a .Case below).
> + StateOption = llvm::StringSwitch<bool>(OptionInfo->getName())
> + .Case("vectorize", true)
> + .Case("interleave", true)
> + .Case("unroll", true)
> + .Default(false);
> +
> + // Validate the argument.
> + if (StateOption) {
> + bool OptionUnroll = OptionInfo->isStr("unroll");
> + SourceLocation StateLoc = Info->Value.getLocation();
> + IdentifierInfo *StateInfo = Info->Value.getIdentifierInfo();
> + if (!StateInfo || ((OptionUnroll ? !StateInfo->isStr("full")
> + : !StateInfo->isStr("enable")) &&
> + !StateInfo->isStr("disable"))) {
> + Diag(StateLoc, diag::err_pragma_invalid_keyword)
> + << /*MissingArgument=*/false << /*FullKeyword=*/OptionUnroll;
> + return false;
> + }
> + Hint.StateLoc = IdentifierLoc::create(Actions.Context, StateLoc, StateInfo);
> + } else {
> // 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;
> - } else {
> - Hint.Range = SourceRange(Info->PragmaName.getLocation());
> - Hint.ValueLoc = nullptr;
> - Hint.ValueExpr = nullptr;
> + else {
> + Diag(Info->Value.getLocation(), diag::err_pragma_loop_numeric_value);
> + return false;
> + }
> }
>
> - return Hint;
> + Hint.Range = SourceRange(Info->PragmaName.getLocation(),
> + Info->Value.getLocation());
> + return true;
> }
>
> // #pragma GCC visibility comes in two variants:
> @@ -1755,12 +1779,10 @@
> }
>
> /// \brief Parses loop or unroll pragma hint value and fills in Info.
> -static bool ParseLoopHintValue(Preprocessor &PP, Token Tok, Token &PragmaName,
> - Token &Option, bool &ValueInParens,
> +static bool ParseLoopHintValue(Preprocessor &PP, Token &Tok, Token PragmaName,
> + Token Option, bool ValueInParens,
> PragmaLoopHintInfo &Info) {
> - ValueInParens = Tok.is(tok::l_paren);
> if (ValueInParens) {
> - PP.Lex(Tok);
> if (Tok.is(tok::r_paren)) {
> // Nothing between the parentheses.
> std::string PragmaString;
> @@ -1784,13 +1806,15 @@
>
> // FIXME: Value should be stored and parsed as a constant expression.
> Token Value = Tok;
> + PP.Lex(Tok);
>
> if (ValueInParens) {
> - PP.Lex(Tok);
> + // Read ')'
> if (Tok.isNot(tok::r_paren)) {
> PP.Diag(Tok.getLocation(), diag::err_expected) << tok::r_paren;
> return true;
> }
> + PP.Lex(Tok);
> }
>
> Info.PragmaName = PragmaName;
> @@ -1872,18 +1896,19 @@
> << /*MissingOption=*/false << OptionInfo;
> return;
> }
> -
> - auto *Info = new (PP.getPreprocessorAllocator()) PragmaLoopHintInfo;
> PP.Lex(Tok);
> - bool ValueInParens;
> - if (ParseLoopHintValue(PP, Tok, PragmaName, Option, ValueInParens, *Info))
> - return;
>
> - if (!ValueInParens) {
> - PP.Diag(Info->Value.getLocation(), diag::err_expected) << tok::l_paren;
> + // Read '('
> + if (Tok.isNot(tok::l_paren)) {
> + PP.Diag(Tok.getLocation(), diag::err_expected) << tok::l_paren;
> return;
> }
> + PP.Lex(Tok);
>
> + auto *Info = new (PP.getPreprocessorAllocator()) PragmaLoopHintInfo;
> + if (ParseLoopHintValue(PP, Tok, PragmaName, Option, /*ValueInParens=*/true, *Info))
> + return;
> +
> // Generate the loop hint token.
> Token LoopHintTok;
> LoopHintTok.startToken();
> @@ -1891,9 +1916,6 @@
> LoopHintTok.setLocation(PragmaName.getLocation());
> LoopHintTok.setAnnotationValue(static_cast<void *>(Info));
> TokenList.push_back(LoopHintTok);
> -
> - // Get next optimization option.
> - PP.Lex(Tok);
> }
>
> if (Tok.isNot(tok::eod)) {
> @@ -1938,7 +1960,6 @@
> if (Tok.is(tok::eod)) {
> // nounroll or unroll pragma without an argument.
> Info->PragmaName = PragmaName;
> - Info->Option = PragmaName;
> Info->HasValue = false;
> } else if (PragmaName.getIdentifierInfo()->getName() == "nounroll") {
> PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
> @@ -1947,9 +1968,12 @@
> } else {
> // Unroll pragma with an argument: "#pragma unroll N" or
> // "#pragma unroll(N)".
> - bool ValueInParens;
> - if (ParseLoopHintValue(PP, Tok, PragmaName, PragmaName, ValueInParens,
> - *Info))
> + // Read '(' if it exists.
> + bool ValueInParens = Tok.is(tok::l_paren);
> + if (ValueInParens)
> + PP.Lex(Tok);
> +
> + if (ParseLoopHintValue(PP, Tok, PragmaName, Token(), ValueInParens, *Info))
> return;
>
> // In CUDA, the argument to '#pragma unroll' should not be contained in
> @@ -1958,7 +1982,6 @@
> PP.Diag(Info->Value.getLocation(),
> diag::warn_pragma_unroll_cuda_value_in_parens);
>
> - PP.Lex(Tok);
> if (Tok.isNot(tok::eod)) {
> PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
> << "unroll";
> Index: lib/Parse/ParseStmt.cpp
> ===================================================================
> --- lib/Parse/ParseStmt.cpp (revision 214227)
> +++ lib/Parse/ParseStmt.cpp (working copy)
> @@ -1817,10 +1817,11 @@
>
> // Get loop hints and consume annotated token.
> while (Tok.is(tok::annot_pragma_loop_hint)) {
> - LoopHint Hint = HandlePragmaLoopHint();
> - ConsumeToken();
> + LoopHint Hint;
> + if (!HandlePragmaLoopHint(Hint))
> + continue;
>
> - ArgsUnion ArgHints[] = {Hint.PragmaNameLoc, Hint.OptionLoc, Hint.ValueLoc,
> + ArgsUnion ArgHints[] = {Hint.PragmaNameLoc, Hint.OptionLoc, Hint.StateLoc,
> ArgsUnion(Hint.ValueExpr)};
> TempAttrs.addNew(Hint.PragmaNameLoc->Ident, Hint.Range, nullptr,
> Hint.PragmaNameLoc->Loc, ArgHints, 4,
> Index: lib/Sema/SemaStmtAttr.cpp
> ===================================================================
> --- lib/Sema/SemaStmtAttr.cpp (revision 214227)
> +++ lib/Sema/SemaStmtAttr.cpp (working copy)
> @@ -47,13 +47,11 @@
> SourceRange) {
> IdentifierLoc *PragmaNameLoc = A.getArgAsIdent(0);
> IdentifierLoc *OptionLoc = A.getArgAsIdent(1);
> - IdentifierInfo *OptionInfo = OptionLoc->Ident;
> - IdentifierLoc *ValueLoc = A.getArgAsIdent(2);
> - IdentifierInfo *ValueInfo = ValueLoc ? ValueLoc->Ident : nullptr;
> + IdentifierLoc *StateLoc = A.getArgAsIdent(2);
> Expr *ValueExpr = A.getArgAsExpr(3);
>
> - assert(OptionInfo && "Attribute must have valid option info.");
> -
> + bool PragmaUnroll = PragmaNameLoc->Ident->getName() == "unroll";
> + bool PragmaNoUnroll = PragmaNameLoc->Ident->getName() == "nounroll";
> if (St->getStmtClass() != Stmt::DoStmtClass &&
> St->getStmtClass() != Stmt::ForStmtClass &&
> St->getStmtClass() != Stmt::CXXForRangeStmtClass &&
> @@ -69,13 +67,16 @@
>
> LoopHintAttr::OptionType Option;
> LoopHintAttr::Spelling Spelling;
> - if (PragmaNameLoc->Ident->getName() == "unroll") {
> - Option = ValueLoc ? LoopHintAttr::UnrollCount : LoopHintAttr::Unroll;
> + if (PragmaUnroll) {
> + Option = ValueExpr ? LoopHintAttr::UnrollCount : LoopHintAttr::Unroll;
> Spelling = LoopHintAttr::Pragma_unroll;
> - } else if (PragmaNameLoc->Ident->getName() == "nounroll") {
> + } else if (PragmaNoUnroll) {
> Option = LoopHintAttr::Unroll;
> Spelling = LoopHintAttr::Pragma_nounroll;
> } else {
> + assert(OptionLoc && OptionLoc->Ident &&
> + "Attribute must have valid option info.");
> + IdentifierInfo *OptionInfo = OptionLoc->Ident;
> Option = llvm::StringSwitch<LoopHintAttr::OptionType>(OptionInfo->getName())
> .Case("vectorize", LoopHintAttr::Vectorize)
> .Case("vectorize_width", LoopHintAttr::VectorizeWidth)
> @@ -87,35 +88,10 @@
> Spelling = LoopHintAttr::Pragma_clang_loop;
> }
>
> - int ValueInt = -1;
> - if (Option == LoopHintAttr::Unroll &&
> - Spelling == LoopHintAttr::Pragma_unroll) {
> - if (ValueInfo)
> - ValueInt = (ValueInfo->isStr("disable") ? 0 : 1);
> - } else if (Option == LoopHintAttr::Unroll &&
> - Spelling == LoopHintAttr::Pragma_nounroll) {
> - ValueInt = 0;
> - } else if (Option == LoopHintAttr::Vectorize ||
> - Option == LoopHintAttr::Interleave ||
> - Option == LoopHintAttr::Unroll) {
> - // Unrolling uses the keyword "full" rather than "enable" to indicate full
> - // unrolling.
> - const char *TrueKeyword =
> - Option == LoopHintAttr::Unroll ? "full" : "enable";
> - if (!ValueInfo) {
> - S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword)
> - << TrueKeyword;
> - return nullptr;
> - }
> - if (ValueInfo->isStr("disable"))
> - ValueInt = 0;
> - else if (ValueInfo->getName() == TrueKeyword)
> - ValueInt = 1;
> - else {
> - S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword)
> - << TrueKeyword;
> - return nullptr;
> - }
> + int ValueInt = 1;
> + LoopHintAttr::StateType State = LoopHintAttr::Default;
> + if (PragmaNoUnroll) {
> + State = LoopHintAttr::Disable;
> } else if (Option == LoopHintAttr::VectorizeWidth ||
> Option == LoopHintAttr::InterleaveCount ||
> Option == LoopHintAttr::UnrollCount) {
> @@ -124,28 +100,39 @@
> llvm::APSInt ValueAPS;
> if (!ValueExpr || !ValueExpr->isIntegerConstantExpr(ValueAPS, S.Context) ||
> (ValueInt = ValueAPS.getSExtValue()) < 1) {
> - S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_value);
> + S.Diag(A.getRange().getEnd(), diag::err_pragma_loop_invalid_value);
A.getLoc() instead of A.getRange().getEnd()?
> return nullptr;
> }
> - } else
> - llvm_unreachable("Unknown loop hint option");
> + } else if (Option == LoopHintAttr::Vectorize ||
> + Option == LoopHintAttr::Interleave ||
> + Option == LoopHintAttr::Unroll) {
> + // Default state is assumed if StateLoc is not specified, such as with
> + // '#pragma unroll'.
> + if (StateLoc && StateLoc->Ident) {
> + if (StateLoc->Ident->isStr("disable"))
> + State = LoopHintAttr::Disable;
> + else
> + State = LoopHintAttr::Enable;
> + }
> + }
>
> - return LoopHintAttr::CreateImplicit(S.Context, Spelling, Option, ValueInt,
> - A.getRange());
> + return LoopHintAttr::CreateImplicit(S.Context, Spelling, Option, State,
> + ValueInt, A.getRange());
> }
>
> -static void CheckForIncompatibleAttributes(
> - Sema &S, const SmallVectorImpl<const Attr *> &Attrs) {
> - // There are 3 categories of loop hints attributes: vectorize, interleave, and
> - // unroll. Each comes in two variants: a boolean form and a numeric form. The
> - // boolean hints selectively enables/disables the transformation for the loop
> - // (for unroll, a nonzero value indicates full unrolling rather than enabling
> - // the transformation). The numeric hint provides an integer hint (for
> - // example, unroll count) to the transformer. The following array accumulates
> - // the hints encountered while iterating through the attributes to check for
> - // compatibility.
> +static void
> +CheckForIncompatibleAttributes(Sema &S,
> + const SmallVectorImpl<const Attr *> &Attrs) {
> + // There are 3 categories of loop hints attributes: vectorize, interleave,
> + // and unroll. Each comes in two variants: a state form and a numeric form.
> + // The state form selectively defaults/enables/disables the transformation
> + // for the loop (for unroll, default indicates full unrolling rather than
> + // enabling the transformation). The numeric form form provides an integer
> + // hint (for example, unroll count) to the transformer. The following array
> + // accumulates the hints encountered while iterating through the attributes
> + // to check for compatibility.
> struct {
> - const LoopHintAttr *EnableAttr;
> + const LoopHintAttr *StateAttr;
> const LoopHintAttr *NumericAttr;
> } HintAttrs[] = {{nullptr, nullptr}, {nullptr, nullptr}, {nullptr, nullptr}};
>
> @@ -179,8 +166,8 @@
> if (Option == LoopHintAttr::Vectorize ||
> Option == LoopHintAttr::Interleave || Option == LoopHintAttr::Unroll) {
> // Enable|disable hint. For example, vectorize(enable).
> - PrevAttr = CategoryState.EnableAttr;
> - CategoryState.EnableAttr = LH;
> + PrevAttr = CategoryState.StateAttr;
> + CategoryState.StateAttr = LH;
> } else {
> // Numeric hint. For example, vectorize_width(8).
> PrevAttr = CategoryState.NumericAttr;
> @@ -195,14 +182,15 @@
> << /*Duplicate=*/true << PrevAttr->getDiagnosticName(Policy)
> << LH->getDiagnosticName(Policy);
>
> - if (CategoryState.EnableAttr && CategoryState.NumericAttr &&
> - (Category == Unroll || !CategoryState.EnableAttr->getValue())) {
> + if (CategoryState.StateAttr && CategoryState.NumericAttr &&
> + (Category == Unroll ||
> + CategoryState.StateAttr->getState() == LoopHintAttr::Disable)) {
> // Disable hints are not compatible with numeric hints of the same
> // category. As a special case, numeric unroll hints are also not
> // compatible with "enable" form of the unroll pragma, unroll(full).
> S.Diag(OptionLoc, diag::err_pragma_loop_compatibility)
> << /*Duplicate=*/false
> - << CategoryState.EnableAttr->getDiagnosticName(Policy)
> + << CategoryState.StateAttr->getDiagnosticName(Policy)
> << CategoryState.NumericAttr->getDiagnosticName(Policy);
> }
> }
>
~Aaron
More information about the cfe-commits
mailing list