Skip to content

[OpenACC][Sema] Implement warning for non-effective 'private' #149004

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.

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 2 commits into from
Jul 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -13478,6 +13478,12 @@ def err_acc_invalid_default_type
def err_acc_device_type_multiple_archs
: Error<"OpenACC 'device_type' clause on a 'set' construct only permits "
"one architecture">;
def warn_acc_var_referenced_lacks_op
: Warning<"variable of type %0 referenced in OpenACC '%1' clause does not "
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason you chose a warning instead of an error here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it is not 'ill-formed' by the standard, there is no requirement in the standard to have constructors/destructors/etc, as far as I can tell, so I left it as 'warning-as-default-error'.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for your clarification.

"have a %enum_select<AccVarReferencedReason>{%DefCtor{default "
"constructor}|%Dtor{destructor}}2; reference has no effect">,
InGroup<DiagGroup<"openacc-var-lacks-operation">>,
DefaultError;

// AMDGCN builtins diagnostics
def err_amdgcn_load_lds_size_invalid_value : Error<"invalid size value">;
Expand Down
70 changes: 65 additions & 5 deletions clang/lib/Sema/SemaOpenACC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,66 @@ void SemaOpenACC::CheckDeclReference(SourceLocation Loc, Expr *E, Decl *D) {
// loop (or we aren't in a loop!) so skip the diagnostic.
}

namespace {
// Check whether the type of the thing we are referencing is OK for things like
// private, firstprivate, and reduction, which require certain operators to be
// available.
ExprResult CheckVarType(SemaOpenACC &S, OpenACCClauseKind CK, Expr *VarExpr,
Expr *InnerExpr) {
// There is nothing to do here, only these three have these sorts of
// restrictions.
if (CK != OpenACCClauseKind::Private &&
CK != OpenACCClauseKind::FirstPrivate &&
CK != OpenACCClauseKind::Reduction)
return VarExpr;

// We can't test this if it isn't here, or if the type isn't clear yet.
if (!InnerExpr || InnerExpr->isTypeDependent())
return VarExpr;

const auto *RD = InnerExpr->getType()->getAsCXXRecordDecl();

// if this isn't a C++ record decl, we can create/copy/destroy this thing at
// will without problem, so this is a success.
if (!RD)
return VarExpr;

// TODO: OpenACC:
// Private must have default ctor + dtor in InnerExpr
// FirstPrivate must have copyctor + dtor in InnerExpr
// Reduction must have copyctor + dtor + operation in InnerExpr
Copy link
Contributor

Choose a reason for hiding this comment

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

Reduction likely does not require a copyctor since its reduction semantics are:

If the reduction var
1298 is a composite variable, the reduction operation is logically equivalent to applying that reduction
1299 operation to each member of the composite variable individually.

So the init region would not be either default or copy constructors.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would still need a 'default' constructor though to start its lifetime, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. This makes sense.


// TODO OpenACC: It isn't clear what the requirements are for default
// constructor/copy constructor are for First private and reduction, but
// private requires a default constructor.
if (CK == OpenACCClauseKind::Private) {
bool HasNonDeletedDefaultCtor =
llvm::find_if(RD->ctors(), [](const CXXConstructorDecl *CD) {
return CD->isDefaultConstructor() && !CD->isDeleted();
}) != RD->ctors().end();
if (!HasNonDeletedDefaultCtor && !RD->needsImplicitDefaultConstructor()) {
S.Diag(InnerExpr->getBeginLoc(),
clang::diag::warn_acc_var_referenced_lacks_op)
<< InnerExpr->getType() << CK
<< clang::diag::AccVarReferencedReason::DefCtor;
return ExprError();
}
}

// All 3 things need to make sure they have a dtor.
bool DestructorDeleted =
RD->getDestructor() && RD->getDestructor()->isDeleted();
if (DestructorDeleted && !RD->needsImplicitDestructor()) {
S.Diag(InnerExpr->getBeginLoc(),
clang::diag::warn_acc_var_referenced_lacks_op)
<< InnerExpr->getType() << CK
<< clang::diag::AccVarReferencedReason::Dtor;
return ExprError();
}
return VarExpr;
}
} // namespace

ExprResult SemaOpenACC::ActOnVar(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
Expr *VarExpr) {
// This has unique enough restrictions that we should split it to a separate
Expand Down Expand Up @@ -660,7 +720,7 @@ ExprResult SemaOpenACC::ActOnVar(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
if (const auto *DRE = dyn_cast<DeclRefExpr>(CurVarExpr)) {
if (isa<VarDecl, NonTypeTemplateParmDecl>(
DRE->getFoundDecl()->getCanonicalDecl()))
return VarExpr;
return CheckVarType(*this, CK, VarExpr, CurVarExpr);
}

// If CK is a Reduction, this special cases for OpenACC3.3 2.5.15: "A var in a
Expand All @@ -679,9 +739,9 @@ ExprResult SemaOpenACC::ActOnVar(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
// declare, reduction, and use_device.
const auto *This = dyn_cast<CXXThisExpr>(ME->getBase());
if (This && This->isImplicit())
return VarExpr;
return CheckVarType(*this, CK, VarExpr, CurVarExpr);
} else {
return VarExpr;
return CheckVarType(*this, CK, VarExpr, CurVarExpr);
}
}
}
Expand All @@ -690,14 +750,14 @@ ExprResult SemaOpenACC::ActOnVar(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
// doesn't fall into 'variable or array name'
if (CK != OpenACCClauseKind::UseDevice &&
DK != OpenACCDirectiveKind::Declare && isa<CXXThisExpr>(CurVarExpr))
return VarExpr;
return CheckVarType(*this, CK, VarExpr, CurVarExpr);

// Nothing really we can do here, as these are dependent. So just return they
// are valid.
if (isa<DependentScopeDeclRefExpr>(CurVarExpr) ||
(CK != OpenACCClauseKind::Reduction &&
isa<CXXDependentScopeMemberExpr>(CurVarExpr)))
return VarExpr;
return CheckVarType(*this, CK, VarExpr, CurVarExpr);

// There isn't really anything we can do in the case of a recovery expr, so
// skip the diagnostic rather than produce a confusing diagnostic.
Expand Down
103 changes: 103 additions & 0 deletions clang/test/SemaOpenACC/private_firstprivate_reduction_required_ops.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// RUN: %clang_cc1 %s -fopenacc -verify

struct ImplicitCtorDtor{};

struct ImplDeletedCtor{
ImplDeletedCtor(int i);
};

struct DefaultedCtor {
DefaultedCtor() = default;
};

struct ImpledCtor {
ImpledCtor() = default;
};


struct DeletedCtor {
DeletedCtor() = delete;
};

struct ImpledDtor {
~ImpledDtor();
};

struct DefaultedDtor {
~DefaultedDtor() = default;
};

struct DeletedDtor {
~DeletedDtor() = delete;
};

struct ImplicitDelDtor {
DeletedDtor d;
};

void private_uses(ImplicitCtorDtor &CDT, ImplDeletedCtor &IDC,
DefaultedCtor &DefC, ImpledCtor &IC, DeletedCtor &DelC,
ImpledDtor &ID, DefaultedDtor &DefD, DeletedDtor &DelD,
ImplicitDelDtor &IDD) {

#pragma acc parallel private(CDT)
;

// expected-error@+1{{variable of type 'ImplDeletedCtor' referenced in OpenACC 'private' clause does not have a default constructor; reference has no effect}}
#pragma acc parallel private(IDC)
;

#pragma acc parallel private(DefC)
;

#pragma acc parallel private(IC)
;

// expected-error@+1{{variable of type 'DeletedCtor' referenced in OpenACC 'private' clause does not have a default constructor; reference has no effect}}
#pragma acc parallel private(DelC)
;

#pragma acc parallel private(ID)
;

#pragma acc parallel private(DefD)
;

// expected-error@+1{{variable of type 'DeletedDtor' referenced in OpenACC 'private' clause does not have a destructor; reference has no effect}}
#pragma acc parallel private(DelD)
;

// expected-error@+1{{variable of type 'ImplicitDelDtor' referenced in OpenACC 'private' clause does not have a destructor; reference has no effect}}
#pragma acc parallel private(IDD)
;

}

template<typename T>
void private_templ(T& t) {
#pragma acc parallel private(t) // #PRIV
;
}

void inst(ImplicitCtorDtor &CDT, ImplDeletedCtor &IDC,
DefaultedCtor &DefC, ImpledCtor &IC, DeletedCtor &DelC,
ImpledDtor &ID, DefaultedDtor &DefD, DeletedDtor &DelD,
ImplicitDelDtor &IDD) {
private_templ(CDT);
// expected-error@#PRIV{{variable of type 'ImplDeletedCtor' referenced in OpenACC 'private' clause does not have a default constructor; reference has no effect}}
// expected-note@+1{{in instantiation}}
private_templ(IDC);
private_templ(DefC);
private_templ(IC);
// expected-error@#PRIV{{variable of type 'DeletedCtor' referenced in OpenACC 'private' clause does not have a default constructor; reference has no effect}}
// expected-note@+1{{in instantiation}}
private_templ(DelC);
private_templ(ID);
private_templ(DefD);
// expected-error@#PRIV{{variable of type 'DeletedDtor' referenced in OpenACC 'private' clause does not have a destructor; reference has no effect}}
// expected-note@+1{{in instantiation}}
private_templ(DelD);
// expected-error@#PRIV{{variable of type 'ImplicitDelDtor' referenced in OpenACC 'private' clause does not have a destructor; reference has no effect}}
// expected-note@+1{{in instantiation}}
private_templ(IDD);
}