[PATCH] Add support for unroll pragma

[Mark Heffernan]

On Fri, Jul 18, 2014 at 1:00 PM, Tyler Nowicki <tnowicki at apple.com> wrote:

> Do you have an updated patch that would be applied on top of the constant
> expression patch? Perhaps you could post that so we can get a head start on
> reviewing it?
>

I haven't tried integrating the constant expression patch.  I've got a
couple other things to work on at the moment.  If I get those things done
then I can take a look at integrating it though your patch will probably
land soon forcing the issue.

Thanks for extending the diagnostics and pragmas to unrolling!
>

HTH!

Mark


>
> Tyler
>
> On Jul 18, 2014, at 10:54 AM, Mark Heffernan <meheff at google.com> wrote:
>
> Ping?  Aaron, any more comments on this?
>
> Thanks,
> Mark
>
>
> On Tue, Jul 15, 2014 at 2:09 PM, Mark Heffernan <meheff at google.com> wrote:
>
>> I added a warning if the wrong pragma unroll syntax is used with CUDA.
>>  When Tyler's change adding support for expressions in loop pragmas lands,
>> I'll add a warning for the case when the pragma argument is not an integer
>> literal in CUDA mode.  Responses to Aaron's comments inline.
>>
>> On Wed, Jul 9, 2014 at 7:42 AM, Aaron Ballman <aaron.ballman at gmail.com>
>> wrote:
>> > On Wed, Jul 9, 2014 at 2:14 AM, Mark Heffernan <meheff at google.com>
>> wrote:
>> >> I also added "#pragma nounroll" as both icc and xlc support this
>> pragma.
>> >
>> > Please split nounroll out into a separate patch. The idea is certainly
>> > reasonable, but it's preferable for patches to cover a limited,
>> > incremental change.
>>
>> Done.
>>
>> >> On Wed, Jul 2, 2014 at 7:54 AM, Aaron Ballman <aaron.ballman at gmail.com>
>> wrote:
>> > Okay, I can see why that would happen. ClangAtterEmitter.cpp generates
>> > code for serialization and deserialization, but it doesn't have any
>> > way of handling additional members (since those are just copy/pasted
>> > into the attribute class definition itself).
>> >
>> > I wonder how horrible it would be to simply canonicalize based on
>> > compiler options when pretty printing, and skip this field entirely.
>> > Eg) when CUDA is on, pretty print does not emit the parens. When CUDA
>> > mode is off, it does emit the parens. Yes, this isn't *exactly* what
>> > the user wrote, but the semantics are identical either way.
>>
>> It's now cannonicalized to the form with parentheses '#pragma unroll(n)'.
>>  Tracking of the existence of parentheses is gone.
>>
>> >> Index: include/clang/Basic/DiagnosticParseKinds.td
>> >> ===================================================================
>> >> --- include/clang/Basic/DiagnosticParseKinds.td
>> >> +++ include/clang/Basic/DiagnosticParseKinds.td
>> >> @@ -811,6 +811,9 @@
>> >>    InGroup<IgnoredPragmas>;
>> >>  def warn_pragma_expected_punc : Warning<
>> >>    "expected ')' or ',' in '#pragma %0'">, InGroup<IgnoredPragmas>;
>> >> +// - Generic errors
>> >> +def err_pragma_missing_argument : Error<
>> >> +  "missing argument to %0 directive">;
>> >
>> > Since it's a generic parsing diagnostic, presumably we know what kind
>> > of argument is missing, and we could tell the user that information.
>> > What's more, since pragmas can have multiple arguments, this doesn't
>> > say *which* argument is missing (there could be more than one).
>> >
>> > I also think the word "directive" could be dropped.
>>
>> Created a generic diagnostic: "missing argument to '#pragma %0'; expected
>> %1".  This new warning is also now shared with one pragma optimize warning.
>>  Regarding stating which argument is missing, all of the pragmas using this
>> warning only take a single argument.  I think it makes sense not to specify
>> argument position unless multiple arguments are allowed.  That is, "missing
>> first argument to '#pragma unroll'" suggests pragma unroll accepts more
>> than one argument.  If this error is later used for pragmas with multiple
>> parameters, the format string can be adjusted accordingly.
>>
>> >>  def err_pragma_loop_precedes_nonloop : Error<
>> >> -  "expected a for, while, or do-while loop to follow the '#pragma
>> clang loop' "
>> >> -  "directive">;
>> >> +  "expected a for, while, or do-while loop to follow the '%0'
>> directive">;
>> >
>> > I would drop "directive" (and "the") here too.
>>
>> Done.
>>
>> http://reviews.llvm.org/D4297
>>
>> Files:
>>   docs/ReleaseNotes.rst
>>   include/clang/Basic/Attr.td
>>   include/clang/Basic/AttrDocs.td
>>   include/clang/Basic/DiagnosticGroups.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
>>   test/Parser/warn-cuda-compat.cu
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140718/0ffb64a3/attachment.html>