Skip to content

[OpenMP] Missing implicit otherwise clause in metadirective. #127113

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 5 commits into from
Feb 28, 2025
+163 −1
Merged

[OpenMP] Missing implicit otherwise clause in metadirective. #127113

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1995b54
[OpenMP] Missing implicit otherwise clause in metadirective.
zahiraam Feb 13, 2025
7fa46c6
Fix LIT test.
zahiraam Feb 13, 2025
ab69f1e
Addressed review comments.
zahiraam Feb 17, 2025
c5160c0
Addressed review comment.
zahiraam Feb 25, 2025
9b4b94a
Added an assert.
zahiraam Feb 27, 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
7 changes: 7 additions & 0 deletions clang/lib/Parse/ParseOpenMP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2882,6 +2882,13 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
/*ReadDirectiveWithinMetadirective=*/true);
break;
}
// If no match is found and no otherwise clause is present, skip
// OMP5.2 Chapter 7.4: If no otherwise clause is specified the effect is as
// if one was specified without an associated directive variant.
if (BestIdx == -1 && Idx == 1) {
ConsumeAnnotationToken();
return StmtEmpty();
}
Copy link
Member

@alexey-bataev alexey-bataev Feb 27, 2025

Choose a reason for hiding this comment

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

Do you have a test for the situation where you have extra tokens at this point? Or only tok::annot_pragma_openmp_end is expected here?

Copy link
Contributor Author

@zahiraam zahiraam Feb 27, 2025

Choose a reason for hiding this comment

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

Line #89 of OpenMP/metadirective_ast_print.c show this situation.

Copy link
Member

@alexey-bataev alexey-bataev Feb 27, 2025

Choose a reason for hiding this comment

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

Could you add an assertion here, that the next expected token is tok::annot_pragma_openmp_end?

Copy link
Contributor Author

@zahiraam zahiraam Feb 27, 2025

Choose a reason for hiding this comment

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

The next expected token should be kw_for no?

Copy link
Member

@alexey-bataev alexey-bataev Feb 27, 2025

Choose a reason for hiding this comment

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

I mean, before ConsumeAnnotationToken();

Copy link
Contributor Author

@zahiraam zahiraam Feb 27, 2025

Choose a reason for hiding this comment

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

In all cases I am seeing, at this point Tok=annot_pragma_openmp_end and NextToken = kw_for. Asserting here would not work! But doesn't it make sense to have the kw_for right at the end of the pragma?

Copy link
Member

@alexey-bataev alexey-bataev Feb 27, 2025

Choose a reason for hiding this comment

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

Yes, check that Tok=annot_pragma_openmp_end, this is what I meant

break;
}
case OMPD_threadprivate: {
Expand Down
28 changes: 28 additions & 0 deletions clang/test/OpenMP/metadirective_ast_print.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,34 @@ void foo(void) {
: parallel) default(nothing)
for (int i = 0; i < 16; i++)
;

#pragma omp metadirective when(user = {condition(0)} \
: parallel for) otherwise()
for (int i=0; i<10; i++)
;
#pragma omp metadirective when(user = {condition(0)} \
: parallel for)
for (int i=0; i<10; i++)
;
#pragma omp metadirective when(user = {condition(0)} \
: parallel for) when(implementation = {extension(match_none)} \
: parallel) default(parallel for)
for (int i=0; i<10; i++)
;

#pragma omp metadirective when(user = {condition(1)} \
: parallel for) otherwise()
for (int i=0; i<10; i++)
;
#pragma omp metadirective when(user = {condition(1)} \
: parallel for)
for (int i=0; i<10; i++)
;
#pragma omp metadirective when(user = {condition(1)} \
: parallel for) when(implementation = {extension(match_none)} \
: parallel) default(parallel for)
for (int i=0; i<10; i++)
;
}

// CHECK: void bar(void);
Expand Down
125 changes: 125 additions & 0 deletions clang/test/OpenMP/metadirective_otherwise.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple x86_64-unknown-linux -emit-llvm %s -o - | FileCheck %s
// expected-no-diagnostics

#ifndef HEADER
#define HEADER

void func1() {
#pragma omp metadirective when(user = {condition(0)} \
: parallel for) otherwise()
for (int i = 0; i < 100; i++)
;

#pragma omp metadirective when(user = {condition(0)} \
: parallel for)
for (int i = 0; i < 100; i++)
;

#pragma omp metadirective when(user = {condition(0)} \
: parallel for) \
when(implementation = {extension(match_none)} \
: parallel) default(parallel for)

for (int i = 0; i < 100; i++)
;


}

// CHECK-LABEL: define dso_local void @_Z5func1v()
// CHECK: entry
// CHECK-NEXT: [[I:%.*]] = alloca i32, align 4
// CHECK-NEXT: [[I1:%.*]] = alloca i32, align 4
// CHECK-NEXT: store i32 0, ptr [[I]], align 4
// CHECK-NEXT: br label %[[FOR_COND:.*]]
// CHECK: [[FOR_COND]]:
// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[I]], align 4
// CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[TMP0]], 100
// CHECK-NEXT: br i1 [[CMP]], label %[[FOR_BODY:.*]], label %[[FOR_END:.*]]
// CHECK: [[FOR_BODY]]:
// CHECK-NEXT: br label %[[FOR_INC:.*]]
// CHECK: [[FOR_INC]]:
// CHECK-NEXT: [[TMP1:%.*]] = load i32, ptr [[I]], align 4
// CHECK-NEXT: [[INC:%.*]] = add nsw i32 [[TMP1]], 1
// CHECK-NEXT: store i32 [[INC]], ptr [[I]], align 4
// CHECK-NEXT: br label %[[FOR_COND]], !llvm.loop [[LOOP3:![0-9]+]]
// CHECK: [[FOR_END]]:
// CHECK-NEXT: store i32 0, ptr [[I1]], align 4
// CHECK-NEXT: br label %[[FOR_COND2:.*]]
// CHECK: [[FOR_COND2]]:
// CHECK-NEXT: [[TMP2:%.*]] = load i32, ptr [[I1]], align 4
// CHECK-NEXT: [[CMP3:%.*]] = icmp slt i32 [[TMP2]], 100
// CHECK-NEXT: br i1 [[CMP3]], label %[[FOR_BODY4:.*]], label %[[FOR_END7:.*]]
// CHECK: [[FOR_BODY4]]:
// CHECK-NEXT: br label %[[FOR_INC5:.*]]
// CHECK: [[FOR_INC5]]:
// CHECK-NEXT: [[TMP3:%.*]] = load i32, ptr [[I1]], align 4
// CHECK-NEXT: [[INC6:%.*]] = add nsw i32 [[TMP3]], 1
// CHECK-NEXT: store i32 [[INC6]], ptr [[I1]], align 4
// CHECK-NEXT: br label %[[FOR_COND2]], !llvm.loop [[LOOP5:![0-9]+]]
// CHECK: [[FOR_END7]]:
// CHECK: ret void

void func2() {
#pragma omp metadirective when(user = {condition(1)} \
: parallel for) otherwise()
for (int i = 0; i < 100; i++)
;

#pragma omp metadirective when(user = {condition(1)} \
: parallel for)
for (int i = 0; i < 100; i++)
;
}

// CHECK-LABEL: define dso_local void @_Z5func2v()
// CHECK: entry
// CHECK-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB2:[0-9]+]], i32 0, ptr @_Z5func2v.omp_outlined)
// CHECK-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @[[GLOB2]], i32 0, ptr @_Z5func2v.omp_outlined.1)
// CHECK-NEXT: ret void


void func3() {
#pragma omp metadirective when(user = {condition(0)} \
: parallel for) \
when(implementation = {extension(match_none)} \
: parallel) default(parallel for)

for (int i = 0; i < 100; i++)
;

}

// CHECK-LABEL: define dso_local void @_Z5func3v()
// CHECK: entry
// CHECK-NEXT: call void (ptr, i32, ptr, ...) @__kmpc_fork_call(ptr @1, i32 0, ptr @_Z5func3v.omp_outlined)
// CHECK-NEXT: ret void
// CHECK-NEXT: }

// CHECK-LABEL: define internal void @_Z5func3v.omp_outlined
// CHECK-SAME: (ptr noalias noundef [[DOTGLOBAL_TID_:%.*]],
// CHECK-SAME: ptr noalias noundef [[DOTBOUND_TID_:%.*]])
// CHECK-NEXT: entry
// CHECK-NEXT: [[GLOB_TID__ADDR:%.*]] = alloca ptr, align 8
// CHECK-NEXT: [[BOUND_TID__ADDR:%.*]] = alloca ptr, align 8
// CHECK-NEXT: [[I:%.*]] = alloca i32, align 4
// CHECK-NEXT: store ptr [[DOTGLOBAL_TID_]], ptr [[GLOB_TID__ADDR]], align 8
// CHECK-NEXT: store ptr [[DOTBOUND_TID_]], ptr [[BOUND_TID__ADDR]], align 8
// CHECK-NEXT: store i32 0, ptr [[I]], align 4
// CHECK-NEXT: br label %for.cond
// CHECK:for.cond:
// CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[I]], align 4
// CHECK-NEXT: [[CMP:%.*]] = icmp slt i32 [[TMP0]], 100
// CHECK-NEXT: br i1 [[CMP]], label [[FOR_BODY:%.*]], label [[FOR_END:%.*]]
// CHECK:for.body:
// CHECK-NEXT: br label [[FOR_INC:%.*]]
// CHECK:for.inc:
// CHECK-NEXT: [[TMP1:%.*]] = load i32, ptr [[I]], align 4
// CHECK-NEXT: [[INC:%.*]] = add nsw i32 [[TMP1]], 1
// CHECK-NEXT: store i32 [[INC]], ptr [[I]], align 4
// CHECK-NEXT: br label [[FOR_COND:%.*]]
// CHECK:for.end:
// CHECK-NEXT: ret void
// CHECK-NEXT:}

#endif
2 changes: 1 addition & 1 deletion llvm/include/llvm/Frontend/OpenMP/OMPContext.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ bool isVariantApplicableInContext(const VariantMatchInfo &VMI,
bool DeviceSetOnly = false);

/// Return the index (into \p VMIs) of the variant with the highest score
/// from the ones applicble in \p Ctx. See llvm::isVariantApplicableInContext.
/// from the ones applicable in \p Ctx. See llvm::isVariantApplicableInContext.
int getBestVariantMatchForContext(const SmallVectorImpl<VariantMatchInfo> &VMIs,
const OMPContext &Ctx);

Expand Down