[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

abhinavgaba accepted this revision. abhinavgaba added a comment. This revision is now accepted and ready to land. Thanks for the changes, Alexey. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86119/new/ https://reviews.llvm.org/D86119

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev marked 2 inline comments as done. ABataev added a comment. In D86119#2562187 , @grokos wrote: > In D86119#2561163 , @abhinavgaba > wrote: > >> Thanks for the changes, Alexey! I tried the patch locally, and

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

grokos added a comment. In D86119#2561163 , @abhinavgaba wrote: > Thanks for the changes, Alexey! I tried the patch locally, and it looks > stable. It handled several tests I tried, including the following case > involving array section on a pointer to p

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

abhinavgaba added a comment. Thanks for the changes, Alexey! I tried the patch locally, and it looks stable. It handled several tests I tried, including the following case involving array section on a pointer to pointer base, and nested mappers with `PTR_AND_OBJ` maps successfully: #include

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

cchen added a comment. Below test asserts inside `generateInfoForComponentList`: Assertion failed: (!IsPointer && "Unexpected base element with the pointer type."), function generateInfoForComponentList, file /Users/cchen/workspace/llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp, line 775

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added a comment. In D86119#2437676 , @cchen wrote: > Just tried this patch and found that below test abort inside > `generateInfoCapture`. > > #include > #define LEN 100 > int buf[LEN]; > > int main() > { > int i; > int *

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

cchen added a comment. Just tried this patch and found that below test abort inside `generateInfoCapture`. #include #define LEN 100 int buf[LEN]; int main() { int i; int *p = buf; for (i = 0; i < LEN; i++) { p[i] = 0; } #pragma omp target m

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added a comment. Entry 0 is the address that should be passed to the kernel (for the captured variable, that's why it is marked as TGT_TARGET_PARAM - target kernel argument), entries 1-3 are the actual mappings. Yes, I assume the frontend can fuse these entries in many cases, but it is

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added a comment. In D86119#2402661 , @ye-luo wrote: > 1. Could you separate the reordering related changes to separate patch? It is impossible. There is a separate patch for removing extra TGT_TARGET_PARAM but it cannot be separated from this pat

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ye-luo added a comment. 1. Could you separate the reordering related changes to separate patch? 2. Could you mention which line in spec 4.5 was the restriction? Even 5.0/5.1 has some restrictions. Need to be clear which one you refer to. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

JonChesterfield added a comment. If data mappings can overlap, then it follows that copies to/from the target must be done sequentially by the runtime, unless additional information on their independence exists. Alias analysis style. I see the programmer convenience angle, but it is a shame to

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Ma

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

grokos added inline comments. Comment at: openmp/libomptarget/src/omptarget.cpp:233 MapperComponents -.Components[target_data_function == targetDataEnd ? I : E - I - 1]; +.Components[target_data_function == targetDataEnd ? E - I - 1 : I]; Map

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added a comment. In D86119#2330601 , @jdenny wrote: > In D86119#2330339 , @ABataev wrote: > >> In D86119#2330310 , @jdenny wrote: >> >>> Thanks for working on this.

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

jdenny added a comment. In D86119#2330339 , @ABataev wrote: > In D86119#2330310 , @jdenny wrote: > >> Thanks for working on this. Sorry to take so long to review. Before I try >> to digest the code, I have a few

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

ABataev added a comment. In D86119#2330310 , @jdenny wrote: > Thanks for working on this. Sorry to take so long to review. Before I try > to digest the code, I have a few high-level questions. > > Based on the test suite changes, `TARGET_PARAM` is disap

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

jdenny added a comment. Thanks for working on this. Sorry to take so long to review. Before I try to digest the code, I have a few high-level questions. Based on the test suite changes, `TARGET_PARAM` is disappearing from many cases. Can you explain a bit how that supports overlapping and re