RaviNarayanaswamy added inline comments.
================ Comment at: openmp/libomptarget/src/interop.cpp:198-201 + if (interop_type == kmp_interop_type_tasksync) { + __kmpc_omp_wait_deps(loc_ref, gtid, ndeps, dep_list, ndeps_noalias, + noalias_dep_list); + } ---------------- jdoerfert wrote: > RaviNarayanaswamy wrote: > > jdoerfert wrote: > > > RaviNarayanaswamy wrote: > > > > Interop object does not wait for any task from libomp. > > > > Init just initializes the interop object. > > > > The initialization of interop object should be done by the plugin as > > > > only the plugin knows what properties are supported > > > > Interop object does not wait for any task from libomp. > > > > > > I don't know why you think we would not wait for libomp tasks. If we have > > > dependences we need to wait for them. > > > > > > > The initialization of interop object should be done by the plugin as > > > > only the plugin knows what properties are supported. > > > > > > It is, below. This is the generic part that then redirects to the plugin. > > Libomp would have not invoked the task which calls this routine if there > > are dependences. They must be executed before the task containing the > > interop creation is scheduled. > > > > The interop_type should be passed to plugin and let it initialize the > > common for all interop-types and then add based on the interop_type > > Libomp would have not invoked the task which calls this routine if there > > are dependences. They must be executed before the task containing the > > interop creation is scheduled. > > To me it seems you are assuming that we create a task in which this routine > is called. We do not, as far as I can tell. See D105876. > > > The interop_type should be passed to plugin and let it initialize the > > common for all interop-types and then add based on the interop_type > > So what you are trying to say is that `init_device_info` should take the > `interop_type` too? That makes sense to me. But as discussed in other reviews > recently, we should not extend the API for "future use cases" but extend it > as use cases become relevant. For now it seems we can simply set up the > `tgt_device_info` part of the `omp_interop_val_t` without knowing the > `interop_type`. Then need to change this code since the interop_type can be both target_sync and target which will not be handled correctly. target_sync and target have common initialization + additional property base don the interop_type requested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106674/new/ https://reviews.llvm.org/D106674 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits