cchen added a comment. In D84192#2167649 <https://reviews.llvm.org/D84192#2167649>, @ABataev wrote:
> In D84192#2167636 <https://reviews.llvm.org/D84192#2167636>, @cchen wrote: > > > In D84192#2165522 <https://reviews.llvm.org/D84192#2165522>, @ABataev wrote: > > > > > In D84192#2165517 <https://reviews.llvm.org/D84192#2165517>, @cchen wrote: > > > > > > > In D84192#2165438 <https://reviews.llvm.org/D84192#2165438>, @ABataev > > > > wrote: > > > > > > > > > In D84192#2165396 <https://reviews.llvm.org/D84192#2165396>, @cchen > > > > > wrote: > > > > > > > > > > > In D84192#2165249 <https://reviews.llvm.org/D84192#2165249>, > > > > > > @ABataev wrote: > > > > > > > > > > > > > In D84192#2165235 <https://reviews.llvm.org/D84192#2165235>, > > > > > > > @cchen wrote: > > > > > > > > > > > > > > > In D84192#2164885 <https://reviews.llvm.org/D84192#2164885>, > > > > > > > > @ABataev wrote: > > > > > > > > > > > > > > > > > Also, what about compatibility with declare mapper? Can you > > > > > > > > > add tests for it? > > > > > > > > > > > > > > > > > > > > > > > > There's a restriction for map clause that non-contiguous is not > > > > > > > > allowed and I guess it also applies to declare mapper. > > > > > > > > > > > > > > > > > > > > > I see that to/from clauses allow to use mappers too: > > > > > > > > > > > > > > to([mapper(mapper-identifier):]locator-list) > > > > > > > from([mapper(mapper-identifier):]locator-list > > > > > > > > > > > > > > > > > > > > > > > > > I'm confused of how to use > > > > > > to([mapper(mapper-identifier):]locator-list) with array section. > > > > > > For this case: > > > > > > > > > > > > class C { > > > > > > public: > > > > > > int a; > > > > > > double b[3][4][5]; > > > > > > }; > > > > > > > > > > > > #pragma omp declare mapper(id: C s) map(s.a, s.b[0:3][0:4][0:5]) > > > > > > > > > > > > #pragma omp target update from(mapper(id): c) > > > > > > > > > > > > > > > > > > Clang already has a semantic check in from clause when mapper is > > > > > > used: "mapper type must be of struct, union or class type". > > > > > > So the only item I can put in from clause in the above example is > > > > > > `c` and I cannot put c.b[0:2][1:2][0:2] or any even c.b[0:2]. > > > > > > > > > > > > > > > Does clang compile your example? If not, shall it be allowed for > > > > > to/from clauses or not? > > > > > > > > > > > > Clang can compile my example but the problem is that Clang do not > > > > accept something like `#pragma omp target update from(mapper(id): > > > > c.b[0:2][1:2][2:2])` (non-contiguous) or even `#pragma omp target > > > > update from(mapper(id): c.b[2][1][0:2])` (contiguous). > > > > Actually, Clang now only accepts `c` as struct/union/class type in > > > > `from(mapper(id): c)`. And the reason for the restriction is from > > > > declare mapper directive - "The type must be of struct, union or class > > > > type in C and C++". > > > > > > > > > And it is fine. How does it work in declare mapper, the main question? > > > Does it support extended array sections format mapoers with maps, to and > > > from? Shall it be supported in declare mapper at all? > > > > > > After discussing with @dreachem, my understanding is that it is not > > incorrect to not allowing extended/non-contiguous array section to appear > > in declare mapper. > > For the declare mapper directive, since spec only allows map clause, > > extended array section (with stride) or non-contiguous array section are > > both not allowed. > > For using the mapper in map/to/from clause, if mapping or updating an > > array section of type T, where there is a mapper declared for T, then the > > mapper needs to apply as if to each element of the array or array section. > > Spec is intentionally not sufficiently clear on this for target update so > > the semantic check in Clang is not incorrect. Which lead to the fact that I > > might not need to support extended/non-contiguous array section for declare > > mapper. > > > What exactly is incorrect in clang sema? I mean "not" incorrect for not allowing extended/non-contiguous array section in `to/from([mapper(mapper-identifier):] location-list). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84192/new/ https://reviews.llvm.org/D84192 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
