r214333 - Add a state variable to the loop hint attribute.

Kostya Serebryany kcc at google.com
Fri Aug 1 05:09:22 PDT 2014


Looks like this change (or the one nearby) broke msan build
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/4103/steps/check-clang%20msan/logs/stdio

==5554== WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x7fe687797e49 in
clang::Parser::HandlePragmaLoopHint(clang::LoopHint&)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/ParsePragma.cpp:738:32
    #1 0x7fe6877c3be6 in
clang::Parser::ParsePragmaLoopHint(llvm::SmallVector<clang::Stmt*,
32u>&, bool, clang::SourceLocation*,
clang::Parser::ParsedAttributesWithRange&)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/ParseStmt.cpp:1821:10
    #2 0x7fe6877afb8f in
clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*,
32u>&, bool, clang::SourceLocation*,
clang::Parser::ParsedAttributesWithRange&)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/ParseStmt.cpp:362:12
    #3 0x7fe6877ad95e in
clang::Parser::ParseStatementOrDeclaration(llvm::SmallVector<clang::Stmt*,
32u>&, bool, clang::SourceLocation*)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/ParseStmt.cpp:106:20
    #4 0x7fe6877c7558 in
clang::Parser::ParseCompoundStatementBody(bool)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/ParseStmt.cpp:938:11
    #5 0x7fe6877c95d4 in
clang::Parser::ParseFunctionStatementBody(clang::Decl*,
clang::Parser::ParseScope&)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/ParseStmt.cpp:1857:21
    #6 0x7fe68763e2a7 in
clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&,
clang::Parser::ParsedTemplateInfo const&,
clang::Parser::LateParsedAttrList*)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/Parser.cpp:1098:10
    #7 0x7fe6876777de in
clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, unsigned int,
bool, clang::SourceLocation*, clang::Parser::ForRangeInit*)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/ParseDecl.cpp:1588:11
    #8 0x7fe68763bdab in
clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&,
clang::ParsingDeclSpec&, clang::AccessSpecifier)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/Parser.cpp:888:10
    #9 0x7fe68763a740 in
clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&,
clang::ParsingDeclSpec*, clang::AccessSpecifier)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/Parser.cpp:904:12
    #10 0x7fe6876376ab in
clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&,
clang::ParsingDeclSpec*)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/Parser.cpp:762:12
    #11 0x7fe68763497d in
clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/Parser.cpp:559:12
    #12 0x7fe687625995 in clang::ParseAST(clang::Sema&, bool, bool)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Parse/ParseAST.cpp:135:7
    #13 0x7fe685a38c34 in clang::FrontendAction::Execute()
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Frontend/FrontendAction.cpp:415:8
    #14 0x7fe6859a8f0e in
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/Frontend/CompilerInstance.cpp:810:7
    #15 0x7fe685bebc35 in
clang::ExecuteCompilerInvocation(clang::CompilerInstance*)
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:222:18
    #16 0x7fe68287db73 in cc1_main(char const**, char const**, char
const*, void*) /home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/tools/driver/cc1_main.cpp:112:13
    #17 0x7fe6828780b2 in main
/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm/tools/clang/tools/driver/driver.cpp:318:14
    #18 0x7fe680b46ec4 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
    #19 0x7fe68287163c in _start
(/home/dtoolsbot/build/sanitizer-x86_64-linux-bootstrap/build/llvm_build_msan/bin/clang-3.6+0x62163c)


valgrind agrees:

valgrind clang -cc1 -internal-isystem lib/clang/3.6.0/include -std=c++11
-verify ~/llvm/tools/clang/test/Parser/pragma-unroll.cpp
==27791== Conditional jump or move depends on uninitialised value(s)
==27791==    at 0x1CF89EB:
clang::Parser::HandlePragmaLoopHint(clang::LoopHint&)
==27791==    by 0x1D02AFA:
clang::Parser::ParsePragmaLoopHint(llvm::SmallVector<clang::Stmt*, 32u>&,
bool, clang::SourceLocation*, clang::Parser::ParsedAttributesWithRange&)
==27791==    by 0x1CFCE3B:
clang::Parser::ParseStatementOrDeclarationAfterAttributes(llvm::SmallVector<clang::Stmt*,
32u>&, bool, clang::SourceLocation*,
clang::Parser::ParsedAttributesWithRange&)




On Thu, Jul 31, 2014 at 12:54 AM, Tyler Nowicki <tnowicki at apple.com> wrote:

> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140801/6c964f7b/attachment.html>


More information about the cfe-commits mailing list