cchen marked an inline comment as done.
cchen added inline comments.

================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:4374
+  case DMIB_unspecified:
+    return OMPC_MAP_tofrom;
+  }
----------------
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.


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

Reply via email to