[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

Fri Nov 8 07:22:14 PST 2019

ABataev added inline comments.

================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:4435-4437
+        if (DMIB == DMIB_alloc) Kind = OMPC_MAP_alloc;
+        else if (DMIB == DMIB_to) Kind = OMPC_MAP_to;
+        else if (DMIB == DMIB_from) Kind = OMPC_MAP_from;
----------------
cchen wrote:
> ABataev wrote:
> > Use `switch`
> In this switch statement, I checked if the implicit map contains any "declare target link" to prove that for the normal case (no declare target link/to), DMIB_default and DMIB_default is unreachable for scalar and pointer.
> However, this change is not quite right since I haven't added any extra ImplicitMap to deal with the case that ImplicitMap contains "declare target link" variable. (instead, I only create an extra field in ImplicitMap only so that I can demonstrate) And the reason why I'm hesitant to do so is that adding another two ImplicitMap only for "declare target link" might be a little be overkill?
Then, I think, you just use the wrong key for the implicit mapping. You're using the kind of the mapped data (scalar, pointer or aggregate), instead used kind of mapping as the key. It means, that you need to have not 3 arrays but arrays for firstprivates, tofrom, to, from, alloc, etc. And all this processing must be in one place, in the class.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204