jdoerfert 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);
+ }
----------------
RaviNarayanaswamy wrote:
> 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
> 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
Could you please elaborate what needs to be changed exactly. What information
is currently not available in the setup as is? What properties would be
different?
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
