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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to