[PATCH] Add support for CUDA unroll pragma

Eli Bendersky eliben at google.com
Wed Jun 25 15:19:31 PDT 2014


================
Comment at: include/clang/Parse/Parser.h:1619
@@ -1612,1 +1618,3 @@
                                  ParsedAttributesWithRange &Attrs);
+  StmtResult ParsePragmaCudaUnroll(StmtVector &Stmts, bool OnlyStatement,
+                                   SourceLocation *TrailingElseLoc,
----------------
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 :)

================
Comment at: include/clang/Sema/CudaUnrollHint.h:19
@@ +18,3 @@
+/// \brief Loop hint specified by a pragma loop directive.
+struct CudaUnrollHint {
+  IdentifierLoc *UnrollLoc;
----------------
Ditto... Documentation of every field in this struct.

================
Comment at: lib/CodeGen/CGStmt.cpp:554
@@ +553,3 @@
+    // Set or overwrite metadata indicated by Name.
+    Metadata.push_back(llvm::MDNode::get(Context, OpValues));
+  }
----------------
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.

================
Comment at: lib/Sema/SemaStmtAttr.cpp:119
@@ +118,3 @@
+
+  IdentifierLoc *ValueLoc = A.getArgAsIdent(0);
+  int ValueInt = 0;
----------------
You could fold this into the if (...)

http://reviews.llvm.org/D4297






More information about the cfe-commits mailing list