https://github.com/MaskRay approved this pull request.
bazel-6.3.2 build --config=generic_clang @llvm-project//clang
build passes with this refactoring. Thanks
https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@li
https://github.com/Endilll updated
https://github.com/llvm/llvm-project/pull/84184
>From 23f4208fb9978370f59cae16db0747acb3e2c906 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov
Date: Wed, 6 Mar 2024 18:01:35 +0300
Subject: [PATCH 1/6] [clang] Factor out OpenACC part of `Sema`
This patch mov
github-actions[bot] wrote:
:white_check_mark: With the latest revision this PR passed the Python code
formatter.
https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/m
Endilll wrote:
I added convenience functions to access `ASTContext` and other widely used
facilities of `Sema` to `SemaOpenACC`. I intentionally didn't go full base
class approach, saving this option for the future.
https://github.com/llvm/llvm-project/pull/84184
__
https://github.com/Endilll updated
https://github.com/llvm/llvm-project/pull/84184
>From 23f4208fb9978370f59cae16db0747acb3e2c906 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov
Date: Wed, 6 Mar 2024 18:01:35 +0300
Subject: [PATCH 1/7] [clang] Factor out OpenACC part of `Sema`
This patch mov
@@ -0,0 +1,74 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -0,0 +1,74 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -0,0 +1,74 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -0,0 +1,74 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
https://github.com/Endilll edited
https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Endilll edited
https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -0,0 +1,74 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
https://github.com/AaronBallman approved this pull request.
LGTM, but I think the next steps are two-fold: add a second `Sema` class for
something else (perhaps OpenMP?) and add a base class for accessing commonly
used functionality (`getASTContext()`, `Diag()`, etc).
https://github.com/llvm/l
https://github.com/Endilll updated
https://github.com/llvm/llvm-project/pull/84184
>From 23f4208fb9978370f59cae16db0747acb3e2c906 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov
Date: Wed, 6 Mar 2024 18:01:35 +0300
Subject: [PATCH 1/7] [clang] Factor out OpenACC part of `Sema`
This patch mov
https://github.com/Endilll closed
https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Endilll created
https://github.com/llvm/llvm-project/pull/84184
This patch moves OpenACC parts of `Sema` into a separate class `SemaOpenACC`
that is placed in a separate header `Sema/SemaOpenACC.h`. This patch is
intended to be a model of factoring things out of `Sema`, so I
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Vlad Serebrennikov (Endilll)
Changes
This patch moves OpenACC parts of `Sema` into a separate class `SemaOpenACC`
that is placed in a separate header `Sema/SemaOpenACC.h`. This patch is
intended to be a model of factoring things out of `S
@@ -0,0 +1,68 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -0,0 +1,68 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -0,0 +1,68 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -63,17 +67,17 @@ void Sema::ActOnOpenACCConstruct(OpenACCDirectiveKind K,
// here as these constructs do not take any arguments.
break;
default:
-Diag(StartLoc, diag::warn_acc_construct_unimplemented) << K;
+Sema.Diag(StartLoc, diag::warn_acc_construct_unim
@@ -1200,6 +1200,27 @@ class Sema final {
//
//
+ /// \name Sema Components
+ /// Parts of Sema
+ ///@{
+
+ // Just in this section, private members are followed by public, because
+ // C++ requires us to create (private) objects before (public) references.
+
+private
@@ -63,17 +67,17 @@ void Sema::ActOnOpenACCConstruct(OpenACCDirectiveKind K,
// here as these constructs do not take any arguments.
break;
default:
-Diag(StartLoc, diag::warn_acc_construct_unimplemented) << K;
+Sema.Diag(StartLoc, diag::warn_acc_construct_unim
@@ -63,17 +67,17 @@ void Sema::ActOnOpenACCConstruct(OpenACCDirectiveKind K,
// here as these constructs do not take any arguments.
break;
default:
-Diag(StartLoc, diag::warn_acc_construct_unimplemented) << K;
+Sema.Diag(StartLoc, diag::warn_acc_construct_unim
https://github.com/Endilll updated
https://github.com/llvm/llvm-project/pull/84184
>From 23f4208fb9978370f59cae16db0747acb3e2c906 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov
Date: Wed, 6 Mar 2024 18:01:35 +0300
Subject: [PATCH 1/3] [clang] Factor out OpenACC part of `Sema`
This patch mov
https://github.com/Endilll updated
https://github.com/llvm/llvm-project/pull/84184
>From 23f4208fb9978370f59cae16db0747acb3e2c906 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov
Date: Wed, 6 Mar 2024 18:01:35 +0300
Subject: [PATCH 1/4] [clang] Factor out OpenACC part of `Sema`
This patch mov
@@ -63,17 +67,17 @@ void Sema::ActOnOpenACCConstruct(OpenACCDirectiveKind K,
// here as these constructs do not take any arguments.
break;
default:
-Diag(StartLoc, diag::warn_acc_construct_unimplemented) << K;
+SemaRef.Diag(StartLoc, diag::warn_acc_construct_u
https://github.com/cor3ntin edited
https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/cor3ntin commented:
I have mixed feelings about that.
One one hand, i appreciate efforts to decouple Sema, on the other hand It's
unclear to me how much benefit we will be able to realize.
I think I'm fine with the change as long as there is no attempt to remove
everything.
AaronBallman wrote:
Thank you for this factoring! Personally, I think this is a good model to go
with for factoring functionality out of Sema and adding a tiny bit of layering
to this part of the compiler (full disclosure: Vlad and I worked on this design
offline). However, I added several oth
@@ -1162,6 +1162,11 @@ class Sema final {
/// CurContext - This is the current declaration context of parsing.
DeclContext *CurContext;
+ SemaOpenACC &OpenACC() {
+assert(OpenACCPtr);
cor3ntin wrote:
In the current design, the assert does nothing use
@@ -1162,6 +1162,11 @@ class Sema final {
/// CurContext - This is the current declaration context of parsing.
DeclContext *CurContext;
+ SemaOpenACC &OpenACC() {
+assert(OpenACCPtr);
erichkeane wrote:
IMO, there is value to having the assert. I can
@@ -1162,6 +1162,11 @@ class Sema final {
/// CurContext - This is the current declaration context of parsing.
DeclContext *CurContext;
+ SemaOpenACC &OpenACC() {
+assert(OpenACCPtr);
cor3ntin wrote:
Sure. But that's not yet the case. I think we shou
@@ -1162,6 +1162,11 @@ class Sema final {
/// CurContext - This is the current declaration context of parsing.
DeclContext *CurContext;
+ SemaOpenACC &OpenACC() {
+assert(OpenACCPtr);
erichkeane wrote:
I don't buy the 'noticeable impact on assertion
Endilll wrote:
Physical separation of parts of `Sema` while improving incremental compile
times means we have to rely on forward declarations, which lead to additional
level of indirection at runtime in the form of `Sema` containing pointers to
its components, and components containing a refer
@@ -1162,6 +1162,11 @@ class Sema final {
/// CurContext - This is the current declaration context of parsing.
DeclContext *CurContext;
+ SemaOpenACC &OpenACC() {
+assert(OpenACCPtr);
Endilll wrote:
Given that moving a hot path getter to out-of-line
@@ -0,0 +1,67 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
https://github.com/shafik commented:
I think this makes sense.
https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/shafik edited https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -1162,6 +1162,11 @@ class Sema final {
/// CurContext - This is the current declaration context of parsing.
DeclContext *CurContext;
+ SemaOpenACC &OpenACC() {
sam-mccall wrote:
nit: per style guide, this should be `openACC()` (lowercase O), and it sh
@@ -0,0 +1,67 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
https://github.com/sam-mccall edited
https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/sam-mccall edited
https://github.com/llvm/llvm-project/pull/84184
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -63,17 +67,17 @@ void Sema::ActOnOpenACCConstruct(OpenACCDirectiveKind K,
// here as these constructs do not take any arguments.
break;
default:
-Diag(StartLoc, diag::warn_acc_construct_unimplemented) << K;
+SemaRef.Diag(StartLoc, diag::warn_acc_construct_u
https://github.com/sam-mccall approved this pull request.
This direction looks good to me, mostly I just have nits.
The one major change I'd consider before landing is to find a way to avoid the
verbosity regression cor3ntin mentions for `Diag()` and friends. Notational
regressions within Sema
@@ -1162,6 +1162,11 @@ class Sema final {
/// CurContext - This is the current declaration context of parsing.
DeclContext *CurContext;
+ SemaOpenACC &OpenACC() {
Endilll wrote:
I'm not too keen to adhere to our style guide here, as this adds noise for
@@ -0,0 +1,67 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -0,0 +1,67 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -1162,6 +1162,11 @@ class Sema final {
/// CurContext - This is the current declaration context of parsing.
DeclContext *CurContext;
+ SemaOpenACC &OpenACC() {
AaronBallman wrote:
I think @sam-mccall is reading the coding guidelines the same way I rea
erichkeane wrote:
> This direction looks good to me, mostly I just have nits.
>
> The one major change I'd consider before landing is to find a way to avoid
> the verbosity regression cor3ntin mentions for `Diag()` and friends.
> Notational regressions within Sema itself may be as important as
@@ -0,0 +1,67 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
@@ -1162,6 +1162,11 @@ class Sema final {
/// CurContext - This is the current declaration context of parsing.
DeclContext *CurContext;
+ SemaOpenACC &OpenACC() {
+assert(OpenACCPtr);
AaronBallman wrote:
I think the assert is reasonable for now; some
https://github.com/Endilll updated
https://github.com/llvm/llvm-project/pull/84184
>From 23f4208fb9978370f59cae16db0747acb3e2c906 Mon Sep 17 00:00:00 2001
From: Vlad Serebrennikov
Date: Wed, 6 Mar 2024 18:01:35 +0300
Subject: [PATCH 1/5] [clang] Factor out OpenACC part of `Sema`
This patch mov
@@ -0,0 +1,67 @@
+//===- SemaOpenACC.h - Semantic Analysis for OpenACC constructs
---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apa
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff e85470232ba2fa49aaee83240741de0bc82a3ffa
4910ff2d9036d855af14582d82a9c44439b9efe0 --
cor3ntin wrote:
> > This direction looks good to me, mostly I just have nits.
> > The one major change I'd consider before landing is to find a way to avoid
> > the verbosity regression cor3ntin mentions for `Diag()` and friends.
> > Notational regressions within Sema itself may be as important
56 matches
Mail list logo