Skip to content

[clang][OpenMP] 6.0: detect privatization of array section/assumed-size array #152786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 3, 2025
Merged

[clang][OpenMP] 6.0: detect privatization of array section/assumed-size array #152786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1fa5cc
[clang][OpenMP] 6.0: detect privatization of array section/assumed-si…
ddpagan Jul 29, 2025
f50173b
Updated test to verify error detection with fully templated type.
ddpagan Aug 27, 2025
00e8520
Update OpenMP support page and clang release notes.
ddpagan Sep 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion clang/docs/OpenMPSupport.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,8 @@ implementation.
+-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+
| Traits for default device envirable | :none:`unclaimed` | :none:`unclaimed` | |
+-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+
| Optionally omit array length expression | :good:`done` | :none:`unclaimed` | https://github.com/llvm/llvm-project/pull/148048 |
| Optionally omit array length expression | :good:`done` | :none:`unclaimed` | (Parse) https://github.com/llvm/llvm-project/pull/148048, |
| | | | (Sema) https://github.com/llvm/llvm-project/pull/152786 |
Comment on lines +446 to +447
Copy link
Member

@alexey-bataev alexey-bataev Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it is unclaimed?

Copy link
Contributor Author

@ddpagan ddpagan Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the current status of Fortran. The "done" status applies to C/C++.

+-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+
| Canonical loop sequences | :none:`unclaimed` | :part:`In Progress` | |
+-------------------------------------------------------------+---------------------------+---------------------------+--------------------------------------------------------------------------+
Expand Down
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ OpenMP Support
modifier in the ``adjust_args`` clause.
- Allow array length to be omitted in array section subscript expression.
- Fixed non-contiguous strided update in the ``omp target update`` directive with the ``from`` clause.
- Properly handle array section/assumed-size array privatization in C/C++.

Improvements
^^^^^^^^^^^^
Expand Down
46 changes: 36 additions & 10 deletions clang/lib/Sema/SemaOpenMP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2780,7 +2780,7 @@ void SemaOpenMP::EndOpenMPClause() {
static std::pair<ValueDecl *, bool>
getPrivateItem(Sema &S, Expr *&RefExpr, SourceLocation &ELoc,
SourceRange &ERange, bool AllowArraySection = false,
StringRef DiagType = "");
bool AllowAssumedSizeArray = false, StringRef DiagType = "");

/// Check consistency of the reduction clauses.
static void checkReductionClauses(Sema &S, DSAStackTy *Stack,
Expand Down Expand Up @@ -5148,11 +5148,10 @@ static bool checkIfClauses(Sema &S, OpenMPDirectiveKind Kind,
return ErrorFound;
}

static std::pair<ValueDecl *, bool> getPrivateItem(Sema &S, Expr *&RefExpr,
SourceLocation &ELoc,
SourceRange &ERange,
bool AllowArraySection,
StringRef DiagType) {
static std::pair<ValueDecl *, bool>
getPrivateItem(Sema &S, Expr *&RefExpr, SourceLocation &ELoc,
SourceRange &ERange, bool AllowArraySection,
bool AllowAssumedSizeArray, StringRef DiagType) {
if (RefExpr->isTypeDependent() || RefExpr->isValueDependent() ||
RefExpr->containsUnexpandedParameterPack())
return std::make_pair(nullptr, true);
Expand All @@ -5162,6 +5161,20 @@ static std::pair<ValueDecl *, bool> getPrivateItem(Sema &S, Expr *&RefExpr,
// OpenMP [2.9.3.3, Restrictions, p.1]
// A variable that is part of another variable (as an array or
// structure element) cannot appear in a private clause.
//
// OpenMP [6.0]
// 5.2.5 Array Sections, p. 166, L28-29
// When the length is absent and the size of the dimension is not known,
// the array section is an assumed-size array.
// 2 Glossary, p. 23, L4-6
// assumed-size array
// For C/C++, an array section for which the length is absent and the
// size of the dimensions is not known.
// 5.2.5 Array Sections, p. 168, L11
// An assumed-size array can appear only in clauses for which it is
// explicitly allowed.
// 7.4 List Item Privatization, Restrictions, p. 222, L15
// Assumed-size arrays must not be privatized.
RefExpr = RefExpr->IgnoreParens();
enum {
NoArrayExpr = -1,
Expand All @@ -5177,6 +5190,17 @@ static std::pair<ValueDecl *, bool> getPrivateItem(Sema &S, Expr *&RefExpr,
IsArrayExpr = ArraySubscript;
} else if (auto *OASE = dyn_cast_or_null<ArraySectionExpr>(RefExpr)) {
Expr *Base = OASE->getBase()->IgnoreParenImpCasts();
if (S.getLangOpts().OpenMP >= 60 && !AllowAssumedSizeArray &&
OASE->getColonLocFirst().isValid() && !OASE->getLength()) {
QualType BaseType = ArraySectionExpr::getBaseOriginalType(Base);
if (BaseType.isNull() || (!BaseType->isConstantArrayType() &&
!BaseType->isVariableArrayType())) {
Comment on lines +5196 to +5197
Copy link
Member

@alexey-bataev alexey-bataev Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can it be templated type?

Copy link
Contributor Author

@ddpagan ddpagan Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The array section type check is done during instantiation, so templated type arrays/section references are handled properly in that sense. There are some checks for this in the updated tests.

Is this what you are referring to here?

Copy link
Member

@alexey-bataev alexey-bataev Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if it is not known yet what is the type, if it is fully dependent type? Will it work correctly or emit error too early?

Copy link
Contributor Author

@ddpagan ddpagan Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification here, Alexey. I'll verify how this behaves.

Copy link
Contributor Author

@ddpagan ddpagan Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Alexey - I've tried various cases and I'm not seeing any issues. I'm concerned that I'm not coming up with a proper test case that addresses your question. Do you have a specific example I can use, or start with, that would create this possibility?

Copy link
Contributor Author

@ddpagan ddpagan Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexey-bataev - Just following up. Is there an example you can share so that I can verify this case? As I noted above, I wasn't able to cause any failure/unexpected behavior. Apologies if I'm missing something obvious here.

Copy link
Member

@alexey-bataev alexey-bataev Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to check if it works in the templates

Copy link
Contributor Author

@ddpagan ddpagan Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Alexey. This does work correctly in templates and is verified in the updated test cases that are part of this pull request.

Copy link
Member

@alexey-bataev alexey-bataev Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it work if BaseType is a fully templated type, i.e., has no info that it is the array?

template <typename T>
int foo(T arr) {
   #pragma omp .... (arr)
}

Copy link
Contributor Author

@ddpagan ddpagan Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for that example, Alexey. Yes, it does work. However, this case was not checked in any of the test updates I made, so I'll go ahead and add one.

S.Diag(OASE->getColonLocFirst(),
diag::err_omp_section_length_undefined)
<< (!BaseType.isNull() && BaseType->isArrayType());
return std::make_pair(nullptr, false);
}
}
while (auto *TempOASE = dyn_cast<ArraySectionExpr>(Base))
Base = TempOASE->getBase()->IgnoreParenImpCasts();
while (auto *TempASE = dyn_cast<ArraySubscriptExpr>(Base))
Expand Down Expand Up @@ -17320,9 +17344,10 @@ static bool isValidInteropVariable(Sema &SemaRef, Expr *InteropVarExpr,
SourceLocation ELoc;
SourceRange ERange;
Expr *RefExpr = InteropVarExpr;
auto Res =
getPrivateItem(SemaRef, RefExpr, ELoc, ERange,
/*AllowArraySection=*/false, /*DiagType=*/"omp_interop_t");
auto Res = getPrivateItem(SemaRef, RefExpr, ELoc, ERange,
/*AllowArraySection=*/false,
/*AllowAssumedSizeArray=*/false,
/*DiagType=*/"omp_interop_t");

if (Res.second) {
// It will be analyzed later.
Expand Down Expand Up @@ -23506,7 +23531,8 @@ SemaOpenMP::ActOnOpenMPUseDeviceAddrClause(ArrayRef<Expr *> VarList,
SourceRange ERange;
Expr *SimpleRefExpr = RefExpr;
auto Res = getPrivateItem(SemaRef, SimpleRefExpr, ELoc, ERange,
/*AllowArraySection=*/true);
/*AllowArraySection=*/true,
/*AllowAssumedSizeArray=*/true);
if (Res.second) {
// It will be analyzed later.
MVLI.ProcessedVarList.push_back(RefExpr);
Expand Down
32 changes: 29 additions & 3 deletions clang/test/OpenMP/scan_messages.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,14 @@

// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 150 %s

// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=60 -ferror-limit 150 %s

// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=60 -ferror-limit 150 %s

template <class T>
T tmain() {
T tfoobar(T ub[]) {
static T argc;
T *ptr;
#pragma omp for
for (int i = 0; i < 10; ++i) {
#pragma omp scan // expected-error {{exactly one of 'inclusive' or 'exclusive' clauses is expected}}
Expand Down Expand Up @@ -83,12 +88,23 @@ T tmain() {
label1 : {
#pragma omp scan inclusive(argc) // expected-error {{orphaned 'omp scan' directives are prohibited; perhaps you forget to enclose the directive into a for, simd, for simd, parallel for, or parallel for simd region?}}
}}
#pragma omp simd reduction(inscan, +: argc)
for (int i = 0; i < 10; ++i) {
#pragma omp scan inclusive(ptr[0:], argc) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
;
}
#pragma omp simd reduction(inscan, +: argc)
for (int i = 0; i < 10; ++i) {
#pragma omp scan exclusive(argc, ub[:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}}
;
}

return T();
}

int main() {
int foobar(int ub[]) {
static int argc;
int *ptr;
#pragma omp simd reduction(inscan, +: argc)
for (int i = 0; i < 10; ++i) {
#pragma omp scan inclusive(argc) inclusive(argc) // expected-error {{exactly one of 'inclusive' or 'exclusive' clauses is expected}}
Expand Down Expand Up @@ -177,6 +193,16 @@ label1 : {
#pragma omp scan inclusive(argc) // expected-error {{orphaned 'omp scan' directives are prohibited; perhaps you forget to enclose the directive into a for, simd, for simd, parallel for, or parallel for simd region?}}
}
}
#pragma omp simd reduction(inscan, +: argc)
for (int i = 0; i < 10; ++i) {
#pragma omp scan inclusive(ptr[0:], argc) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
;
}
#pragma omp simd reduction(inscan, +: argc)
for (int i = 0; i < 10; ++i) {
#pragma omp scan exclusive(argc, ub[:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is an array of unknown bound}}
;
}

return tmain<int>(); // expected-note {{in instantiation of function template specialization 'tmain<int>' requested here}}
return tfoobar<int>(ub); // expected-note {{in instantiation of function template specialization 'tfoobar<int>' requested here}}
}
Loading
Loading