[PATCH] Add support for unroll pragma
Tyler Nowicki
tnowicki at apple.com
Fri Jul 18 13:00:31 PDT 2014
Hi Mark,
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?
Thanks for extending the diagnostics and pragmas to unrolling!
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/1d66b5b9/attachment.html>
More information about the cfe-commits
mailing list