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

[Ye Luo via Phabricator via cfe-commits]

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];
     MapperArgsBase[I] = C.Base;
----------------
ABataev wrote:
> ABataev wrote:
> > ye-luo wrote:
> > > ye-luo wrote:
> > > > ABataev wrote:
> > > > > ye-luo wrote:
> > > > > > ABataev wrote:
> > > > > > > grokos wrote:
> > > > > > > > What is the current status of the order of the arguments clang emits? Is it still necessary to traverse arguments in reverse order here?
> > > > > > > Yes, still required
> > > > > > Based on the conversation in
> > > > > > https://reviews.llvm.org/D85216
> > > > > > This line of code neither before nor after the change plays well.
> > > > > > Shall we fix the order in targetDataEnd first?
> > > > > This change is part of this patch and cannot be committed separately. 
> > > > I mean could you fix that issue as a parent of this patch?
> > > > This change is part of this patch and cannot be committed separately. 
> > > 
> > > If fixing the reordering is part of this patch, I should have seen "target_data_function == targetDataEnd ?" branches disappear.
> > Nope, just with this patch. It reorders the maps and need to change the cleanup order too. 
> It works just like constructors/destructors: allocate in direct order, deallocate in reversed to correctly handle map order. 
The description says that "present and alloc mappings are processed first and then all others."
Why the order of arguments in targetDataBegin, targetDataEnd and targetDataUpdate all get reversed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86119