[PATCH] D16686: [OpenCL] Generate metadata for opencl_unroll_hint attribute

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.

================
Comment at: include/clang/Parse/Parser.h:2200
@@ -2199,1 +2199,3 @@
   void ParseOpenCLQualifiers(ParsedAttributes &Attrs);
+  /// \brief Parses opencl_unroll_hint attribute if language is OpenCL 2.0+.
+  /// \return false if error happens.
----------------
May be better:
 OpenCL 2.0+ -> OpenCL v2.0 or higher

================
Comment at: lib/CodeGen/CGLoopInfo.cpp:131
@@ +130,3 @@
+    // equivalent LoopHintAttr enums.
+    // See OpenCL 2.0 spec section 6.11.5 for details.
+    // 0 corresponds to opencl_unroll_hint attribute without
----------------
Just to match the other comment style:

OpenCL 2.0 spec section 6.11.5 -> OpenCL v2.0 s6.11.5

================
Comment at: lib/Sema/SemaStmtAttr.cpp:212
@@ +211,3 @@
+  // factor) or 1 argument (the unroll factor provided by the user).
+  // See OpenCL 2.0 spec section 6.11.5 for details.
+
----------------
OpenCL v2.0 s6.11.5

================
Comment at: lib/Sema/SemaStmtAttr.cpp:221
@@ +220,3 @@
+
+  unsigned unrollingFactor = 0;
+
----------------
Variable names are still not compliant to coding style. Could you please change.

================
Comment at: lib/Sema/SemaStmtAttr.cpp:234
@@ +233,3 @@
+    }
+
+    int64_t val = ArgVal.getSExtValue();
----------------
Could you please change the type to int to match similar code in other places.

================
Comment at: lib/Sema/SemaStmtAttr.cpp:235
@@ +234,3 @@
+
+    int64_t val = ArgVal.getSExtValue();
+
----------------
Also could you change variable name here too and in other places to follow the coding style.

================
Comment at: test/CodeGenOpenCL/unroll-hint.cl:9
@@ +8,3 @@
+    for( int i = 0; i < 1000; ++i) {
+        *sum += i;
+    }
----------------
I would make all loops with an empty body here too as it doesn't play any role in testing. :)


http://reviews.llvm.org/D16686