[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