[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: Richard Smith <richard () metafoo ! co ! uk>
Date: 2015-07-30 23:38:43
Message-ID: 6c9e78004c906d4fb1aaf2c1bd5d6397 () localhost ! localdomain
[Download RAW message or body]
rsmith added a comment.
Please find a better way to handle this attribute in DeclPrinter. Perhaps you could \
generalize your existing support to cover all attributes with pragma spelling?
================
Comment at: lib/Parse/ParseDeclCXX.cpp:2816
@@ -2818,1 +2815,3 @@
+Parser::DeclGroupPtrTy Parser::ParseCXXClassMemberDeclarationWithPragmas(
+ AccessSpecifier &AS, ParsedAttributesWithRange &AccessAttrs,
----------------
Please can you split this refactoring out into a separate commit? That'll make the \
functional change here (handling `annot_pragma_openmp`) much easier for future code \
archaeologists to see.
================
Comment at: lib/Parse/ParseOpenMP.cpp:50-73
@@ -48,26 +49,26 @@
for (unsigned i = 0; i < llvm::array_lengthof(F); ++i) {
- if (!Tok.isAnnotation() && DKind == OMPD_unknown) {
+ if (!Tok.isAnnotation() && DKind == OMPD_unknown)
TokenMatched =
- (i == 0) &&
- !P.getPreprocessor().getSpelling(Tok).compare("cancellation");
- } else {
+ ((i == 0) &&
+ !P.getPreprocessor().getSpelling(Tok).compare("declare")) ||
+ ((i == 1) &&
+ !P.getPreprocessor().getSpelling(Tok).compare("cancellation"));
+ else
TokenMatched = DKind == F[i][0] && DKind != OMPD_unknown;
- }
if (TokenMatched) {
Tok = P.getPreprocessor().LookAhead(0);
auto TokenIsAnnotation = Tok.isAnnotation();
auto SDKind =
TokenIsAnnotation
? OMPD_unknown
: getOpenMPDirectiveKind(P.getPreprocessor().getSpelling(Tok));
if (!TokenIsAnnotation && SDKind == OMPD_unknown) {
TokenMatched =
- ((i == 0) &&
+ ((i == 1) &&
!P.getPreprocessor().getSpelling(Tok).compare("point")) ||
- ((i == 1) && !P.getPreprocessor().getSpelling(Tok).compare("data"));
- } else {
+ ((i == 2) && !P.getPreprocessor().getSpelling(Tok).compare("data"));
+ } else
TokenMatched = SDKind == F[i][1] && SDKind != OMPD_unknown;
----------------
Bracing here is now inconsistent.
================
Comment at: lib/Parse/ParseOpenMP.cpp:150
@@ +149,3 @@
+ TagDecl);
+ else
+ // Here we expect to see some function declaration.
----------------
Add braces around this multi-line `else` (and the corresponding `if` for \
consistency).
================
Comment at: lib/Parse/ParseOpenMP.cpp:154
@@ +153,3 @@
+ (!Ptr || Ptr.get().isNull())) {
+ if (AS == AS_none || TagType == DeclSpec::TST_unspecified) {
+ MaybeParseCXX11Attributes(Attrs);
----------------
Do you really need both these checks?
================
Comment at: lib/Parse/ParseOpenMP.cpp:171-177
@@ +170,9 @@
+
+ // Append the current token at the end of the new token stream so that it
+ // doesn't get lost.
+ CachedPragmas.push_back(Tok);
+ // Push back tokens for pragma.
+ PP.EnterTokenStream(CachedPragmas.data(), CachedPragmas.size(),
+ /*DisableMacroExpansion=*/true,
+ /*OwnsTokens=*/false);
+ // Parse pragma itself.
----------------
Why are you doing this delayed parsing? You still seem to have no tests that require \
it.
================
Comment at: lib/Sema/SemaOpenMP.cpp:2153
@@ +2152,3 @@
+Sema::DeclGroupPtrTy
+Sema::ActOnOpenMPDeclareSimdDirective(ArrayRef<OMPClause *> Clauses,
+ Decl *ADecl, SourceLocation StartLoc) {
----------------
You don't use `Clauses` for anything in here; is that intentional?
http://reviews.llvm.org/D10599
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic