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

[Alexey Bataev via Phabricator via Openmp-commits]

ABataev added inline comments.


================
Comment at: clang/include/clang/Sema/Sema.h:10333
+  /// Check if there is an active global `omp begin assumes` directive.
+  bool isInOpenMPAssumeScope() { return !OMPAssumeScoped.empty(); }
+
----------------
jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > ABataev wrote:
> > > > > `const` member functions.
> > > > It may have side-effects with template instantiations. What if we have something like this:
> > > > ```
> > > > #pragma omp begin assumes ...
> > > > template <typename T>
> > > > struct S {
> > > > ...
> > > > }
> > > > #pragma omp end assumes
> > > > 
> > > > void bar() {
> > > > #pragma omp assumes ...
> > > >   {
> > > >     S<int> s;
> > > >     s.foo();
> > > >   }
> > > > }
> > > > ```
> > > > ?
> > > > 
> > > > `struct S<int>` will be instantiated in the second assume region context though I doubt this is the user intention.
> > > I'm not sure what problem you expect here. Could you elaborate what you think should happen or should not?
> > May it lead to the wrong compiler assumptions and result in the incorrect codegen?
> I'm still not following your example. What wrong assumptions and/or incorrect code generation do you expect? Could you provide an example with actual assumptions and then describe what the problem is please.
Say, you have something like this:
```
template <typename T>
struct S {
  int a;
  void foo() {
    #pragma omp parallel
    ...
  }
};

void bar() {
#pragma omp assumems no_openmp
  S<int> s; // S<int> is instantiated here and function S<int>::foo() gets no_openmp attribute.
  s.a = 0; // S<int>::foo is instantiated here but not used and the attribute no_openmp should not be propagated for this function.
#pragma omp end assumes

  S<int> b; // S<int> already instantiated and S<int>::foo is instantiated with attribute no_openmp
  b.foo(); // calls function S<int>::foo with no_openmp attribute.
}
```
What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91980