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

Joel E. Denny via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 27 14:59:16 PST 2018


jdenny created this revision.
jdenny added a reviewer: ABataev.
Herald added a subscriber: guansong.

The following appears in OpenMP 3.1 sec. 2.9.1.1 as a predetermined
data-sharing attribute:

> Variables with const-qualified type having no mutable member are
>  shared.

It does not appear in OpenmP 4.0, 4.5, or 5.0.  This patch removes the
implementation of that attribute.

Without this patch, if such a variable was explicitly lastprivate or
private, it was an error because it was predetermined shared.  This
patch leaves such variables as errors because their private copies can
never be initialized and because lastprivate cannot assign back to the
original.  However, instead of complaining that they're predetermined
shared, clang now complains that they're const without mutable fields.

An important effect of this patch is that it eliminates some bugs that
were caused by the removed implementation.  For example:

  void foo() {
    const int n;
    #pragma omp target teams firstprivate(n)
    #pragma omp parallel shared(n)
    (void)n;
  }

Bug 1: At the shared clause, clang used to complain "error:
firstprivate variable cannot be shared", implying n was predetermined
firstprivate simply because it was explicitly firstprivate on the
enclosing directive.  If we drop the explicit shared clause, clang
handled n as shared on the `omp parallel` anyway, demonstrating that
the error for the explicit shared clause was at least worded
incorrectly.  With this patch, both cases compile successfully with n
as (explicitly or implicitly) shared on the `omp parallel`.

Bug 2: Switching the shared and firstprivate clauses in the example
above used to produce an assert failure as follows.  Clang's search
for variable uses in the `omp target teams` block found n used in the
`omp parallel` clauses.  Clang then looked up n's existing
data-sharing attribute, incorrectly identified it as predetermined
shared instead of explicitly shared, incorrectly handled that as
simply no explicit data-sharing attribute, and so concluded that n
should be implicitly firstprivate instead.  That conflicted with n's
explicit shared clause on the `omp target teams`, producing the assert
failure.  With this patch, that case instead compiles successfully.
(If we split `omp target teams` into two directives, this bug didn't
occur, so the new test case exercises both combined and uncombined
cases.)

Question: For the scenario described above for bug 2, even if we
remove the const, clang implements shared(n) on `omp target teams` as
firstprivate instead of shared (passes by value instead of pointer).
Is this wrong?  The new test case has FIXMEs marking code that
demonstrates this behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D56113

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_messages.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_messages.cpp
  clang/test/OpenMP/distribute_private_messages.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_messages.cpp
  clang/test/OpenMP/for_lastprivate_messages.cpp
  clang/test/OpenMP/for_simd_lastprivate_messages.cpp
  clang/test/OpenMP/parallel_for_lastprivate_messages.cpp
  clang/test/OpenMP/parallel_for_simd_lastprivate_messages.cpp
  clang/test/OpenMP/parallel_private_messages.cpp
  clang/test/OpenMP/parallel_sections_lastprivate_messages.cpp
  clang/test/OpenMP/sections_lastprivate_messages.cpp
  clang/test/OpenMP/shared_for_const_not_mutable.cpp
  clang/test/OpenMP/simd_lastprivate_messages.cpp
  clang/test/OpenMP/target_parallel_for_lastprivate_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_lastprivate_messages.cpp
  clang/test/OpenMP/target_parallel_private_messages.cpp
  clang/test/OpenMP/target_simd_lastprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_lastprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_lastprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_private_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_lastprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_simd_private_messages.cpp
  clang/test/OpenMP/target_teams_distribute_private_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_lastprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_private_messages.cpp
  clang/test/OpenMP/target_teams_private_messages.cpp
  clang/test/OpenMP/task_private_messages.cpp
  clang/test/OpenMP/taskloop_lastprivate_messages.cpp
  clang/test/OpenMP/taskloop_simd_lastprivate_messages.cpp
  clang/test/OpenMP/teams_distribute_lastprivate_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_lastprivate_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_private_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_lastprivate_messages.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_simd_private_messages.cpp
  clang/test/OpenMP/teams_distribute_private_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_lastprivate_messages.cpp
  clang/test/OpenMP/teams_distribute_simd_private_messages.cpp
  clang/test/OpenMP/teams_private_messages.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D56113.179586.patch
Type: text/x-patch
Size: 133685 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20181227/0b40bd1e/attachment-0001.bin>


More information about the cfe-commits mailing list