[PATCH] D91980: [OpenMP] Add initial support for `omp [begin/end] assumes`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert marked 6 inline comments as done.
jdoerfert added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3210
+
+    SmallVector<DeclContext *, 8> DeclContexts;
+    DeclContexts.push_back(CurContext);
----------------
ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > ABataev wrote:
> > > > > What are you trying to do here? Scan all the decls in the current context and mark all the functions with the assumption attributes?
> > > > Pretty much, yeah. Find all function declarations and add the attribute. I'll add a comment.
> > > Does it mean that for this code
> > > ```
> > > void foo();
> > >  #pragma omp assume ...
> > > void bar();
> > >  #pragma omp end assume
> > > ```
> > > `foo()` must have the same assumptions like `bar()`? It looks to me, currently we'll end up with the same attributes for both functions. But I'm not sure that this is correct to mark the function outside of the `assumes` region with the same attributes just like the function inside.
> > Your example mixes the global `omp assumes` with the scoped `begin/end omp assumes`. The former has the effect to be added to prior declarations, the latter does not have this effect. This is how it is supposed to work:
> > ```
> > void foo();  // no assumption
> >  #pragma omp begin assumes ext_XYZ
> > void bar();  // XYZ assumption
> >  #pragma omp end assumes
> > ```
> > ```
> > void foo();  // XYZ assumption
> >  #pragma omp assumes ext_XYZ
> > void bar();  // XYZ assumption
> > ```
> > 
> > 
> > 
> Ah, I see. `omp assumes` looks weird. :( It may lead to conflicts
> PS. What about lambdas? Could add a test that lambdas get annotations too?
> Ah, I see. omp assumes looks weird. :( It may lead to conflicts

The reason we have `assumes` and `assume` is to avoid conflicts. Let's hope it works.

> PS. What about lambdas? Could add a test that lambdas get annotations too?

Done, assuming lambdas are function declarations. Otherwise we would need to exclude them from the scoped assumptions as we do for template instantiations now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91980/new/

https://reviews.llvm.org/D91980