[Openmp-commits] [PATCH] D64626: [libomptarget] Handle offload policy in push_target_tripcount
Jonas Hahnfeld via Phabricator via Openmp-commits
openmp-commits at lists.llvm.org
Mon Jul 22 23:19:15 PDT 2019
Hahnfeld added a comment.
Herald added a reviewer: jdoerfert.
In D64626#1596837 <https://reviews.llvm.org/D64626#1596837>, @jdoerfert wrote:
> In D64626#1596347 <https://reviews.llvm.org/D64626#1596347>, @Hahnfeld wrote:
> > In D64626#1596295 <https://reviews.llvm.org/D64626#1596295>, @jdoerfert wrote:
> > > In D64626#1596246 <https://reviews.llvm.org/D64626#1596246>, @Hahnfeld wrote:
> > >
> > > > In D64626#1596237 <https://reviews.llvm.org/D64626#1596237>, @jdoerfert wrote:
> > > >
> > > > > I do not understand what caused the error you linked. Which part of the push_tripcount call is problematic? I would have expected get_default_device to return the host id when offload is disabled and the code to not choke one the host device.
> > > >
> > > >
> > > > According to the spec `omp_get_default_device` returns //default-device-var// and I couldn't find a paragraph that implies special handling when offloading is disabled.
> > > >
> > > > Consequently, `omp_get_default_device` always returns a "real" device and if `CheckDeviceAndCtors` fails (in the linked case, it was because the user didn't compile for the right architecture), the call to `HandleTargetOutcome(false)` will result in the error message as seen in the linked post.
> > >
> > >
> > > So why not change `omp_get_default_device` to return the host device number if offloading is disabled? My reasoning is: Why should "offload disabled" be different from "no offload", e.g., because there are no devices available.
> > So you propose to change the //default-device-var// ICV? Because at the moment, I think `omp_get_default_device` returns the value of `OMP_DEFAULT_DEVICE` or 0, regardless of whether the device exists or not. The user can only find out via `omp_get_num_devices` which will return the correct value 0 in both cases.
> (I might not fully grasp the situation here, if so, feel free to present me a bigger picture).
> I actually did not try to suggest a change of any ICV value. Let me try to explain:
> My thought was that the following conditions should more or less result in the same execution:
> - offloading is disabled, e.g., through an environment variable,
> - offloading is not possible, e.g., there is no device,
> - offloading is not possible, e.g., there was a problem initializing the device.
Agreed, and it already does.
> Part of what confuses me now is:
>> I think omp_get_default_device returns the value of OMP_DEFAULT_DEVICE or 0, regardless of whether the device exists or not.
> I would have thought that `omp_get_default_device` returns the host device id if there is no other one available.
No, as I described above, the routine only returns the value of the ICV. This may be a device that doesn't exist, but `omp_get_default_device` doesn't care for this case.
>> [..] After all, setting `OMP_TARGET_OFFLOAD=disabled` should disable all of `libomptarget`.
> I agree. That is why I think adding a conditional to `__kmpc_push_target_tripcount` is not a conceptual fix of the issue. (Maybe I am wrong though).
> The "building blocks", or alternatively all entry points, of the library have to work fine when offloading is disabled. Fixing `__kmpc_push_target_tripcount`
> looks like a triage of one entry point.
All other entry points have this guard, `__kmpc_push_target_tripcount` is the only one where it's missing. Maybe I should have stated this upfront, sorry.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the Openmp-commits