[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

Joel E. Denny via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 28 14:33:02 PST 2018


jdenny added a comment.

In D56113#1341940 <https://reviews.llvm.org/D56113#1341940>, @ABataev wrote:

> The patch does not seem quite correct. I committed another fix for this problem.


Thanks for the fix, r350127, which does seem to address the use cases I care about right now.

However, you are still implementing predetermined shared for const variables.  Assuming OpenMP 4.0 or later, that produces misleading diagnostics, such as "error: shared variable cannot be lastprivate".  Moreover, default(none) doesn't have the expected effect on such variables.  Then again, neither of these issues seems like a severe problem, and your approach is more compliant with earlier OpenMP versions.  Does all that match your view?

> There is still the problem with the explicitly specified `shared` clause on `target teams` directive, but I don't think that it must cause some troubles. The variable still should not be transferred from device to the host, so it is fine to pass by value into inner teams regions. Though, generally speaking, it must be fixed.

Should there be a copy allocated on the device and shared among the teams?  By the way, removing const from the example doesn't avoid this bug, but splitting `omp target teams` into two directives does avoid it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56113





More information about the cfe-commits mailing list