ABataev added inline comments.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:4374
+ case DMIB_unspecified:
+ return OMPC_MAP_tofrom;
+ }
----------------
cchen wrote:
> ABataev wrote:
> > cchen wrote:
> > > ABataev wrote:
> > > > cchen wrote:
> > > > > ABataev wrote:
> > > > > > `OMPC_MAP_tofrom` must be returned only for `DMIB_tofrom`, for
> > > > > > others, it must be `llvm_unreachable(....)`
> > > > > For DMIB_firstprivate, I think you are right, it should be
> > > > > unreachable, however, for DMIB_default and DMIB_unspecified, they are
> > > > > definitely reachable in `ActOnOpenMPExecutableDirective`.
> > > > Then this function is not correct because for scalars default is not
> > > > tofrom.
> > > You are right, for scalar and pointer, the implicit behavior is
> > > firstprivate, so they are unreachable in this case, however, for
> > > aggregate, the implicit behavior is tofrom (I emit the .ll file for
> > > aggregate using the master branch and found that the maptype is 547).
> > You need to add extra checks for scalars and pointers in this function.
> In fact, it is possible for scalar or pointer to reach `DMIB_default` and
> `DMIB_unspecified` case. For this example:
> ```
> extern int c;
> #pragma omp declare target link(c)
>
> void foo() {
> #pragma omp target defaultmap(default:scalar)
> {
> c++;
> }
> }
> ```
> `c` will be added into `ImplicitMap` since `Res` is not empty, therefore,
> `DMIB_default` and `DMIB_unspecified` is reachable.
Again, it means that this function is not correct in all cases and should be
reworked somehow or deleted.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69204/new/
https://reviews.llvm.org/D69204
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
