[Openmp-commits] [PATCH] D82718: [OpenMP] Use primary context in CUDA plugin

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

Not overwriting the user/system choice is the right thing to do.

I'm fine with the warning in case there is a different option set in the existing context, but from what I understand there is no correctness but only a performance implication anyway.

This seems to have addressed all concerns and I doubt it will change the behavior on most systems anyway. On the ones it does, it is important to do so.

Thanks for the fix!

One nit, otherwise LGTM.



================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:420
+    unsigned int FormerPrimaryCtxFlags;
+    int FormerPrimaryCtxIsActive;
+    Err = cuDevicePrimaryCtxGetState(Device, &FormerPrimaryCtxFlags,
----------------
We should initialize these, I never trust runtime calls to do that for some reason.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82718/new/

https://reviews.llvm.org/D82718