[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
https://github.com/sandeepkosuri closed https://github.com/llvm/llvm-project/pull/73690 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
https://github.com/alexey-bataev approved this pull request. https://github.com/llvm/llvm-project/pull/73690 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
@@ -12,7 +12,6 @@ int mixed() {
x=d;
}
-// expected-error@+2 {{#pragma omp nothing' cannot be an immediate
substatement}}
alexey-bataev wrote:
Ah, ok, I see. Then yes, this check must be removed.
https://github.com/llvm/llvm-project/pull/73690
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
@@ -12,7 +12,6 @@ int mixed() {
x=d;
}
-// expected-error@+2 {{#pragma omp nothing' cannot be an immediate
substatement}}
dreachem wrote:
As I understand it, the `nothing` directive is not the equivalent of a null
statement. It is an "ignore me" directive.
With OpenMP support, this:
```
if (x)
#pragma omp nothing
f();
```
should be transformed to this (i.e., the nothing directive should simply be
ignored):
```
if (x)
f();
```
If we are transforming it into a null statement, that is wrong.
https://github.com/llvm/llvm-project/pull/73690
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
@@ -12,7 +12,6 @@ int mixed() {
x=d;
}
-// expected-error@+2 {{#pragma omp nothing' cannot be an immediate
substatement}}
alexey-bataev wrote:
@dreachem The problem is, that the compiler has no idea what to do with this
code.
`nothing` has no associated structured block, right? If so, then this code is
actually this:
```
if (x)
#pragma omp nothing
f();
```
With OpenMP support, this code represents something like this:
```
if (x)
;
f();
```
If we disable OpenMP support, this code will be transformed into this:
```
if (x)
f();
```
because the directive will be ignored completely.
So, having something like this:
```
if (x)
#pragma omp nothing
```
should not be allowed, it is not correct program if OpenMP is disabled.
To make it correct, you need to have something like this:
```
if (x) {
#pragma omp nothing
}
```
This will be correct.
The check, you're trying to remove, checks exactly for such stand-alone
directives, which may cause troubles when OpenMP is disabled.
https://github.com/llvm/llvm-project/pull/73690
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
@@ -12,7 +12,6 @@ int mixed() {
x=d;
}
-// expected-error@+2 {{#pragma omp nothing' cannot be an immediate
substatement}}
dreachem wrote:
@alexey-bataev That restriction shouldn't apply to the `nothing` directive.
The `nothing` directive is not executable, and so is not considered a
"stand-alone directive". Note that allowing the insertion of the `barrier`
directive in the following would push the call to `f()` outside of the if
block; that's why it is restricted:
```
if (x)
#pragma omp barrier
f();
```
On the other hand, allowing the `nothing` directive in the following should
have no effect on the behavior of the program and therefore should be permitted:
```
if (x)
#pragma omp nothing
f();
```
https://github.com/llvm/llvm-project/pull/73690
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
@@ -12,7 +12,6 @@ int mixed() {
x=d;
}
-// expected-error@+2 {{#pragma omp nothing' cannot be an immediate
substatement}}
alexey-bataev wrote:
Seems to me, this should be allowed
https://github.com/llvm/llvm-project/pull/73690
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
@@ -12,7 +12,6 @@ int mixed() {
x=d;
}
-// expected-error@+2 {{#pragma omp nothing' cannot be an immediate
substatement}}
sandeepkosuri wrote:
oh sorry for missing that ! So, is this supposed to produce a compilation error
(provided arch is x86_64)?
```
#pragma omp target
#pragma omp metadirective \
when( device={arch("x86_64")}: nothing) \
default(parallel)
for (int i= 0; i< N; i++) v3[i] = v1[i] * v2[i];
```
https://github.com/llvm/llvm-project/pull/73690
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
@@ -12,7 +12,6 @@ int mixed() {
x=d;
}
-// expected-error@+2 {{#pragma omp nothing' cannot be an immediate
substatement}}
alexey-bataev wrote:
3 Directive and Construct Syntax
Neither a stand-alone directive nor a declarative directive may be used in
place of a substatement
15 in a selection statement or iteration statement, or in place of the
statement that follows a label.
https://github.com/llvm/llvm-project/pull/73690
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
@@ -12,7 +12,6 @@ int mixed() {
x=d;
}
-// expected-error@+2 {{#pragma omp nothing' cannot be an immediate
substatement}}
sandeepkosuri wrote:
But I don't see such a restriction for `nothing` in spec 5.1, is it really
required ?
https://github.com/llvm/llvm-project/pull/73690
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
@@ -12,7 +12,6 @@ int mixed() {
x=d;
}
-// expected-error@+2 {{#pragma omp nothing' cannot be an immediate
substatement}}
alexey-bataev wrote:
I think this error message should remain
https://github.com/llvm/llvm-project/pull/73690
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
llvmbot wrote:
@llvm/pr-subscribers-openmp
Author: Sandeep Kosuri (sandeepkosuri)
Changes
- Removed an unnecessary check that was preventing `nothing` to work properly
inside `metadirective`.
---
Full diff: https://github.com/llvm/llvm-project/pull/73690.diff
4 Files Affected:
- (modified) clang/lib/Parse/ParseOpenMP.cpp (+7-5)
- (modified) clang/test/OpenMP/metadirective_ast_print.c (+12)
- (modified) clang/test/OpenMP/metadirective_empty.cpp (+20)
- (modified) clang/test/OpenMP/nothing_messages.cpp (-1)
``diff
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index ca70bb241d6f723..fb7e7a979e49f7e 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2518,12 +2518,14 @@ StmtResult
Parser::ParseOpenMPDeclarativeOrExecutableDirective(
switch (DKind) {
case OMPD_nothing:
-if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) ==
-ParsedStmtContext())
- Diag(Tok, diag::err_omp_immediate_directive)
-<< getOpenMPDirectiveName(DKind) << 0;
ConsumeToken();
-skipUntilPragmaOpenMPEnd(DKind);
+// If we are parsing the directive within a metadirective, the directive
+// ends with a ')'.
+if (ReadDirectiveWithinMetadirective && Tok.is(tok::r_paren))
+ while (Tok.isNot(tok::annot_pragma_openmp_end))
+ConsumeAnyToken();
+else
+ skipUntilPragmaOpenMPEnd(DKind);
if (Tok.is(tok::annot_pragma_openmp_end))
ConsumeAnnotationToken();
break;
diff --git a/clang/test/OpenMP/metadirective_ast_print.c
b/clang/test/OpenMP/metadirective_ast_print.c
index ddd5b8633cc5013..d9ff7e764521607 100644
--- a/clang/test/OpenMP/metadirective_ast_print.c
+++ b/clang/test/OpenMP/metadirective_ast_print.c
@@ -67,6 +67,16 @@ void foo(void) {
default(parallel for)
for (int i = 0; i < 100; i++)
;
+
+#pragma omp metadirective when(implementation = {extension(match_all)} \
+ : nothing) default(parallel for)
+ for (int i = 0; i < 16; i++)
+;
+
+#pragma omp metadirective when(implementation = {extension(match_any)} \
+ : parallel) default(nothing)
+ for (int i = 0; i < 16; i++)
+;
}
// CHECK: void bar(void);
@@ -95,5 +105,7 @@ void foo(void) {
// CHECK-NEXT: for (int j = 0; j < 16; j++)
// CHECK-AMDGCN: #pragma omp teams distribute parallel for
// CHECK-AMDGCN-NEXT: for (int i = 0; i < 100; i++)
+// CHECK: for (int i = 0; i < 16; i++)
+// CHECK: for (int i = 0; i < 16; i++)
#endif
diff --git a/clang/test/OpenMP/metadirective_empty.cpp
b/clang/test/OpenMP/metadirective_empty.cpp
index 8708aa45b156309..b93ed722cb6e904 100644
--- a/clang/test/OpenMP/metadirective_empty.cpp
+++ b/clang/test/OpenMP/metadirective_empty.cpp
@@ -14,11 +14,17 @@ void func() {
:) default(parallel for)
for (int i = 0; i < N; i++)
;
+
+#pragma omp metadirective when(implementation = {vendor(llvm)} \
+ :nothing) default(parallel for)
+ for (int i = 0; i < N; i++)
+;
}
// CHECK-LABEL: void @_Z4funcv()
// CHECK: entry:
// CHECK: [[I:%.+]] = alloca i32,
+// CHECK: [[I1:%.+]] = alloca i32,
// CHECK: store i32 0, ptr [[I]],
// CHECK: br label %[[FOR_COND:.+]]
// CHECK: [[FOR_COND]]:
@@ -33,6 +39,20 @@ void func() {
// CHECK: store i32 [[INC]], ptr [[I]],
// CHECK: br label %[[FOR_COND]],
// CHECK: [[FOR_END]]:
+// CHECK: store i32 0, ptr [[I1]],
+// CHECK: br label %[[FOR_COND1:.+]]
+// CHECK: [[FOR_COND1]]:
+// CHECK: [[TWO:%.+]] = load i32, ptr [[I1]],
+// CHECK: [[CMP1:%.+]] = icmp slt i32 [[TWO]], 1000
+// CHECK: br i1 [[CMP1]], label %[[FOR_BODY1:.+]], label %[[FOR_END1:.+]]
+// CHECK: [[FOR_BODY1]]:
+// CHECK: br label %[[FOR_INC1:.+]]
+// CHECK: [[FOR_INC1]]:
+// CHECK: [[THREE:%.+]] = load i32, ptr [[I1]],
+// CHECK: [[INC1:%.+]] = add nsw i32 [[THREE]], 1
+// CHECK: store i32 [[INC1]], ptr [[I1]],
+// CHECK: br label %[[FOR_COND1]],
+// CHECK: [[FOR_END1]]:
// CHECK: ret void
// CHECK: }
diff --git a/clang/test/OpenMP/nothing_messages.cpp
b/clang/test/OpenMP/nothing_messages.cpp
index cd6d0defe492fb4..0e27e3c27076a7b 100644
--- a/clang/test/OpenMP/nothing_messages.cpp
+++ b/clang/test/OpenMP/nothing_messages.cpp
@@ -12,7 +12,6 @@ int mixed() {
x=d;
}
-// expected-error@+2 {{#pragma omp nothing' cannot be an immediate
substatement}}
if(!x)
#pragma omp nothing
x=d;
``
https://github.com/llvm/llvm-project/pull/73690
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenMP] Support for `nothing` in `metadirective` (PR #73690)
https://github.com/sandeepkosuri created
https://github.com/llvm/llvm-project/pull/73690
- Removed an unnecessary check that was preventing `nothing` to work properly
inside `metadirective`.
>From 0aad8e5ca5d7e9f2d2400e6c2f3db0a374d55ed8 Mon Sep 17 00:00:00 2001
From: Sandeep Kosuri
Date: Tue, 28 Nov 2023 13:03:25 -0600
Subject: [PATCH] [OpenMP] Support for `nothing` in `metadirective`
---
clang/lib/Parse/ParseOpenMP.cpp | 12 +++-
clang/test/OpenMP/metadirective_ast_print.c | 12
clang/test/OpenMP/metadirective_empty.cpp | 20
clang/test/OpenMP/nothing_messages.cpp | 1 -
4 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index ca70bb241d6f723..fb7e7a979e49f7e 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2518,12 +2518,14 @@ StmtResult
Parser::ParseOpenMPDeclarativeOrExecutableDirective(
switch (DKind) {
case OMPD_nothing:
-if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) ==
-ParsedStmtContext())
- Diag(Tok, diag::err_omp_immediate_directive)
-<< getOpenMPDirectiveName(DKind) << 0;
ConsumeToken();
-skipUntilPragmaOpenMPEnd(DKind);
+// If we are parsing the directive within a metadirective, the directive
+// ends with a ')'.
+if (ReadDirectiveWithinMetadirective && Tok.is(tok::r_paren))
+ while (Tok.isNot(tok::annot_pragma_openmp_end))
+ConsumeAnyToken();
+else
+ skipUntilPragmaOpenMPEnd(DKind);
if (Tok.is(tok::annot_pragma_openmp_end))
ConsumeAnnotationToken();
break;
diff --git a/clang/test/OpenMP/metadirective_ast_print.c
b/clang/test/OpenMP/metadirective_ast_print.c
index ddd5b8633cc5013..d9ff7e764521607 100644
--- a/clang/test/OpenMP/metadirective_ast_print.c
+++ b/clang/test/OpenMP/metadirective_ast_print.c
@@ -67,6 +67,16 @@ void foo(void) {
default(parallel for)
for (int i = 0; i < 100; i++)
;
+
+#pragma omp metadirective when(implementation = {extension(match_all)} \
+ : nothing) default(parallel for)
+ for (int i = 0; i < 16; i++)
+;
+
+#pragma omp metadirective when(implementation = {extension(match_any)} \
+ : parallel) default(nothing)
+ for (int i = 0; i < 16; i++)
+;
}
// CHECK: void bar(void);
@@ -95,5 +105,7 @@ void foo(void) {
// CHECK-NEXT: for (int j = 0; j < 16; j++)
// CHECK-AMDGCN: #pragma omp teams distribute parallel for
// CHECK-AMDGCN-NEXT: for (int i = 0; i < 100; i++)
+// CHECK: for (int i = 0; i < 16; i++)
+// CHECK: for (int i = 0; i < 16; i++)
#endif
diff --git a/clang/test/OpenMP/metadirective_empty.cpp
b/clang/test/OpenMP/metadirective_empty.cpp
index 8708aa45b156309..b93ed722cb6e904 100644
--- a/clang/test/OpenMP/metadirective_empty.cpp
+++ b/clang/test/OpenMP/metadirective_empty.cpp
@@ -14,11 +14,17 @@ void func() {
:) default(parallel for)
for (int i = 0; i < N; i++)
;
+
+#pragma omp metadirective when(implementation = {vendor(llvm)} \
+ :nothing) default(parallel for)
+ for (int i = 0; i < N; i++)
+;
}
// CHECK-LABEL: void @_Z4funcv()
// CHECK: entry:
// CHECK: [[I:%.+]] = alloca i32,
+// CHECK: [[I1:%.+]] = alloca i32,
// CHECK: store i32 0, ptr [[I]],
// CHECK: br label %[[FOR_COND:.+]]
// CHECK: [[FOR_COND]]:
@@ -33,6 +39,20 @@ void func() {
// CHECK: store i32 [[INC]], ptr [[I]],
// CHECK: br label %[[FOR_COND]],
// CHECK: [[FOR_END]]:
+// CHECK: store i32 0, ptr [[I1]],
+// CHECK: br label %[[FOR_COND1:.+]]
+// CHECK: [[FOR_COND1]]:
+// CHECK: [[TWO:%.+]] = load i32, ptr [[I1]],
+// CHECK: [[CMP1:%.+]] = icmp slt i32 [[TWO]], 1000
+// CHECK: br i1 [[CMP1]], label %[[FOR_BODY1:.+]], label %[[FOR_END1:.+]]
+// CHECK: [[FOR_BODY1]]:
+// CHECK: br label %[[FOR_INC1:.+]]
+// CHECK: [[FOR_INC1]]:
+// CHECK: [[THREE:%.+]] = load i32, ptr [[I1]],
+// CHECK: [[INC1:%.+]] = add nsw i32 [[THREE]], 1
+// CHECK: store i32 [[INC1]], ptr [[I1]],
+// CHECK: br label %[[FOR_COND1]],
+// CHECK: [[FOR_END1]]:
// CHECK: ret void
// CHECK: }
diff --git a/clang/test/OpenMP/nothing_messages.cpp
b/clang/test/OpenMP/nothing_messages.cpp
index cd6d0defe492fb4..0e27e3c27076a7b 100644
--- a/clang/test/OpenMP/nothing_messages.cpp
+++ b/clang/test/OpenMP/nothing_messages.cpp
@@ -12,7 +12,6 @@ int mixed() {
x=d;
}
-// expected-error@+2 {{#pragma omp nothing' cannot be an immediate
substatement}}
if(!x)
#pragma omp nothing
x=d;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
