[clang] [OpenACC] Implement compound construct parsing (PR #72692)
alexey-bataev wrote: Need to add tests https://github.com/llvm/llvm-project/pull/72692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
@@ -45,11 +47,36 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
P.ConsumeToken();
std::string FirstTokSpelling = P.getPreprocessor().getSpelling(FirstTok);
- OpenACCDirectiveKind DirKind = GetOpenACCDirectiveKind(FirstTokSpelling);
+ OpenACCDirectiveKind DirKind = getOpenACCDirectiveKind(FirstTokSpelling);
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ if (!SecondTok.isAnnotation() &&
+ P.getPreprocessor().getSpelling(SecondTok) == "loop") {
alexey-bataev wrote:
The better to add a function isOpenACCDirectiveKind(OpenACCDirectiveKind Kind,
StringRef Tok)
```
switch (Kind) {
case OpenACCDirectiveKind::Loop:
return Tok == "loop";
case OpenACCDirectiveKind::Parallel:
return Tok == "parallel";
...
}
```
https://github.com/llvm/llvm-project/pull/72692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
erichkeane wrote: > Need to add tests I mentioned in the commit message, that unfortunately this doesn't change behavior in a way that we can discover in our testing (unless you know how to check which character a diagnostic was issued on with `VerifyDiagnosticsConsumer`?). I'd left the tests for these in place already, but the `loop` was being interpreted as the beginning of a `clause`, so it ends up being diagnosed as `unknown clause`. However, now that we're consuming 'loop' as well, we still get the same diagnostic for the NEXT token. https://github.com/llvm/llvm-project/pull/72692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
erichkeane wrote: > > Need to add tests > > I mentioned in the commit message, that unfortunately this doesn't change > behavior in a way that we can discover in our testing (unless you know how to > check which character a diagnostic was issued on with > `VerifyDiagnosticsConsumer`?). I'd left the tests for these in place already, > but the `loop` was being interpreted as the beginning of a `clause`, so it > ends up being diagnosed as `unknown clause`. However, now that we're > consuming 'loop' as well, we still get the same diagnostic for the NEXT token. Actually, just realized there would be a difference in behavior for `pragma acc parallel loop` (with no clause). So I'll add tests for those. https://github.com/llvm/llvm-project/pull/72692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
@@ -45,11 +47,36 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
P.ConsumeToken();
std::string FirstTokSpelling = P.getPreprocessor().getSpelling(FirstTok);
- OpenACCDirectiveKind DirKind = GetOpenACCDirectiveKind(FirstTokSpelling);
+ OpenACCDirectiveKind DirKind = getOpenACCDirectiveKind(FirstTokSpelling);
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ if (!SecondTok.isAnnotation() &&
+ P.getPreprocessor().getSpelling(SecondTok) == "loop") {
erichkeane wrote:
Done.
https://github.com/llvm/llvm-project/pull/72692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
https://github.com/erichkeane updated
https://github.com/llvm/llvm-project/pull/72692
>From 33ca88871b48fcfb16bdf7fa636a9ecfa4f38e08 Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Fri, 17 Nov 2023 11:34:43 -0800
Subject: [PATCH 1/4] [OpenACC] Implement compound construct parsing
This patch implements the compound construct parsing, which allows
'parallel loop', 'serial loop', and 'kernel loop' to act as their own
constructs.
Note that this doesn't end up making any changes to the tests, as
previously these ended up being considered clauses, which are also not
implemented. However, the location of that diagnostic is now 1 token
further along.
---
clang/include/clang/Basic/OpenACCKinds.h | 5 +++-
clang/lib/Parse/ParseOpenACC.cpp | 33 +++-
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Basic/OpenACCKinds.h
b/clang/include/clang/Basic/OpenACCKinds.h
index 8da02b93b2b974c..d53b7223b5334b3 100644
--- a/clang/include/clang/Basic/OpenACCKinds.h
+++ b/clang/include/clang/Basic/OpenACCKinds.h
@@ -33,7 +33,10 @@ enum class OpenACCDirectiveKind {
Loop,
// FIXME: 'cache'
- // FIXME: Combined Constructs.
+ // Combined Constructs.
+ ParallelLoop,
+ SerialLoop,
+ KernelsLoop,
// FIXME: atomic Construct variants.
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index ba29d75fe35a500..a3c802b1f829877 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -22,7 +22,9 @@ using namespace llvm;
namespace {
-// Translate single-token string representations to the OpenACC Directive Kind.
+// This doesn't completely comprehend 'Compound Constructs' (as it just
+// identifies the first token) just the first token of each. So
+// this should only be used by `ParseOpenACCDirectiveKind`.
OpenACCDirectiveKind GetOpenACCDirectiveKind(StringRef Name) {
return llvm::StringSwitch(Name)
.Case("parallel", OpenACCDirectiveKind::Parallel)
@@ -50,6 +52,35 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ switch (DirKind) {
+ default:
+// Nothing to do except in the below cases, as they should be diagnosed as
+// a clause.
+break;
+ case OpenACCDirectiveKind::Parallel:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::ParallelLoop;
+}
+break;
+ case OpenACCDirectiveKind::Serial:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::SerialLoop;
+}
+break;
+ case OpenACCDirectiveKind::Kernels:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::KernelsLoop;
+}
+break;
+ }
+
return DirKind;
}
>From bb2020318ec35858293f4c7a42e3fb2904a3013f Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Fri, 17 Nov 2023 13:35:53 -0800
Subject: [PATCH 2/4] Fix Alexeys comments
---
clang/lib/Parse/ParseOpenACC.cpp | 48 +++-
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index a3c802b1f829877..7a29c825c146f6b 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -22,10 +22,10 @@ using namespace llvm;
namespace {
-// This doesn't completely comprehend 'Compound Constructs' (as it just
-// identifies the first token) just the first token of each. So
-// this should only be used by `ParseOpenACCDirectiveKind`.
-OpenACCDirectiveKind GetOpenACCDirectiveKind(StringRef Name) {
+/// This doesn't completely comprehend 'Compound Constructs' (as it just
+/// identifies the first token) just the first token of each. So
+/// this should only be used by `ParseOpenACCDirectiveKind`.
+OpenACCDirectiveKind getOpenACCDirectiveKind(StringRef Name) {
return llvm::StringSwitch(Name)
.Case("parallel", OpenACCDirectiveKind::Parallel)
.Case("serial", OpenACCDirectiveKind::Serial)
@@ -47,7 +47,7 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
P.ConsumeToken();
std::string FirstTokSpelling = P.getPreprocessor().getSpelling(FirstTok);
- OpenACCDirectiveKind DirKind = GetOpenACCDirectiveKind(FirstTokSpelling);
+ OpenACCDirectiveKind DirKind = getOpenACCDirectiveKind(FirstTokSpelling);
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
@@ -56,29 +56,25 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
// other attempt at a
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/72692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
https://github.com/alexey-bataev approved this pull request. https://github.com/llvm/llvm-project/pull/72692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
https://github.com/erichkeane closed https://github.com/llvm/llvm-project/pull/72692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
https://github.com/erichkeane created
https://github.com/llvm/llvm-project/pull/72692
This patch implements the compound construct parsing, which allows 'parallel
loop', 'serial loop', and 'kernel loop' to act as their own constructs.
Note that this doesn't end up making any changes to the tests, as previously
these ended up being considered clauses, which are also not implemented.
However, the location of that diagnostic is now 1 token further along.
>From 33ca88871b48fcfb16bdf7fa636a9ecfa4f38e08 Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Fri, 17 Nov 2023 11:34:43 -0800
Subject: [PATCH] [OpenACC] Implement compound construct parsing
This patch implements the compound construct parsing, which allows
'parallel loop', 'serial loop', and 'kernel loop' to act as their own
constructs.
Note that this doesn't end up making any changes to the tests, as
previously these ended up being considered clauses, which are also not
implemented. However, the location of that diagnostic is now 1 token
further along.
---
clang/include/clang/Basic/OpenACCKinds.h | 5 +++-
clang/lib/Parse/ParseOpenACC.cpp | 33 +++-
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Basic/OpenACCKinds.h
b/clang/include/clang/Basic/OpenACCKinds.h
index 8da02b93b2b974c..d53b7223b5334b3 100644
--- a/clang/include/clang/Basic/OpenACCKinds.h
+++ b/clang/include/clang/Basic/OpenACCKinds.h
@@ -33,7 +33,10 @@ enum class OpenACCDirectiveKind {
Loop,
// FIXME: 'cache'
- // FIXME: Combined Constructs.
+ // Combined Constructs.
+ ParallelLoop,
+ SerialLoop,
+ KernelsLoop,
// FIXME: atomic Construct variants.
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index ba29d75fe35a500..a3c802b1f829877 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -22,7 +22,9 @@ using namespace llvm;
namespace {
-// Translate single-token string representations to the OpenACC Directive Kind.
+// This doesn't completely comprehend 'Compound Constructs' (as it just
+// identifies the first token) just the first token of each. So
+// this should only be used by `ParseOpenACCDirectiveKind`.
OpenACCDirectiveKind GetOpenACCDirectiveKind(StringRef Name) {
return llvm::StringSwitch(Name)
.Case("parallel", OpenACCDirectiveKind::Parallel)
@@ -50,6 +52,35 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ switch (DirKind) {
+ default:
+// Nothing to do except in the below cases, as they should be diagnosed as
+// a clause.
+break;
+ case OpenACCDirectiveKind::Parallel:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::ParallelLoop;
+}
+break;
+ case OpenACCDirectiveKind::Serial:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::SerialLoop;
+}
+break;
+ case OpenACCDirectiveKind::Kernels:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::KernelsLoop;
+}
+break;
+ }
+
return DirKind;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Erich Keane (erichkeane)
Changes
This patch implements the compound construct parsing, which allows 'parallel
loop', 'serial loop', and 'kernel loop' to act as their own constructs.
Note that this doesn't end up making any changes to the tests, as previously
these ended up being considered clauses, which are also not implemented.
However, the location of that diagnostic is now 1 token further along.
---
Full diff: https://github.com/llvm/llvm-project/pull/72692.diff
2 Files Affected:
- (modified) clang/include/clang/Basic/OpenACCKinds.h (+4-1)
- (modified) clang/lib/Parse/ParseOpenACC.cpp (+32-1)
``diff
diff --git a/clang/include/clang/Basic/OpenACCKinds.h
b/clang/include/clang/Basic/OpenACCKinds.h
index 8da02b93b2b974c..d53b7223b5334b3 100644
--- a/clang/include/clang/Basic/OpenACCKinds.h
+++ b/clang/include/clang/Basic/OpenACCKinds.h
@@ -33,7 +33,10 @@ enum class OpenACCDirectiveKind {
Loop,
// FIXME: 'cache'
- // FIXME: Combined Constructs.
+ // Combined Constructs.
+ ParallelLoop,
+ SerialLoop,
+ KernelsLoop,
// FIXME: atomic Construct variants.
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index ba29d75fe35a500..a3c802b1f829877 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -22,7 +22,9 @@ using namespace llvm;
namespace {
-// Translate single-token string representations to the OpenACC Directive Kind.
+// This doesn't completely comprehend 'Compound Constructs' (as it just
+// identifies the first token) just the first token of each. So
+// this should only be used by `ParseOpenACCDirectiveKind`.
OpenACCDirectiveKind GetOpenACCDirectiveKind(StringRef Name) {
return llvm::StringSwitch(Name)
.Case("parallel", OpenACCDirectiveKind::Parallel)
@@ -50,6 +52,35 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ switch (DirKind) {
+ default:
+// Nothing to do except in the below cases, as they should be diagnosed as
+// a clause.
+break;
+ case OpenACCDirectiveKind::Parallel:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::ParallelLoop;
+}
+break;
+ case OpenACCDirectiveKind::Serial:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::SerialLoop;
+}
+break;
+ case OpenACCDirectiveKind::Kernels:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::KernelsLoop;
+}
+break;
+ }
+
return DirKind;
}
``
https://github.com/llvm/llvm-project/pull/72692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
@@ -22,7 +22,9 @@ using namespace llvm;
namespace {
-// Translate single-token string representations to the OpenACC Directive Kind.
+// This doesn't completely comprehend 'Compound Constructs' (as it just
+// identifies the first token) just the first token of each. So
+// this should only be used by `ParseOpenACCDirectiveKind`.
alexey-bataev wrote:
Use /// style of comments
https://github.com/llvm/llvm-project/pull/72692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
https://github.com/alexey-bataev edited https://github.com/llvm/llvm-project/pull/72692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
@@ -50,6 +52,35 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ switch (DirKind) {
+ default:
+// Nothing to do except in the below cases, as they should be diagnosed as
+// a clause.
+break;
+ case OpenACCDirectiveKind::Parallel:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::ParallelLoop;
+}
+break;
+ case OpenACCDirectiveKind::Serial:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::SerialLoop;
+}
+break;
+ case OpenACCDirectiveKind::Kernels:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::KernelsLoop;
+}
+break;
+ }
alexey-bataev wrote:
```suggestion
if (GetOpenACCDirectiveKind(P.getPreprocessor().getSpelling(SecondTok)) ==
OpenACCDirectiveKind::Loop) {
P.ConsumeToken();
switch (DirKind) {
default:
// error??
break;
case OpenACCDirectiveKind::Parallel:
return OpenACCDirectiveKind::ParallelLoop;
case OpenACCDirectiveKind::Serial:
return OpenACCDirectiveKind::SerialLoop;
case OpenACCDirectiveKind::Kernels:
return OpenACCDirectiveKind::KernelsLoop;
}
}
```
https://github.com/llvm/llvm-project/pull/72692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
@@ -22,7 +22,9 @@ using namespace llvm;
namespace {
-// Translate single-token string representations to the OpenACC Directive Kind.
+// This doesn't completely comprehend 'Compound Constructs' (as it just
+// identifies the first token) just the first token of each. So
+// this should only be used by `ParseOpenACCDirectiveKind`.
OpenACCDirectiveKind GetOpenACCDirectiveKind(StringRef Name) {
alexey-bataev wrote:
Missed ёрш in the previous reviews, better to name it `getOpenACCDirectiveKind`
https://github.com/llvm/llvm-project/pull/72692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
https://github.com/alexey-bataev edited https://github.com/llvm/llvm-project/pull/72692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
https://github.com/erichkeane updated
https://github.com/llvm/llvm-project/pull/72692
>From 33ca88871b48fcfb16bdf7fa636a9ecfa4f38e08 Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Fri, 17 Nov 2023 11:34:43 -0800
Subject: [PATCH 1/2] [OpenACC] Implement compound construct parsing
This patch implements the compound construct parsing, which allows
'parallel loop', 'serial loop', and 'kernel loop' to act as their own
constructs.
Note that this doesn't end up making any changes to the tests, as
previously these ended up being considered clauses, which are also not
implemented. However, the location of that diagnostic is now 1 token
further along.
---
clang/include/clang/Basic/OpenACCKinds.h | 5 +++-
clang/lib/Parse/ParseOpenACC.cpp | 33 +++-
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Basic/OpenACCKinds.h
b/clang/include/clang/Basic/OpenACCKinds.h
index 8da02b93b2b974c..d53b7223b5334b3 100644
--- a/clang/include/clang/Basic/OpenACCKinds.h
+++ b/clang/include/clang/Basic/OpenACCKinds.h
@@ -33,7 +33,10 @@ enum class OpenACCDirectiveKind {
Loop,
// FIXME: 'cache'
- // FIXME: Combined Constructs.
+ // Combined Constructs.
+ ParallelLoop,
+ SerialLoop,
+ KernelsLoop,
// FIXME: atomic Construct variants.
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index ba29d75fe35a500..a3c802b1f829877 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -22,7 +22,9 @@ using namespace llvm;
namespace {
-// Translate single-token string representations to the OpenACC Directive Kind.
+// This doesn't completely comprehend 'Compound Constructs' (as it just
+// identifies the first token) just the first token of each. So
+// this should only be used by `ParseOpenACCDirectiveKind`.
OpenACCDirectiveKind GetOpenACCDirectiveKind(StringRef Name) {
return llvm::StringSwitch(Name)
.Case("parallel", OpenACCDirectiveKind::Parallel)
@@ -50,6 +52,35 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ switch (DirKind) {
+ default:
+// Nothing to do except in the below cases, as they should be diagnosed as
+// a clause.
+break;
+ case OpenACCDirectiveKind::Parallel:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::ParallelLoop;
+}
+break;
+ case OpenACCDirectiveKind::Serial:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::SerialLoop;
+}
+break;
+ case OpenACCDirectiveKind::Kernels:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::KernelsLoop;
+}
+break;
+ }
+
return DirKind;
}
>From bb2020318ec35858293f4c7a42e3fb2904a3013f Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Fri, 17 Nov 2023 13:35:53 -0800
Subject: [PATCH 2/2] Fix Alexeys comments
---
clang/lib/Parse/ParseOpenACC.cpp | 48 +++-
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index a3c802b1f829877..7a29c825c146f6b 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -22,10 +22,10 @@ using namespace llvm;
namespace {
-// This doesn't completely comprehend 'Compound Constructs' (as it just
-// identifies the first token) just the first token of each. So
-// this should only be used by `ParseOpenACCDirectiveKind`.
-OpenACCDirectiveKind GetOpenACCDirectiveKind(StringRef Name) {
+/// This doesn't completely comprehend 'Compound Constructs' (as it just
+/// identifies the first token) just the first token of each. So
+/// this should only be used by `ParseOpenACCDirectiveKind`.
+OpenACCDirectiveKind getOpenACCDirectiveKind(StringRef Name) {
return llvm::StringSwitch(Name)
.Case("parallel", OpenACCDirectiveKind::Parallel)
.Case("serial", OpenACCDirectiveKind::Serial)
@@ -47,7 +47,7 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
P.ConsumeToken();
std::string FirstTokSpelling = P.getPreprocessor().getSpelling(FirstTok);
- OpenACCDirectiveKind DirKind = GetOpenACCDirectiveKind(FirstTokSpelling);
+ OpenACCDirectiveKind DirKind = getOpenACCDirectiveKind(FirstTokSpelling);
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
@@ -56,29 +56,25 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
// other attempt at a
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
@@ -45,11 +47,36 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
P.ConsumeToken();
std::string FirstTokSpelling = P.getPreprocessor().getSpelling(FirstTok);
- OpenACCDirectiveKind DirKind = GetOpenACCDirectiveKind(FirstTokSpelling);
+ OpenACCDirectiveKind DirKind = getOpenACCDirectiveKind(FirstTokSpelling);
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ if (!SecondTok.isAnnotation() &&
+ P.getPreprocessor().getSpelling(SecondTok) == "loop") {
alexey-bataev wrote:
Better to use `getOpenACCDirectiveKind` here to aavoid spreading constant
strings across code.
https://github.com/llvm/llvm-project/pull/72692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
https://github.com/alexey-bataev edited https://github.com/llvm/llvm-project/pull/72692 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
@@ -50,6 +52,35 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ switch (DirKind) {
+ default:
+// Nothing to do except in the below cases, as they should be diagnosed as
+// a clause.
+break;
+ case OpenACCDirectiveKind::Parallel:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::ParallelLoop;
+}
+break;
+ case OpenACCDirectiveKind::Serial:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::SerialLoop;
+}
+break;
+ case OpenACCDirectiveKind::Kernels:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::KernelsLoop;
+}
+break;
+ }
erichkeane wrote:
That changes the meaning semantically, but I like the idea of inverting the
check. I initially shied away from it because of the additional cost
(calculating the spelling and the string comparison on every directive, not
just in the special cases), but perhaps that isn't so bad. Using the
`GetOpenACCDirectiveKind` here seems like doing significantly more work than
necessary in every directive, so I think there is value to sticking to a single
string comparison.
That said, I've done the check inversion in the next patch.
https://github.com/llvm/llvm-project/pull/72692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
@@ -45,11 +47,36 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
P.ConsumeToken();
std::string FirstTokSpelling = P.getPreprocessor().getSpelling(FirstTok);
- OpenACCDirectiveKind DirKind = GetOpenACCDirectiveKind(FirstTokSpelling);
+ OpenACCDirectiveKind DirKind = getOpenACCDirectiveKind(FirstTokSpelling);
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ if (!SecondTok.isAnnotation() &&
+ P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+OpenACCDirectiveKind ReturnKind;
+switch (DirKind) {
+default:
+ // Nothing to do except in the below cases, as they should be diagnosed
as
+ // a clause.
+ break;
+case OpenACCDirectiveKind::Parallel:
+ ReturnKind = OpenACCDirectiveKind::ParallelLoop;
+ LLVM_FALLTHROUGH;
+case OpenACCDirectiveKind::Serial:
+ ReturnKind = OpenACCDirectiveKind::SerialLoop;
+ LLVM_FALLTHROUGH;
+case OpenACCDirectiveKind::Kernels:
+ ReturnKind = OpenACCDirectiveKind::KernelsLoop;
+ P.ConsumeToken();
+ return ReturnKind;
alexey-bataev wrote:
Hmm, ReturnKind will be rewritten in the last case
https://github.com/llvm/llvm-project/pull/72692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
@@ -45,11 +47,36 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
P.ConsumeToken();
std::string FirstTokSpelling = P.getPreprocessor().getSpelling(FirstTok);
- OpenACCDirectiveKind DirKind = GetOpenACCDirectiveKind(FirstTokSpelling);
+ OpenACCDirectiveKind DirKind = getOpenACCDirectiveKind(FirstTokSpelling);
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ if (!SecondTok.isAnnotation() &&
+ P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+OpenACCDirectiveKind ReturnKind;
+switch (DirKind) {
+default:
+ // Nothing to do except in the below cases, as they should be diagnosed
as
+ // a clause.
+ break;
+case OpenACCDirectiveKind::Parallel:
+ ReturnKind = OpenACCDirectiveKind::ParallelLoop;
+ LLVM_FALLTHROUGH;
+case OpenACCDirectiveKind::Serial:
+ ReturnKind = OpenACCDirectiveKind::SerialLoop;
+ LLVM_FALLTHROUGH;
+case OpenACCDirectiveKind::Kernels:
+ ReturnKind = OpenACCDirectiveKind::KernelsLoop;
+ P.ConsumeToken();
+ return ReturnKind;
erichkeane wrote:
Ah, doh! This is what I get for trying to be clever! You're absolutely right.
I'll go back to just doing a ConsumeToken in each.
https://github.com/llvm/llvm-project/pull/72692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
https://github.com/erichkeane updated
https://github.com/llvm/llvm-project/pull/72692
>From 33ca88871b48fcfb16bdf7fa636a9ecfa4f38e08 Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Fri, 17 Nov 2023 11:34:43 -0800
Subject: [PATCH 1/3] [OpenACC] Implement compound construct parsing
This patch implements the compound construct parsing, which allows
'parallel loop', 'serial loop', and 'kernel loop' to act as their own
constructs.
Note that this doesn't end up making any changes to the tests, as
previously these ended up being considered clauses, which are also not
implemented. However, the location of that diagnostic is now 1 token
further along.
---
clang/include/clang/Basic/OpenACCKinds.h | 5 +++-
clang/lib/Parse/ParseOpenACC.cpp | 33 +++-
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Basic/OpenACCKinds.h
b/clang/include/clang/Basic/OpenACCKinds.h
index 8da02b93b2b974c..d53b7223b5334b3 100644
--- a/clang/include/clang/Basic/OpenACCKinds.h
+++ b/clang/include/clang/Basic/OpenACCKinds.h
@@ -33,7 +33,10 @@ enum class OpenACCDirectiveKind {
Loop,
// FIXME: 'cache'
- // FIXME: Combined Constructs.
+ // Combined Constructs.
+ ParallelLoop,
+ SerialLoop,
+ KernelsLoop,
// FIXME: atomic Construct variants.
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index ba29d75fe35a500..a3c802b1f829877 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -22,7 +22,9 @@ using namespace llvm;
namespace {
-// Translate single-token string representations to the OpenACC Directive Kind.
+// This doesn't completely comprehend 'Compound Constructs' (as it just
+// identifies the first token) just the first token of each. So
+// this should only be used by `ParseOpenACCDirectiveKind`.
OpenACCDirectiveKind GetOpenACCDirectiveKind(StringRef Name) {
return llvm::StringSwitch(Name)
.Case("parallel", OpenACCDirectiveKind::Parallel)
@@ -50,6 +52,35 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ switch (DirKind) {
+ default:
+// Nothing to do except in the below cases, as they should be diagnosed as
+// a clause.
+break;
+ case OpenACCDirectiveKind::Parallel:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::ParallelLoop;
+}
+break;
+ case OpenACCDirectiveKind::Serial:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::SerialLoop;
+}
+break;
+ case OpenACCDirectiveKind::Kernels:
+if (P.getPreprocessor().getSpelling(SecondTok) == "loop") {
+ P.ConsumeToken();
+ return OpenACCDirectiveKind::KernelsLoop;
+}
+break;
+ }
+
return DirKind;
}
>From bb2020318ec35858293f4c7a42e3fb2904a3013f Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Fri, 17 Nov 2023 13:35:53 -0800
Subject: [PATCH 2/3] Fix Alexeys comments
---
clang/lib/Parse/ParseOpenACC.cpp | 48 +++-
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index a3c802b1f829877..7a29c825c146f6b 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -22,10 +22,10 @@ using namespace llvm;
namespace {
-// This doesn't completely comprehend 'Compound Constructs' (as it just
-// identifies the first token) just the first token of each. So
-// this should only be used by `ParseOpenACCDirectiveKind`.
-OpenACCDirectiveKind GetOpenACCDirectiveKind(StringRef Name) {
+/// This doesn't completely comprehend 'Compound Constructs' (as it just
+/// identifies the first token) just the first token of each. So
+/// this should only be used by `ParseOpenACCDirectiveKind`.
+OpenACCDirectiveKind getOpenACCDirectiveKind(StringRef Name) {
return llvm::StringSwitch(Name)
.Case("parallel", OpenACCDirectiveKind::Parallel)
.Case("serial", OpenACCDirectiveKind::Serial)
@@ -47,7 +47,7 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
P.ConsumeToken();
std::string FirstTokSpelling = P.getPreprocessor().getSpelling(FirstTok);
- OpenACCDirectiveKind DirKind = GetOpenACCDirectiveKind(FirstTokSpelling);
+ OpenACCDirectiveKind DirKind = getOpenACCDirectiveKind(FirstTokSpelling);
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
@@ -56,29 +56,25 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
// other attempt at a
[clang] [OpenACC] Implement compound construct parsing (PR #72692)
@@ -45,11 +47,36 @@ OpenACCDirectiveKind ParseOpenACCDirectiveKind(Parser &P) {
P.ConsumeToken();
std::string FirstTokSpelling = P.getPreprocessor().getSpelling(FirstTok);
- OpenACCDirectiveKind DirKind = GetOpenACCDirectiveKind(FirstTokSpelling);
+ OpenACCDirectiveKind DirKind = getOpenACCDirectiveKind(FirstTokSpelling);
if (DirKind == OpenACCDirectiveKind::Invalid)
P.Diag(FirstTok, diag::err_acc_invalid_directive) << FirstTokSpelling;
+ // Combined Constructs allows parallel loop, serial loop, or kernels loop.
Any
+ // other attempt at a combined construct will be diagnosed as an invalid
+ // clause.
+ Token SecondTok = P.getCurToken();
+ if (!SecondTok.isAnnotation() &&
+ P.getPreprocessor().getSpelling(SecondTok) == "loop") {
erichkeane wrote:
Hmm... while I see the benefit of that, I'm actually a bit concerned about the
performance implications of doing what amounts to a dozen string comparisons in
that case, vs 1 here. Extracting this above has already increased the number
of checks significantly (by checking for loop ALWAYS rather than just 1x), but
it seems significantly worse to do multiples here. WDYT?
https://github.com/llvm/llvm-project/pull/72692
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
