[PATCH] D10599: [OPENMP 4.0] Initial support for '#pragma omp declare simd' directive.

Alexey Bataev via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 16 01:49:43 PST 2015


ABataev marked 4 inline comments as done.

================
Comment at: include/clang/Basic/AttrDocs.td:1620
@@ +1619,3 @@
+  let Content = [{
+The declare simd construct can be applied to a function to enable the creation of one or more versions that can process multiple arguments using SIMD instructions from a single invocation from a SIMD loop. The declare simd directive is a declarative directive. There may be multiple declare simd directives for a function. The use of a declare simd construct on a function enables the creation of SIMD
+versions of the associated function that can be used to process multiple arguments from a single invocation from a SIMD loop concurrently.
----------------
aaron.ballman wrote:
> Line length is an issue here. ;-)
> 
> Also, "from a single invocation from a SIMD loop"; should that be "from a single invocation of a SIMD loop" instead?
> 
> Any time you use "declare simd" as a syntactic phrase, it should be properly formatted for RST as ``declare simd``.
Fixed, thanks

================
Comment at: lib/Parse/ParseOpenMP.cpp:97
@@ -89,1 +96,3 @@
+    AccessSpecifier &AS, ParsedAttributesWithRange &Attrs,
+    DeclSpec::TST TagType, Decl *TagDecl) {
   assert(Tok.is(tok::annot_pragma_openmp) && "Not an OpenMP directive!");
----------------
aaron.ballman wrote:
> Can we not name the last parameter with an identifier that is also a type name? That always weirds me out. ;-)
Fixed, thanks.

================
Comment at: lib/Parse/ParseOpenMP.cpp:152
@@ +151,3 @@
+      } else {
+        Ptr = ParseCXXClassMemberDeclarationWithPragmas(AS, Attrs, TagType,
+                                                        TagDecl);
----------------
aaron.ballman wrote:
> Can you add a test case for a member declaration that has attributes to ensure the attributes are properly handled here? In fact, it would be good to add one for the top-level function as well.
Ok, will add

================
Comment at: lib/Sema/SemaOpenMP.cpp:2370
@@ +2369,3 @@
+
+  auto *NewAttr = new (Context) OMPDeclareSimdDeclAttr(
+      SourceRange(StartLoc, StartLoc), Context, /*SI=*/0);
----------------
aaron.ballman wrote:
> Please use OMPDeclareSimdDeclAttr::CreateImplicit instead.
Fixed, thanks


http://reviews.llvm.org/D10599





More information about the cfe-commits mailing list