ABataev added a comment. In D86119#2330601 <https://reviews.llvm.org/D86119#2330601>, @jdenny wrote:
> In D86119#2330339 <https://reviews.llvm.org/D86119#2330339>, @ABataev wrote: > >> In D86119#2330310 <https://reviews.llvm.org/D86119#2330310>, @jdenny wrote: >> >>> 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. >>> >>> Based on the test suite changes, `TARGET_PARAM` is disappearing from many >>> cases. Can you explain a bit how that supports overlapping and reordering? >> >> `TARGET_PARAM` is used only for marking the data that should be passed to >> the kernel function as an argument. We just generate it in many cases but >> the runtime actually does not use them. Thу patch relies on this thing, >> otherwise, we may pass an incorrect number of arguments to the kernel >> functions. > > Is it reasonable to extract that change into a parent patch? That would at > least make the test suite changes easier to follow. I'll see what I can do about it. >>> Have you considered issue 2337 for the OpenMP spec and how your >>> implementation handles the ambiguous cases cited there? >> >> Can you provide the details about this issue? > > It discusses ambiguities in the way the spec describes map clause ordering. > Here's one example relevant to `present`: > > #pragma omp target exit data map(from: x) map(present, release: x) > > One statement in the spec says a map clause with `from` is effectively > ordered before one with `release`. Another statement says a map clause with > `present` is effectively ordered before one without. Which applies in the > above example? In some discussions, people agreed the statement about > `present` was intended to have precedence, but the spec doesn't say that. > That resolution probably makes sense at entry to a region, but does it make > sense at exit? Would it suppress `from` in this example? (Actually, should > there be two reference counter decrements in this example or just one?) > > These ambiguities are the reason I punted on this issue when implementing > `present`. If we can come up with a reasonable implementation for all cases, > perhaps we can propose a new wording for the spec. In tgis implementation, the mapping withthe present modifier will be ordered to be the first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86119/new/ https://reviews.llvm.org/D86119 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
