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

[Alexey Bataev via Phabricator via cfe-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:
> > > > 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?
> Thanks for the extra information. I'll add a test based on this for sure, no matter what we come up with for now.
> 
> ```
> #pragma omp begin assumes no_openmp
>   S<int> s; // S<int> is instantiated here and function S<int>::foo() gets no_openmp attribute.
>   s.a = 0;
> #pragma omp end assumes
> ```
> 
> Hm, I guess the question is: 
> Is `S<int>::foo()` a function defined in between the `begin`/`end` or not. If we say it is, the behavior right now seems consistent but the standard might need some clarifications. If we say it is not, we should not annotate it. Though, if it is not, where is it defined? I could avoid annotating template instantiations I guess. I might start with that. Update is coming.
> 
> 
Yep, this is what I'm asking about. I would check С++ standard, I believe we shall follow С++ rules here about the instantiation context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91980