- User Since
- Nov 2 2017, 3:15 PM (155 w, 5 d)
Fri, Oct 16
Thu, Oct 15
Wed, Oct 14
Thanks for working on this. Sorry to take so long to review. Before I try to digest the code, I have a few high-level questions.
Tue, Oct 13
Thanks for working on this. I'm sorry to start reviewing so late. I haven't digested the implementation fully yet, but I have a few comments to get started.
Aug 20 2020
Aug 19 2020
Does this mean we officially no longer support python 2, which this change breaks?
Aug 11 2020
Aug 6 2020
Aug 5 2020
Don't examine LIBOMPTARGET_DEBUG=1 output in the test. It doesn't work in release builds.
Thanks for the quick reviews!
Add user diagnostic for mapping extension. Use PRId64 where appropriate.
Aug 4 2020
This patch fixes DeviceTy::getTgtPtrBegin to return null when
target_data_update is the caller. I'm wondering if it should do the
same for more callers.
Thanks for the quick review.
Jul 30 2020
Thanks for the review.
Jul 29 2020
Jul 28 2020
Thanks for the review.
Adjust the fixme as requested.
Should this patch be marked NFC?
Adjusted logic for rejecting present as requested. Rebased.
Replaced SeparateBeginEnd parameter with new TargetDataInfo field as requested. Rebased.
Are you following an existing TableGen emitter as a precedent for this kind of thin wrapper design?
Sure. Thanks for the reply.
I've pushed it to master as 9b4826d18b5f.
Jul 27 2020
not expects a non-zero exit not a signal. Adding --crash adjusts the expectation.
Rewrite patch as discussed: instead of generating different runtime calls for the end of an omp target data vs. the beginning of an omp target exit data so that the runtime can determine when to ignore present, change Clang to filter present from the map types at the end of an omp target data.
Other than the nit I just added about comment duplication, this LGTM. Thanks. However, let's a wait a couple of days in case anyone else has an opinion.
This change broke a number of libomptarget tests. Adding --crash to the %libomptarget-run-fail-* definitions in openmp/libomptarget/test/lit.cfg fixes that for me. Any objections?
Jul 24 2020
Thanks for working on this. I've been wanting to see this fixed too. I agree that lit is the culprit.
Jul 23 2020
I don't know if the OpenMP committee has any documented rationale for this behavior. I can say that the OpenACC committee is considering the same semantics. However, the issues to consider are not identical. For example, OpenACC has a separate structured reference counter, meaning it should be impossible for such data not to be present at the exit of a data construct unless you've shut down the runtime.