jdenny marked an inline comment as done. jdenny added a comment. In D84422#2173449 <https://reviews.llvm.org/D84422#2173449>, @RaviNarayanaswamy wrote:
> In D84422#2173372 <https://reviews.llvm.org/D84422#2173372>, @jdenny wrote: > > > In D84422#2172898 <https://reviews.llvm.org/D84422#2172898>, @jdenny wrote: > > > > > Has anyone clarified the motivation for this behavior? > > > > > > I meant, is there any insight into why the spec specifies this behavior? > > > > In D84422#2172926 <https://reviews.llvm.org/D84422#2172926>, @grokos wrote: > > > > > Instead of introducing new API functions and making all these changes in > > > all these files, wouldn't it be easier if we just unset the `PRESENT` > > > flag from arg_types in clang when we generate the call to > > > `__tgt_target_data_end_*` if we are exiting from a scoped environment? > > > > > > Ah, that does sound simpler. Thanks. I'll look into it. > > > > Suppressing the presence check on exit from `omp target` would require a > > runtime change in addition to the Clang change you suggest for `omp target > > data`. However, I've so far failed to formulate a reasonable test case. > > Specifically, I don't yet see a way to guarantee that the data will > > definitely be present at the start of `omp target` but might not be present > > by the end. Is it possible? If not, then maybe we should leave the check > > in place for `omp target`. > > > I would rather not have a check if not required by the spec as it would an > unnecessary overhead to performance. I've added a comment to the runtime code that performs the check. As you can see, the check is performed regardless. It's just a question of whether the runtime treats it as an error. I don't think performance is an issue. My concern here is that it will be hard to justify changes to the runtime if I cannot formulate a use case. ================ Comment at: openmp/libomptarget/src/omptarget.cpp:511 + // "omp target exit data" but not upon exiting an "omp target data". + if (HasPresentModifier && for_exit_data) { MESSAGE("device mapping required by 'present' map type modifier does " ---------------- This is where the runtime performs the check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84422/new/ https://reviews.llvm.org/D84422 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
