Skip to content

[OpenACC] Implement Duffs-Device restriction for Compute Constructs #83460

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 1 commit into from
Mar 1, 2024
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
19 changes: 19 additions & 0 deletions clang/include/clang/Sema/Scope.h
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,25 @@ class Scope {
return false;
}

/// Determine if this scope (or its parents) are a compute construct inside of
/// the nearest 'switch' scope. This is needed to check whether we are inside
/// of a 'duffs' device, which is an illegal branch into a compute construct.
bool isInOpenACCComputeConstructBeforeSwitch() const {
for (const Scope *S = this; S; S = S->getParent()) {
if (S->getFlags() & Scope::OpenACCComputeConstructScope)
return true;
if (S->getFlags() & Scope::SwitchScope)
return false;

if (S->getFlags() &
(Scope::FnScope | Scope::ClassScope | Scope::BlockScope |
Scope::TemplateParamScope | Scope::FunctionPrototypeScope |
Scope::AtCatchScope | Scope::ObjCMethodScope))
return false;
Comment on lines +544 to +551
Copy link
Member

Choose a reason for hiding this comment

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

Merge this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wasn't really sure how to be honest. it has slightly different logic, so without some flag to choose between the two behaviors, I couldn't come up with a way? And I wasn't sure that was better.

WDYT""

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I see, then better to keep it as is.

}
return false;
}

/// Determine whether this scope is a while/do/for statement, which can have
/// continue statements embedded into it.
bool isContinueScope() const {
Expand Down
14 changes: 14 additions & 0 deletions clang/lib/Sema/SemaStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,13 @@ Sema::ActOnCaseStmt(SourceLocation CaseLoc, ExprResult LHSVal,
return StmtError();
}

if (LangOpts.OpenACC &&
getCurScope()->isInOpenACCComputeConstructBeforeSwitch()) {
Diag(CaseLoc, diag::err_acc_branch_in_out_compute_construct)
<< /*branch*/ 0 << /*into*/ 1;
return StmtError();
}

auto *CS = CaseStmt::Create(Context, LHSVal.get(), RHSVal.get(),
CaseLoc, DotDotDotLoc, ColonLoc);
getCurFunction()->SwitchStack.back().getPointer()->addSwitchCase(CS);
Expand All @@ -546,6 +553,13 @@ Sema::ActOnDefaultStmt(SourceLocation DefaultLoc, SourceLocation ColonLoc,
return SubStmt;
}

if (LangOpts.OpenACC &&
getCurScope()->isInOpenACCComputeConstructBeforeSwitch()) {
Diag(DefaultLoc, diag::err_acc_branch_in_out_compute_construct)
<< /*branch*/ 0 << /*into*/ 1;
return StmtError();
}

DefaultStmt *DS = new (Context) DefaultStmt(DefaultLoc, ColonLoc, SubStmt);
getCurFunction()->SwitchStack.back().getPointer()->addSwitchCase(DS);
return DS;
Expand Down
27 changes: 27 additions & 0 deletions clang/test/SemaOpenACC/no-branch-in-out.c
Original file line number Diff line number Diff line change
Expand Up @@ -310,3 +310,30 @@ LABEL4:{}

ptr=&&LABEL5;
}

void DuffsDevice() {
int j;
switch (j) {
#pragma acc parallel
for(int i =0; i < 5; ++i) {
case 0: // expected-error{{invalid branch into OpenACC Compute Construct}}
{}
}
}

switch (j) {
#pragma acc parallel
for(int i =0; i < 5; ++i) {
default: // expected-error{{invalid branch into OpenACC Compute Construct}}
{}
}
}

switch (j) {
#pragma acc parallel
for(int i =0; i < 5; ++i) {
case 'a' ... 'z': // expected-error{{invalid branch into OpenACC Compute Construct}}
{}
}
}
}
30 changes: 29 additions & 1 deletion clang/test/SemaOpenACC/no-branch-in-out.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ void ReturnTest() {

template<typename T>
void BreakContinue() {

#pragma acc parallel
for(int i =0; i < 5; ++i) {
switch(i) {
Expand Down Expand Up @@ -109,6 +108,35 @@ void BreakContinue() {
} while (j );
}

template<typename T>
void DuffsDevice() {
int j;
switch (j) {
#pragma acc parallel
for(int i =0; i < 5; ++i) {
case 0: // expected-error{{invalid branch into OpenACC Compute Construct}}
{}
}
}

switch (j) {
#pragma acc parallel
for(int i =0; i < 5; ++i) {
default: // expected-error{{invalid branch into OpenACC Compute Construct}}
{}
}
}

switch (j) {
#pragma acc parallel
for(int i =0; i < 5; ++i) {
case 'a' ... 'z': // expected-error{{invalid branch into OpenACC Compute Construct}}
{}
}
}
}

void Instantiate() {
BreakContinue<int>();
DuffsDevice<int>();
}