[PATCH] Add support for CUDA unroll pragma
Mark Heffernan
meheff at google.com
Wed Jun 25 16:31:28 PDT 2014
================
Comment at: include/clang/Parse/Parser.h:1619
@@ -1612,1 +1618,3 @@
ParsedAttributesWithRange &Attrs);
+ StmtResult ParsePragmaCudaUnroll(StmtVector &Stmts, bool OnlyStatement,
+ SourceLocation *TrailingElseLoc,
----------------
Eli Bendersky wrote:
> How about a comment above the method declaration that explains what those arguments are? I know the surrounding code doesn't do this, but with new code we can do better :)
Done.
================
Comment at: include/clang/Sema/CudaUnrollHint.h:19
@@ +18,3 @@
+/// \brief Loop hint specified by a pragma loop directive.
+struct CudaUnrollHint {
+ IdentifierLoc *UnrollLoc;
----------------
Eli Bendersky wrote:
> Ditto... Documentation of every field in this struct.
Done.
================
Comment at: lib/CodeGen/CGStmt.cpp:554
@@ +553,3 @@
+ // Set or overwrite metadata indicated by Name.
+ Metadata.push_back(llvm::MDNode::get(Context, OpValues));
+ }
----------------
Eli Bendersky wrote:
> Since MDNode::get accepts a ArrayRef, I wonder if there's a way to not have the OpValues vector at all?
>
> At the minimum:
>
> llvm::Value *Values[] = {MetadataPair.first, MetadataPair.second};
> ... MDNode::get(Context, Values);
>
> It may be possible to inline the {...} into the ::get too, I haven't checked.
Done.
================
Comment at: lib/Sema/SemaStmtAttr.cpp:119
@@ +118,3 @@
+
+ IdentifierLoc *ValueLoc = A.getArgAsIdent(0);
+ int ValueInt = 0;
----------------
Eli Bendersky wrote:
> You could fold this into the if (...)
Done.
http://reviews.llvm.org/D4297
More information about the cfe-commits
mailing list