[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:       "Bataev, Alexey" <a.bataev () hotmail ! com>
Date:       2015-08-03 4:24:01
Message-ID: BLU436-SMTP219029EE8BEB73402B044A285770 () phx ! gbl
[Download RAW message or body]

Ok,
I'll remove it for now and rework it later when it will be required.

Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team

01.08.2015 0:07, Richard Smith пишет:
> rsmith added inline comments.
> 
> ================
> 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.
> ----------------
> ABataev wrote:
> > rsmith wrote:
> > > Why are you doing this delayed parsing? You still seem to have no tests that \
> > > require it.
> > We need it for future parsing of clauses associated with the 'declare simd' \
> > construct. Some of the clauses may have references to function arguments. That's \
> > why I'm using delayed parsing: at first we need to parse a function along with \
> > arguments and only then we'll parse all the pragmas along with their clauses and \
> > references to args. Of course currently it is not used, because this patch does \
> > not introduce parsing of clauses. It will be added later.
> This code won't work for that; you've left the scope of the function parameters, so \
> lookup for them will fail here. Generally-speaking, we don't like speculative / \
> untested code to be committed, and would prefer to hold back on those changes until \
> we can actually test them in some way. 
> Here's what I suggest: remove the delayed parsing code from here for this commit \
> (parse the pragma first, then parse the nested declaration, then act on the \
> result), and bring that code back once you introduce parsing for a clause that \
> needs it, if indeed that's the right approach for those clauses. (As I mentioned \
> before, if all they do is to provide a list of identifiers naming parameters, you \
> don't need delay parsing and can instead store them as identifiers and look them up \
> in Sema after the fact. On the other hand, if this pragma allows an arbitrary \
> expression referencing a parameter to appear before the declaration of that \
> parameter, then we'll need something like delayed parsing.) 
> 
> 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]