Thanks, I don’t have valgrind or a windows machine. Let me know if the resubmit works for you.
Tyler On Jul 30, 2014, at 7:45 PM, NAKAMURA Takumi <[email protected]> wrote: > Tyler, excuse me, reverted in r214376. > > Valgrind complains as below. > > $ bin/llvm-lit -v --vg /path/to/clang/test/Parser/pragma-unroll.cpp > > ==13928== Conditional jump or move depends on uninitialised value(s) > ==13928== at 0x22AFD62: clang::Token::getIdentifierInfo() const > (Token.h:161) > ==13928== by 0x302BF54: > clang::Parser::HandlePragmaLoopHint(clang::LoopHint&) > (ParsePragma.cpp:735) > ==13928== by 0x3038F22: > clang::Parser::ParsePragmaLoopHint(llvm::SmallVector<clang::Stmt*, > 32u>&, bool, clang::SourceLocation*, > clang::Parser::ParsedAttributesWithRange&) (ParseStmt.cpp:1821) > ==13928== by 0x3033DB1: > clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*, > 32u>&, bool, clang::SourceLocation*, > clang::Parser::ParsedAttributesWithRange&) (ParseStmt.cpp:362) > ==13928== by 0x3032FF7: > clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*, > 32u>&, bool, clang::SourceLocation*) (ParseStmt.cpp:107) > ==13928== by 0x3035DFC: > clang::Parser::ParseCompoundStatementBody(bool) (ParseStmt.cpp:938) > ==13928== by 0x30391C3: > clang::Parser::ParseFunctionStatementBody(clang::Decl*, > clang::Parser::ParseScope&) (ParseStmt.cpp:1857) > ==13928== by 0x2FCBB5D: > clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, > clang::Parser::ParsedTemplateInfo const&, > clang::Parser::LateParsedAttrList*) (Parser.cpp:1098) > ==13928== by 0x2FDCB47: > clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, unsigned int, > bool, clang::SourceLocation*, clang::Parser::ForRangeInit*) > (ParseDecl.cpp:1588) > ==13928== by 0x2FCADAD: > clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, > clang::ParsingDeclSpec&, clang::AccessSpecifier) (Parser.cpp:888) > ==13928== by 0x2FCAE91: > clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, > clang::ParsingDeclSpec*, clang::AccessSpecifier) (Parser.cpp:904) > ==13928== by 0x2FCA5C6: > clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, > clang::ParsingDeclSpec*) (Parser.cpp:762) > > Also, a msvc builder complains. > http://bb.pgr.jp/builders/ninja-clang-i686-msc17-R/builds/9657 > > 2014-07-31 5:54 GMT+09:00 Tyler Nowicki <[email protected]>: >> Author: tnowicki >> Date: Wed Jul 30 15:54:33 2014 >> New Revision: 214333 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=214333&view=rev >> Log: >> Add a state variable to the loop hint attribute. >> >> This patch is necessary to support constant expressions which replaces the >> integer value in the loop hint attribute with an expression. The integer >> value was also storing the pragma’s state for options like >> vectorize(enable/disable) and the pragma unroll and nounroll directive. The >> state variable is introduced to hold the state of those options/pragmas. >> This moves the validation of the state (keywords) from SemaStmtAttr handler >> to the loop hint annotation token handler. >> >> Reviewed by Aaron Ballman >> >> Modified: >> cfe/trunk/include/clang/Basic/Attr.td >> cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> cfe/trunk/include/clang/Parse/Parser.h >> cfe/trunk/include/clang/Sema/LoopHint.h >> cfe/trunk/lib/CodeGen/CGStmt.cpp >> cfe/trunk/lib/Parse/ParsePragma.cpp >> cfe/trunk/lib/Parse/ParseStmt.cpp >> cfe/trunk/lib/Sema/SemaStmtAttr.cpp >> cfe/trunk/test/Parser/pragma-loop.cpp >> >> Modified: cfe/trunk/include/clang/Basic/Attr.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=214333&r1=214332&r2=214333&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/Attr.td (original) >> +++ cfe/trunk/include/clang/Basic/Attr.td Wed Jul 30 15:54:33 2014 >> @@ -1784,12 +1784,18 @@ def Unaligned : IgnoredAttr { >> } >> >> 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 @@ def LoopHint : Attr { >> "unroll", "unroll_count"], >> ["Vectorize", "VectorizeWidth", "Interleave", >> "InterleaveCount", >> "Unroll", "UnrollCount"]>, >> + EnumArgument<"State", "LoopHintState", >> + ["default", "enable", "disable"], >> + ["Default", "Enable", "Disable"]>, >> DefaultIntArgument<"Value", 1>]; >> >> let AdditionalMembers = [{ >> @@ -1841,9 +1850,9 @@ def LoopHint : Attr { >> 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"; >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=214333&r1=214332&r2=214333&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Wed Jul 30 >> 15:54:33 2014 >> @@ -917,10 +917,16 @@ def err_omp_immediate_directive : Error< >> 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< >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=214333&r1=214332&r2=214333&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jul 30 15:54:33 >> 2014 >> @@ -541,8 +541,6 @@ def note_surrounding_namespace_starts_he >> "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< >> >> Modified: cfe/trunk/include/clang/Parse/Parser.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=214333&r1=214332&r2=214333&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Parse/Parser.h (original) >> +++ cfe/trunk/include/clang/Parse/Parser.h Wed Jul 30 15:54:33 2014 >> @@ -525,7 +525,7 @@ private: >> >> /// \brief Handle the annotation token produced for >> /// #pragma clang loop and #pragma unroll. >> - LoopHint HandlePragmaLoopHint(); >> + bool HandlePragmaLoopHint(LoopHint &Hint); >> >> /// GetLookAheadToken - This peeks ahead N tokens and returns that token >> /// without consuming any tokens. LookAhead(0) returns 'Tok', LookAhead(1) >> >> Modified: cfe/trunk/include/clang/Sema/LoopHint.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/LoopHint.h?rev=214333&r1=214332&r2=214333&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Sema/LoopHint.h (original) >> +++ cfe/trunk/include/clang/Sema/LoopHint.h Wed Jul 30 15:54:33 2014 >> @@ -29,11 +29,15 @@ struct LoopHint { >> // "#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 >> >> Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=214333&r1=214332&r2=214333&view=diff >> ============================================================================== >> --- cfe/trunk/lib/CodeGen/CGStmt.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGStmt.cpp Wed Jul 30 15:54:33 2014 >> @@ -588,6 +588,7 @@ void CodeGenFunction::EmitCondBrHints(ll >> continue; >> >> LoopHintAttr::OptionType Option = LH->getOption(); >> + LoopHintAttr::LoopHintState State = LH->getState(); >> int ValueInt = LH->getValue(); >> >> const char *MetadataName; >> @@ -602,8 +603,8 @@ void CodeGenFunction::EmitCondBrHints(ll >> 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 @@ void CodeGenFunction::EmitCondBrHints(ll >> 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"); >> >> Modified: cfe/trunk/lib/Parse/ParsePragma.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParsePragma.cpp?rev=214333&r1=214332&r2=214333&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Parse/ParsePragma.cpp (original) >> +++ cfe/trunk/lib/Parse/ParsePragma.cpp Wed Jul 30 15:54:33 2014 >> @@ -722,38 +722,65 @@ struct PragmaLoopHintInfo { >> 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); >> + >> + // Return a valid hint if pragma unroll or nounroll were specified >> + // without an argument. >> + bool PragmaUnroll = PragmaNameInfo->getName() == "unroll"; >> + bool PragmaNoUnroll = PragmaNameInfo->getName() == "nounroll"; >> + if (!Info->HasValue && (PragmaUnroll || PragmaNoUnroll)) { >> + Hint.Range = Info->PragmaName.getLocation(); >> + return true; >> + } >> + >> + // If no option is specified the argument is assumed to be numeric. >> + bool StateOption = false; >> + if (OptionInfo) >> + 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 +1782,10 @@ void PragmaOptimizeHandler::HandlePragma >> } >> >> /// \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 +1809,15 @@ static bool ParseLoopHintValue(Preproces >> >> // 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,17 +1899,19 @@ void PragmaLoopHintHandler::HandlePragma >> << /*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; >> @@ -1891,9 +1920,6 @@ void PragmaLoopHintHandler::HandlePragma >> 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 +1964,6 @@ void PragmaUnrollHintHandler::HandlePrag >> 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 +1972,12 @@ void PragmaUnrollHintHandler::HandlePrag >> } 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 +1986,6 @@ void PragmaUnrollHintHandler::HandlePrag >> 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"; >> >> Modified: cfe/trunk/lib/Parse/ParseStmt.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=214333&r1=214332&r2=214333&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Parse/ParseStmt.cpp (original) >> +++ cfe/trunk/lib/Parse/ParseStmt.cpp Wed Jul 30 15:54:33 2014 >> @@ -1817,10 +1817,11 @@ StmtResult Parser::ParsePragmaLoopHint(S >> >> // 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, >> >> Modified: cfe/trunk/lib/Sema/SemaStmtAttr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmtAttr.cpp?rev=214333&r1=214332&r2=214333&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaStmtAttr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaStmtAttr.cpp Wed Jul 30 15:54:33 2014 >> @@ -47,13 +47,11 @@ static Attr *handleLoopHintAttr(Sema &S, >> 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 @@ static Attr *handleLoopHintAttr(Sema &S, >> >> 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 @@ static Attr *handleLoopHintAttr(Sema &S, >> 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::LoopHintState State = LoopHintAttr::Default; >> + if (PragmaNoUnroll) { >> + State = LoopHintAttr::Disable; >> } else if (Option == LoopHintAttr::VectorizeWidth || >> Option == LoopHintAttr::InterleaveCount || >> Option == LoopHintAttr::UnrollCount) { >> @@ -124,28 +100,39 @@ static Attr *handleLoopHintAttr(Sema &S, >> 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.getLoc(), diag::err_pragma_loop_invalid_value); >> 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 @@ static void CheckForIncompatibleAttribut >> 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 @@ static void CheckForIncompatibleAttribut >> << /*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); >> } >> } >> >> Modified: cfe/trunk/test/Parser/pragma-loop.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/pragma-loop.cpp?rev=214333&r1=214332&r2=214333&view=diff >> ============================================================================== >> --- cfe/trunk/test/Parser/pragma-loop.cpp (original) >> +++ cfe/trunk/test/Parser/pragma-loop.cpp Wed Jul 30 15:54:33 2014 >> @@ -83,6 +83,9 @@ void test(int *List, int Length) { >> List[i] = i; >> } >> >> +/* expected-error {{expected ')'}} */ #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 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) >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
