[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] D10599: [OPENMP 4.0] Initial support for '#pragma omp declare simd' directive.
From:       Alexey Bataev via cfe-commits <cfe-commits () lists ! llvm ! org>
Date:       2015-10-13 8:21:25
Message-ID: fea4ea1670d9ad581bfd94867dda0c2f () localhost ! localdomain
[Download RAW message or body]

ABataev marked 3 inline comments as done.

================
Comment at: include/clang/Basic/Attr.td:2098-2102
@@ +2097,7 @@
+  let Documentation = [OMPDeclareSimdDocs];
+  let AdditionalMembers = [{
+  void printPrettyPragma(raw_ostream &OS, const PrintingPolicy &Policy) const {
+    OS << "\n";
+  }
+  }];
+}
----------------
rsmith wrote:
> Can we instead handle this in the generated code? Presumably we'd get this wrong in \
> the same way for all attributes with pragma spelling?
Done.

================
Comment at: lib/Parse/ParseOpenMP.cpp:133
@@ +132,3 @@
+          << getOpenMPDirectiveName(OMPD_declare_simd);
+      if (!SkipUntil(tok::annot_pragma_openmp_end, StopBeforeMatch))
+        return DeclGroupPtrTy();
----------------
rsmith wrote:
> Hmm, on reflection it would be better to use:
> 
> while (Tok.isNot(tok::annot_pragma_openmp_end))
> ConsumeAnyToken();
> 
> We know that there is an `annot_pragma_openmp_end` token coming, but `SkipUntil` \
> might stop early if there's (say) a stray right-paren in the token stream.
Done, thanks.

================
Comment at: lib/Parse/ParseOpenMP.cpp:144-145
@@ +143,4 @@
+    } else {
+      // Here we expect to see some function declaration.
+      while (Tok.isNot(tok::r_brace) && !isEofOrEom() && !Ptr) {
+        if (AS == AS_none) {
----------------
rsmith wrote:
> I don't think your example is reasonable to accept. Right now (prior to your \
> patch), we have two fundamentally different kinds of pragmas: 
> 1) Lexer-level pragmas. These can appear anywhere in the token stream, and take \
> effect immediately. These are handled entirely by the lexer. 
> 2) Declaration-like pragmas. These can only appear where a declaration is \
> permitted, and act as if they declare ... something. We transform these into tokens \
> and handle them in the parser. 
> You're adding a third kind of pragma, an attribute-like pragma, that can only \
> appear before declarations. As such, if your pragma appears before a pragma of kind \
> 2 (such as `#pragma GCC visibility`), it should act as an attribute applying to \
> *that* declaration-like entity. 
> So I think the only thing you should allow between your pragma and the declaration \
> to which it applies is more attribute-like pragmas.
Reworked.


http://reviews.llvm.org/D10599



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]