[PATCH] Add support for unroll pragma

[Mark Heffernan]

Thanks for the comments.  Responses are below.  With this patch "#pragma unroll" and "#pragma clang loop unroll" syntaxes are both supported.  I also added "#pragma nounroll" as both icc and xlc support this pragma.

On Wed, Jul 2, 2014 at 7:54 AM, Aaron Ballman <aaron.ballman at gmail.com> wrote:
>
> >    /// State of the loop optimization specified by the spelling.
> >    let Args = [EnumArgument<"Option", "OptionType",
> >                            ["vectorize", "vectorize_width", "interleave", "interleave_count",
> >                             "unroll", "unroll_count"],
> >                            ["Vectorize", "VectorizeWidth", "Interleave", "InterleaveCount",
> >                             "Unroll", "UnrollCount"]>,
> > -              DefaultIntArgument<"Value", 1>];
> > +              DefaultIntArgument<"Value", 1>,
> > +              DefaultBoolArgument<"ValueInParens", 0>];
>
> Hmm... we usually want the arguments to match the syntax of the
> attribute unless the argument is really key to the semantics.
> ValueInParens doesn't really fit that model. I would prefer for this
> to be an AdditionalMembers entry.

I tried putting ValueInParens in the AdditionalMembers, but then ran into issues with test/PCH/pragma-loop.  I just added a member variable and a method to set it, but then ValueInParens is then not set when reading in the PCH file.  It only works if ValueInParens is not in Args.  Am I missing something?
 
>
> >    "%select{invalid|missing}0 option%select{ %1|}0; expected vectorize, "
> >    "vectorize_width, interleave, interleave_count, unroll, or unroll_count">;
> >  def err_pragma_loop_missing_argument : Error<
> > -  "missing argument to loop pragma %0">;
> > +  "missing argument to %0 pragma">;
>
> The name of this diagnostic should probably change given how generic
> the text now is.


Done.

> > +def err_pragma_loop_duplicate : Error<
> > +  "duplicate '%0' directives">;
> > +def err_pragma_loop_incompatible : Error<
> > +  "incompatible directives '%0(%1)' and '%2(%3)'">;
>
> Why was this split into two errors?

This was originally done because the duplicate form was also used when there was no parameter such as '#pragma unroll'.  I've refactored it and now they are back to being one error.
 
> +// Parses loop or unroll pragma hint value and fills in Info.
>
> > +bool ParseLoopHintValue(Preprocessor &PP, Token Tok, Token &PragmaName,
> > +                        Token &Option, PragmaLoopHintInfo &Info) {
> > +  bool ValueInParens;
>
> Could simply initialize to Tok.is(tok::l_paren), then not need the
> else clause at all.

Good point.  Done.
 
>
> > -static void
> > -CheckForIncompatibleAttributes(Sema &S, SmallVectorImpl<const Attr *> &Attrs) {
> > +static void CheckForIncompatibleAttributes(
> > +    Sema &S, SmallVectorImpl<const Attr *> &Attrs) {
>
> Attrs should be const.

Done.

http://reviews.llvm.org/D4297

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/LoopHint.h
  lib/Parse/ParsePragma.cpp
  lib/Parse/ParseStmt.cpp
  lib/Sema/SemaStmtAttr.cpp
  test/CodeGen/pragma-unroll.cpp
  test/PCH/pragma-loop.cpp
  test/Parser/pragma-loop.cpp
  test/Parser/pragma-unroll.cpp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D4297.11185.patch
Type: text/x-patch
Size: 40826 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140709/d7b40b74/attachment.bin>