[clang] [OpenACC] Implement 'return' branch-out of Compute Construct (PR #82814)
erichkeane wrote: > Looks good. Looks like something similar should be done for goto Thanks! Yes, I agree we probably want to do something similar for goto, but I'm still looking at it. Trying to keep patches small :) https://github.com/llvm/llvm-project/pull/82814 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'return' branch-out of Compute Construct (PR #82814)
https://github.com/erichkeane updated
https://github.com/llvm/llvm-project/pull/82814
>From cdbf2a137ed7ba0a6d40f955072ef636ee93b292 Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Fri, 23 Feb 2024 10:36:31 -0800
Subject: [PATCH 1/2] [OpenACC] Implement 'return' branch-out of Compute
Construct
Like with 'break'/'continue', returning out of a compute construct is
ill-formed, so this implements the diagnostic. However, unlike the
OpenMP implementation of this same diagnostic, OpenACC doesn't have a
concept of 'capture region', so this is implemented as just checking the
'scope'.
---
.../clang/Basic/DiagnosticSemaKinds.td| 5 +++--
clang/include/clang/Sema/Scope.h | 13
clang/lib/Sema/SemaStmt.cpp | 16 +++
clang/test/SemaOpenACC/no-branch-in-out.c | 20 +++
clang/test/SemaOpenACC/no-branch-in-out.cpp | 17
5 files changed, 65 insertions(+), 6 deletions(-)
create mode 100644 clang/test/SemaOpenACC/no-branch-in-out.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a7f2858477bee6..608265ca78432f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12203,6 +12203,7 @@ def warn_acc_clause_unimplemented
def err_acc_construct_appertainment
: Error<"OpenACC construct '%0' cannot be used here; it can only "
"be used in a statement context">;
-def err_acc_branch_in_out
-: Error<"invalid branch %select{out of|into}0 OpenACC Compute Construct">;
+def err_acc_branch_in_out_compute_construct
+: Error<"invalid %select{branch|return}0 %select{out of|into}1 OpenACC "
+"Compute Construct">;
} // end of sema component.
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index e7f166fe3461fd..370133cd7429b0 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -521,6 +521,19 @@ class Scope {
return getFlags() & Scope::OpenACCComputeConstructScope;
}
+ bool isInOpenACCComputeConstructScope() const {
+for (const Scope *S = this; S; S = S->getParent()) {
+ if (S->getFlags() & Scope::OpenACCComputeConstructScope)
+return true;
+ else if (S->getFlags() & (Scope::FnScope | Scope::ClassScope |
+Scope::BlockScope | Scope::TemplateParamScope |
+Scope::FunctionPrototypeScope |
+Scope::AtCatchScope | Scope::ObjCMethodScope))
+return false;
+}
+return false;
+ }
+
/// Determine whether this scope is a while/do/for statement, which can have
/// continue statements embedded into it.
bool isContinueScope() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index fcad09a63662ba..0a5c2b23a90c8e 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3361,8 +3361,9 @@ Sema::ActOnContinueStmt(SourceLocation ContinueLoc, Scope
*CurScope) {
// of a compute construct counts as 'branching out of' the compute construct,
// so diagnose here.
if (S->isOpenACCComputeConstructScope())
-return StmtError(Diag(ContinueLoc, diag::err_acc_branch_in_out)
- << /*out of */ 0);
+return StmtError(
+Diag(ContinueLoc, diag::err_acc_branch_in_out_compute_construct)
+<< /*branch*/ 0 << /*out of */ 0);
CheckJumpOutOfSEHFinally(*this, ContinueLoc, *S);
@@ -3390,8 +3391,9 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope
*CurScope) {
if (S->isOpenACCComputeConstructScope() ||
(S->isLoopScope() && S->getParent() &&
S->getParent()->isOpenACCComputeConstructScope()))
-return StmtError(Diag(BreakLoc, diag::err_acc_branch_in_out)
- << /*out of */ 0);
+return StmtError(
+Diag(BreakLoc, diag::err_acc_branch_in_out_compute_construct)
+<< /*branch*/ 0 << /*out of */ 0);
CheckJumpOutOfSEHFinally(*this, BreakLoc, *S);
@@ -3947,6 +3949,12 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr
*RetValExp,
RetValExp, nullptr, /*RecoverUncorrectedTypos=*/true);
if (RetVal.isInvalid())
return StmtError();
+
+ if (getCurScope()->isInOpenACCComputeConstructScope())
+return StmtError(
+Diag(ReturnLoc, diag::err_acc_branch_in_out_compute_construct)
+<< /*return*/ 1 << /*out of */ 0);
+
StmtResult R =
BuildReturnStmt(ReturnLoc, RetVal.get(), /*AllowRecovery=*/true);
if (R.isInvalid() || ExprEvalContexts.back().isDiscardedStatementContext())
diff --git a/clang/test/SemaOpenACC/no-branch-in-out.c
b/clang/test/SemaOpenACC/no-branch-in-out.c
index 33a171f1b68d51..f8fb40a1ca8f72 100644
--- a/clang/test/SemaOpenACC/no-branch-in-out.c
+++ b/clang/test/SemaOpenACC/no-branch-in-out.c
@@ -93,3 +93,23 @@ void BreakContinue() {
}
+void Return() {
+#pragma acc
[clang] [OpenACC] Implement 'return' branch-out of Compute Construct (PR #82814)
https://github.com/erichkeane created
https://github.com/llvm/llvm-project/pull/82814
Like with 'break'/'continue', returning out of a compute construct is
ill-formed, so this implements the diagnostic. However, unlike the OpenMP
implementation of this same diagnostic, OpenACC doesn't have a concept of
'capture region', so this is implemented as just checking the 'scope'.
>From cdbf2a137ed7ba0a6d40f955072ef636ee93b292 Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Fri, 23 Feb 2024 10:36:31 -0800
Subject: [PATCH] [OpenACC] Implement 'return' branch-out of Compute Construct
Like with 'break'/'continue', returning out of a compute construct is
ill-formed, so this implements the diagnostic. However, unlike the
OpenMP implementation of this same diagnostic, OpenACC doesn't have a
concept of 'capture region', so this is implemented as just checking the
'scope'.
---
.../clang/Basic/DiagnosticSemaKinds.td| 5 +++--
clang/include/clang/Sema/Scope.h | 13
clang/lib/Sema/SemaStmt.cpp | 16 +++
clang/test/SemaOpenACC/no-branch-in-out.c | 20 +++
clang/test/SemaOpenACC/no-branch-in-out.cpp | 17
5 files changed, 65 insertions(+), 6 deletions(-)
create mode 100644 clang/test/SemaOpenACC/no-branch-in-out.cpp
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a7f2858477bee6..608265ca78432f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12203,6 +12203,7 @@ def warn_acc_clause_unimplemented
def err_acc_construct_appertainment
: Error<"OpenACC construct '%0' cannot be used here; it can only "
"be used in a statement context">;
-def err_acc_branch_in_out
-: Error<"invalid branch %select{out of|into}0 OpenACC Compute Construct">;
+def err_acc_branch_in_out_compute_construct
+: Error<"invalid %select{branch|return}0 %select{out of|into}1 OpenACC "
+"Compute Construct">;
} // end of sema component.
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index e7f166fe3461fd..370133cd7429b0 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -521,6 +521,19 @@ class Scope {
return getFlags() & Scope::OpenACCComputeConstructScope;
}
+ bool isInOpenACCComputeConstructScope() const {
+for (const Scope *S = this; S; S = S->getParent()) {
+ if (S->getFlags() & Scope::OpenACCComputeConstructScope)
+return true;
+ else if (S->getFlags() & (Scope::FnScope | Scope::ClassScope |
+Scope::BlockScope | Scope::TemplateParamScope |
+Scope::FunctionPrototypeScope |
+Scope::AtCatchScope | Scope::ObjCMethodScope))
+return false;
+}
+return false;
+ }
+
/// Determine whether this scope is a while/do/for statement, which can have
/// continue statements embedded into it.
bool isContinueScope() const {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index fcad09a63662ba..0a5c2b23a90c8e 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3361,8 +3361,9 @@ Sema::ActOnContinueStmt(SourceLocation ContinueLoc, Scope
*CurScope) {
// of a compute construct counts as 'branching out of' the compute construct,
// so diagnose here.
if (S->isOpenACCComputeConstructScope())
-return StmtError(Diag(ContinueLoc, diag::err_acc_branch_in_out)
- << /*out of */ 0);
+return StmtError(
+Diag(ContinueLoc, diag::err_acc_branch_in_out_compute_construct)
+<< /*branch*/ 0 << /*out of */ 0);
CheckJumpOutOfSEHFinally(*this, ContinueLoc, *S);
@@ -3390,8 +3391,9 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope
*CurScope) {
if (S->isOpenACCComputeConstructScope() ||
(S->isLoopScope() && S->getParent() &&
S->getParent()->isOpenACCComputeConstructScope()))
-return StmtError(Diag(BreakLoc, diag::err_acc_branch_in_out)
- << /*out of */ 0);
+return StmtError(
+Diag(BreakLoc, diag::err_acc_branch_in_out_compute_construct)
+<< /*branch*/ 0 << /*out of */ 0);
CheckJumpOutOfSEHFinally(*this, BreakLoc, *S);
@@ -3947,6 +3949,12 @@ Sema::ActOnReturnStmt(SourceLocation ReturnLoc, Expr
*RetValExp,
RetValExp, nullptr, /*RecoverUncorrectedTypos=*/true);
if (RetVal.isInvalid())
return StmtError();
+
+ if (getCurScope()->isInOpenACCComputeConstructScope())
+return StmtError(
+Diag(ReturnLoc, diag::err_acc_branch_in_out_compute_construct)
+<< /*return*/ 1 << /*out of */ 0);
+
StmtResult R =
BuildReturnStmt(ReturnLoc, RetVal.get(), /*AllowRecovery=*/true);
if (R.isInvalid() || ExprEvalContexts.back().isDiscardedStatementContext())
diff --git
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -194,3 +204,87 @@ __global__ void non_cexpr_waves_per_eu_2() {}
// expected-error@+1{{'amdgpu_waves_per_eu' attribute requires parameter 1 to
be an integer constant}}
__attribute__((amdgpu_waves_per_eu(2, ipow2(2
__global__ void non_cexpr_waves_per_eu_2_4() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires exactly
3 arguments}}
+__attribute__((amdgpu_max_num_work_groups(32)))
+__global__ void max_num_work_groups_32() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires exactly
3 arguments}}
+__attribute__((amdgpu_max_num_work_groups(32, 1)))
+__global__ void max_num_work_groups_32_1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires exactly
3 arguments}}
+__attribute__((amdgpu_max_num_work_groups(32, 1, 1, 1)))
+__global__ void max_num_work_groups_32_1_1_1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires
parameter 0 to be an integer constant}}
+__attribute__((amdgpu_max_num_work_groups(ipow2(5), 1, 1)))
+__global__ void max_num_work_groups_32_1_1_non_int_arg0() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires
parameter 1 to be an integer constant}}
+__attribute__((amdgpu_max_num_work_groups(32, "1", 1)))
+__global__ void max_num_work_groups_32_1_1_non_int_arg1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires a
non-negative integral compile time constant expression}}
+__attribute__((amdgpu_max_num_work_groups(-32, 1, 1)))
+__global__ void max_num_work_groups_32_1_1_neg_int_arg0() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires a
non-negative integral compile time constant expression}}
+__attribute__((amdgpu_max_num_work_groups(32, -1, 1)))
+__global__ void max_num_work_groups_32_1_1_neg_int_arg1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires a
non-negative integral compile time constant expression}}
+__attribute__((amdgpu_max_num_work_groups(32, 1, -1)))
+__global__ void max_num_work_groups_32_1_1_neg_int_arg2() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute must be greater
than 0}}
+__attribute__((amdgpu_max_num_work_groups(0, 1, 1)))
+__global__ void max_num_work_groups_0_1_1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute must be greater
than 0}}
+__attribute__((amdgpu_max_num_work_groups(32, 0, 1)))
+__global__ void max_num_work_groups_32_0_1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute must be greater
than 0}}
+__attribute__((amdgpu_max_num_work_groups(32, 1, 0)))
+__global__ void max_num_work_groups_32_1_0() {}
+
+
+int num_wg_x = 32;
+int num_wg_y = 1;
+int num_wg_z = 1;
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires
parameter 0 to be an integer constant}}
+__attribute__((amdgpu_max_num_work_groups(num_wg_x, 1, 1)))
+__global__ void max_num_work_groups_32_1_1_non_const_arg0() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires
parameter 1 to be an integer constant}}
+__attribute__((amdgpu_max_num_work_groups(32, num_wg_y, 1)))
+__global__ void max_num_work_groups_32_1_1_non_const_arg1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires
parameter 2 to be an integer constant}}
+__attribute__((amdgpu_max_num_work_groups(32, 1, num_wg_z)))
+__global__ void max_num_work_groups_32_1_1_non_const_arg2() {}
+
+const int c_num_wg_x = 32;
+__attribute__((amdgpu_max_num_work_groups(c_num_wg_x, 1, 1)))
+__global__ void max_num_work_groups_32_1_1_const_arg0() {}
+
+// expected-error@+2{{'amdgpu_max_num_work_groups' attribute requires
parameter 0 to be an integer constant}}
erichkeane wrote:
I realize that, but an attribute should be able to take template arguments, see
the example I gave above. You should test before calling `CheckUInt32Argument`
if it is dependent, and then only check it once the template is instantiated.
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add __builtin_start_object_lifetime builtin. (PR #82776)
@@ -4386,6 +4386,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl
GD, unsigned BuiltinID,
return RValue::get(nullptr);
}
+ case Builtin::BI__builtin_start_object_lifetime:
case Builtin::BI__builtin_launder: {
erichkeane wrote:
Should the `TypeRequiresBuiltinLaunder` function be generalized, at least in
name?
https://github.com/llvm/llvm-project/pull/82776
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add __builtin_start_object_lifetime builtin. (PR #82776)
https://github.com/erichkeane commented: This would also need a release note I believe. I don't have the codegen expertise to review this with high confidence, but it looks right to me. https://github.com/llvm/llvm-project/pull/82776 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add __builtin_start_object_lifetime builtin. (PR #82776)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/82776 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -194,3 +204,87 @@ __global__ void non_cexpr_waves_per_eu_2() {}
// expected-error@+1{{'amdgpu_waves_per_eu' attribute requires parameter 1 to
be an integer constant}}
__attribute__((amdgpu_waves_per_eu(2, ipow2(2
__global__ void non_cexpr_waves_per_eu_2_4() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires exactly
3 arguments}}
+__attribute__((amdgpu_max_num_work_groups(32)))
+__global__ void max_num_work_groups_32() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires exactly
3 arguments}}
+__attribute__((amdgpu_max_num_work_groups(32, 1)))
+__global__ void max_num_work_groups_32_1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires exactly
3 arguments}}
+__attribute__((amdgpu_max_num_work_groups(32, 1, 1, 1)))
+__global__ void max_num_work_groups_32_1_1_1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires
parameter 0 to be an integer constant}}
+__attribute__((amdgpu_max_num_work_groups(ipow2(5), 1, 1)))
+__global__ void max_num_work_groups_32_1_1_non_int_arg0() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires
parameter 1 to be an integer constant}}
+__attribute__((amdgpu_max_num_work_groups(32, "1", 1)))
+__global__ void max_num_work_groups_32_1_1_non_int_arg1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires a
non-negative integral compile time constant expression}}
+__attribute__((amdgpu_max_num_work_groups(-32, 1, 1)))
+__global__ void max_num_work_groups_32_1_1_neg_int_arg0() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires a
non-negative integral compile time constant expression}}
+__attribute__((amdgpu_max_num_work_groups(32, -1, 1)))
+__global__ void max_num_work_groups_32_1_1_neg_int_arg1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires a
non-negative integral compile time constant expression}}
+__attribute__((amdgpu_max_num_work_groups(32, 1, -1)))
+__global__ void max_num_work_groups_32_1_1_neg_int_arg2() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute must be greater
than 0}}
+__attribute__((amdgpu_max_num_work_groups(0, 1, 1)))
+__global__ void max_num_work_groups_0_1_1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute must be greater
than 0}}
+__attribute__((amdgpu_max_num_work_groups(32, 0, 1)))
+__global__ void max_num_work_groups_32_0_1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute must be greater
than 0}}
+__attribute__((amdgpu_max_num_work_groups(32, 1, 0)))
+__global__ void max_num_work_groups_32_1_0() {}
+
+
+int num_wg_x = 32;
+int num_wg_y = 1;
+int num_wg_z = 1;
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires
parameter 0 to be an integer constant}}
+__attribute__((amdgpu_max_num_work_groups(num_wg_x, 1, 1)))
+__global__ void max_num_work_groups_32_1_1_non_const_arg0() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires
parameter 1 to be an integer constant}}
+__attribute__((amdgpu_max_num_work_groups(32, num_wg_y, 1)))
+__global__ void max_num_work_groups_32_1_1_non_const_arg1() {}
+
+// expected-error@+1{{'amdgpu_max_num_work_groups' attribute requires
parameter 2 to be an integer constant}}
+__attribute__((amdgpu_max_num_work_groups(32, 1, num_wg_z)))
+__global__ void max_num_work_groups_32_1_1_non_const_arg2() {}
+
+const int c_num_wg_x = 32;
+__attribute__((amdgpu_max_num_work_groups(c_num_wg_x, 1, 1)))
+__global__ void max_num_work_groups_32_1_1_const_arg0() {}
+
+// expected-error@+2{{'amdgpu_max_num_work_groups' attribute requires
parameter 0 to be an integer constant}}
erichkeane wrote:
This isn't a good diagnostic, `a` IS an integer constant! All of these are
actually integer constants and likely should work.
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -8069,6 +8069,38 @@ static void handleAMDGPUNumVGPRAttr(Sema , Decl *D,
const ParsedAttr ) {
D->addAttr(::new (S.Context) AMDGPUNumVGPRAttr(S.Context, AL, NumVGPR));
}
+static void handleAMDGPUMaxNumWorkGroupsAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+ if (AL.getNumArgs() != 3) {
+S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 3;
+return;
+ }
+ uint32_t NumWGX = 0;
+ uint32_t NumWGY = 0;
+ uint32_t NumWGZ = 0;
+ Expr *NumWGXExpr = AL.getArgAsExpr(0);
+ Expr *NumWGYExpr = AL.getArgAsExpr(1);
+ Expr *NumWGZExpr = AL.getArgAsExpr(2);
+ if (!checkUInt32Argument(S, AL, NumWGXExpr, NumWGX, 0, true))
+return;
+ if (!checkUInt32Argument(S, AL, NumWGYExpr, NumWGY, 1, true))
+return;
+ if (!checkUInt32Argument(S, AL, NumWGZExpr, NumWGZ, 2, true))
+return;
+
+ if (NumWGX == 0 || NumWGY == 0 || NumWGZ == 0) {
erichkeane wrote:
It seems that this whole function is effectively just `handleWorkGroupSize`.
Can we just use that function here instead? It seems to do exactly what you
want here without such awkward layout.
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -8069,6 +8069,38 @@ static void handleAMDGPUNumVGPRAttr(Sema , Decl *D,
const ParsedAttr ) {
D->addAttr(::new (S.Context) AMDGPUNumVGPRAttr(S.Context, AL, NumVGPR));
}
+static void handleAMDGPUMaxNumWorkGroupsAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+ if (AL.getNumArgs() != 3) {
+S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments) << AL << 3;
+return;
+ }
+ uint32_t NumWGX = 0;
+ uint32_t NumWGY = 0;
+ uint32_t NumWGZ = 0;
+ Expr *NumWGXExpr = AL.getArgAsExpr(0);
+ Expr *NumWGYExpr = AL.getArgAsExpr(1);
+ Expr *NumWGZExpr = AL.getArgAsExpr(2);
+ if (!checkUInt32Argument(S, AL, NumWGXExpr, NumWGX, 0, true))
+return;
+ if (!checkUInt32Argument(S, AL, NumWGYExpr, NumWGY, 1, true))
+return;
+ if (!checkUInt32Argument(S, AL, NumWGZExpr, NumWGZ, 2, true))
+return;
+
+ if (NumWGX == 0 || NumWGY == 0 || NumWGZ == 0) {
+Expr *E = NumWGZExpr;
+if (NumWGY == 0)
+ E = NumWGYExpr;
+if (NumWGX == 0)
+ E = NumWGXExpr;
+S.Diag(AL.getLoc(), diag::err_attribute_argument_is_zero)
+<< AL << E->getSourceRange();
+ } else
+D->addAttr(::new (S.Context) AMDGPUMaxNumWorkGroupsAttr(
erichkeane wrote:
still need curley brackets around the 'else'.
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
https://github.com/erichkeane closed https://github.com/llvm/llvm-project/pull/82543 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
https://github.com/erichkeane updated
https://github.com/llvm/llvm-project/pull/82543
>From 5ea47a31eb0ce851441cb7f1851b13303732ca91 Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Wed, 21 Feb 2024 10:08:06 -0800
Subject: [PATCH 1/4] [OpenACC] Implement 'break' and 'continue' errors for
Compute Cnstrcts
OpenACC3.3 2.5.4 says: "A program may not branch into or out of a
compute construct". While some of this restriction isn't particularly
checkable, 'break' and 'continue' are possible and pretty trivial, so
this patch implements those limitations.
It IS unclear in the case of a 'break' in a 'switch' what should happen
(an antagonistic reading of the standard would prevent it from
appearing), however we're choosing to special-case the break-in-switch
to ensure that this works (albeit, a 'parallel' directive on a 'switch'
isn't particularly useful, though permitted).
Future implementations of this rule will be in a follow-up patch.
---
.../clang/Basic/DiagnosticSemaKinds.td| 2 +
clang/include/clang/Sema/Scope.h | 15 +++
clang/lib/Parse/ParseOpenACC.cpp | 17
clang/lib/Sema/SemaStmt.cpp | 22 +
clang/test/SemaOpenACC/no-branch-in-out.c | 95 +++
5 files changed, 151 insertions(+)
create mode 100644 clang/test/SemaOpenACC/no-branch-in-out.c
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 11411883e1bfc6..dbe6eecaa73df4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12201,4 +12201,6 @@ def warn_acc_clause_unimplemented
def err_acc_construct_appertainment
: Error<"OpenACC construct '%0' cannot be used here; it can only "
"be used in a statement context">;
+def err_acc_branch_in_out
+: Error<"invalid branch %select{out of|into}0 OpenACC region">;
} // end of sema component.
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index 9e81706cd2aa1d..3ffcc3590ae0e0 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -150,6 +150,9 @@ class Scope {
/// template scope in between), the outer scope does not increase the
/// depth of recursion.
LambdaScope = 0x800,
+/// This is the scope of an OpenACC Compute Construct, which restricts
+/// jumping into/out of it.
+OpenACCComputeConstructScope = 0x1000,
};
private:
@@ -469,6 +472,12 @@ class Scope {
return false;
}
+ /// Return true if this exact scope (and not one of it's parents) is a switch
+ /// scope.
+ bool isDirectlySwitchScope() const {
+return getFlags() & Scope::SwitchScope;
+ }
+
/// Determines whether this scope is the OpenMP directive scope
bool isOpenMPDirectiveScope() const {
return (getFlags() & Scope::OpenMPDirectiveScope);
@@ -504,6 +513,12 @@ class Scope {
return getFlags() & Scope::OpenMPOrderClauseScope;
}
+ /// Determine whether this scope is the statement associated with an OpenACC
+ /// Compute construct directive.
+ bool isOpenACCComputeConstructScope() const {
+return getFlags() & Scope::OpenACCComputeConstructScope;
+ }
+
/// Determine whether this scope is a while/do/for statement, which can have
/// continue statements embedded into it.
bool isContinueScope() const {
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index 50e78e8687aea1..4946a61fca007f 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -560,6 +560,21 @@ bool doesDirectiveHaveAssociatedStmt(OpenACCDirectiveKind
DirKind) {
llvm_unreachable("Unhandled directive->assoc stmt");
}
+unsigned getOpenACCScopeFlags(OpenACCDirectiveKind DirKind) {
+ switch (DirKind) {
+ case OpenACCDirectiveKind::Parallel:
+// Mark this as a BreakScope/ContinueScope as well as a compute construct
+// so that we can diagnose trying to 'break'/'continue' inside of one.
+return Scope::BreakScope | Scope::ContinueScope |
+ Scope::OpenACCComputeConstructScope;
+ case OpenACCDirectiveKind::Invalid:
+llvm_unreachable("Shouldn't be creating a scope for an invalid construct");
+ default:
+break;
+ }
+ return 0;
+}
+
} // namespace
// OpenACC 3.3, section 1.7:
@@ -1228,6 +1243,8 @@ StmtResult Parser::ParseOpenACCDirectiveStmt() {
if (doesDirectiveHaveAssociatedStmt(DirInfo.DirKind)) {
ParsingOpenACCDirectiveRAII DirScope(*this, /*Value=*/false);
+ParseScope ACCScope(this, getOpenACCScopeFlags(DirInfo.DirKind));
+
AssocStmt = getActions().ActOnOpenACCAssociatedStmt(DirInfo.DirKind,
ParseStatement());
}
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index dde3bd84e89f8b..b1025c05e5db50 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3356,6 +3356,14 @@
[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)
@@ -4097,6 +4105,14 @@ FieldDecl *Expr::getSourceBitField() {
return nullptr;
}
+EnumConstantDecl *Expr::getEnumConstantDecl() {
+ Expr *E = this->IgnoreParenImpCasts();
+ if (DeclRefExpr *DRE = dyn_cast(E))
+if (EnumConstantDecl *ECD = dyn_cast(DRE->getDecl()))
erichkeane wrote:
```suggestion
return dyn_cast(DRE->getDecl());
```
https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)
@@ -263,6 +263,14 @@ namespace {
}
}
+QualType Expr::getEnumCoercedType(const ASTContext ) const {
+ bool NotEnumType = dyn_cast(this->getType()) == nullptr;
+ if (NotEnumType)
erichkeane wrote:
```suggestion
if (!isa(this->getType()))
```
Though, would probably suggest just something like:
```
if (isa(getType())) return getType();
else if (const EnumConstantDecl *ECD = getEnumConstantDecl())
return Ctx.getTypeDeclType...
return getType();
```
https://github.com/llvm/llvm-project/pull/81418
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)
https://github.com/erichkeane commented: Not the best one to review this, but some drive-bys https://github.com/llvm/llvm-project/pull/81418 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix missing warning when comparing mismatched enums in … (PR #81418)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/81418 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
@@ -3371,6 +3379,20 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope
*CurScope) {
if (S->isOpenMPLoopScope())
return StmtError(Diag(BreakLoc, diag::err_omp_loop_cannot_use_stmt)
<< "break");
+
+ // OpenACC doesn't allow 'break'ing from a compute construct, so diagnose if
+ // we are trying to do so. This can come in 2 flavors: 1-the break'able
thing
+ // (besides the compute construct) 'contains' the compute construct, at which
+ // point the 'break' scope will be the compute construct. Else it could be a
+ // loop of some sort that has a direct parent of the compute construct.
+ // However, a 'break' in a 'switch' marked as a compute construct doesn't
+ // count as 'branch out of' the compute construct.
+ if (S->isOpenACCComputeConstructScope() ||
+ (!S->isDirectlySwitchScope() && S->getParent() &&
erichkeane wrote:
> No problem we're in the same position. :) I'm not asking about the very first
> check, it is ok and I did not ask about it. What I'm asking is exactly the
> second part (after OR, second part of the logic). I'm suggesting instead of
> checking for non-switch just to check explicitly for 'for scope'. "Not switch
> scope" is too broad, I just suggest to check for the scopes, which are not
> allowed instead (isForScope). I think it is much easier to read and
> understand the context.
Ah! So we don't actually HAVE a `ForScope` as far as I can tell, let alone the
`while` or `do-while`: https://clang.llvm.org/doxygen/classclang_1_1Scope.html
Perhaps I could change the name of `isDirectlySwitchScope` to be: `isBreakLoop`
or something, and have it do the inverse of what `isDirectlySwitchScope` does
now?
https://github.com/llvm/llvm-project/pull/82543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
@@ -3371,6 +3379,20 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope
*CurScope) {
if (S->isOpenMPLoopScope())
return StmtError(Diag(BreakLoc, diag::err_omp_loop_cannot_use_stmt)
<< "break");
+
+ // OpenACC doesn't allow 'break'ing from a compute construct, so diagnose if
+ // we are trying to do so. This can come in 2 flavors: 1-the break'able
thing
+ // (besides the compute construct) 'contains' the compute construct, at which
+ // point the 'break' scope will be the compute construct. Else it could be a
+ // loop of some sort that has a direct parent of the compute construct.
+ // However, a 'break' in a 'switch' marked as a compute construct doesn't
+ // count as 'branch out of' the compute construct.
+ if (S->isOpenACCComputeConstructScope() ||
+ (!S->isDirectlySwitchScope() && S->getParent() &&
erichkeane wrote:
Sorry if I'm being dumb here, but I don't really get the question? We're
trying to avoid diagnosing when the `breakParent` is a `switch`, so we are
excluding it. If we were to NOT use the inversion, we would be diagnosing ONLY
on a the switch (the opposite of what I want?).
So the logic for when to diagnose is:
`S->isOpenACCComputeConstructScope()` <-- Diagnose if the 'current' breakParent
is a Compute Construct
OR
`(!S->isDirectlySwitchScope() && S->getParent() &&
S->getParent()->isOpenACCComputeConstructScope()))` <-- If the Parent of the
`breakParent` is a Compute Construct, unless it is a `switch`.
https://github.com/llvm/llvm-project/pull/82543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
@@ -3371,6 +3379,20 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope
*CurScope) {
if (S->isOpenMPLoopScope())
return StmtError(Diag(BreakLoc, diag::err_omp_loop_cannot_use_stmt)
<< "break");
+
+ // OpenACC doesn't allow 'break'ing from a compute construct, so diagnose if
+ // we are trying to do so. This can come in 2 flavors: 1-the break'able
thing
+ // (besides the compute construct) 'contains' the compute construct, at which
+ // point the 'break' scope will be the compute construct. Else it could be a
+ // loop of some sort that has a direct parent of the compute construct.
+ // However, a 'break' in a 'switch' marked as a compute construct doesn't
+ // count as 'branch out of' the compute construct.
+ if (S->isOpenACCComputeConstructScope() ||
+ (!S->isDirectlySwitchScope() && S->getParent() &&
erichkeane wrote:
I was just thinking... since the concern here is readability: would it be
acceptable to move the checking in this 'if' statement to a static function
with multiple exits/a series of comments to describe what is happening?
https://github.com/llvm/llvm-project/pull/82543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
@@ -3371,6 +3379,20 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope
*CurScope) {
if (S->isOpenMPLoopScope())
return StmtError(Diag(BreakLoc, diag::err_omp_loop_cannot_use_stmt)
<< "break");
+
+ // OpenACC doesn't allow 'break'ing from a compute construct, so diagnose if
+ // we are trying to do so. This can come in 2 flavors: 1-the break'able
thing
+ // (besides the compute construct) 'contains' the compute construct, at which
+ // point the 'break' scope will be the compute construct. Else it could be a
+ // loop of some sort that has a direct parent of the compute construct.
+ // However, a 'break' in a 'switch' marked as a compute construct doesn't
+ // count as 'branch out of' the compute construct.
+ if (S->isOpenACCComputeConstructScope() ||
+ (!S->isDirectlySwitchScope() && S->getParent() &&
erichkeane wrote:
> Ok, I see. For OpenMP this is correct behavior per standard.
>
> > And checking the 'is loop with a parent openacc scope' doesn't work if the
> > openacc-pragma is inside the loop.
>
> Why it does not work?
Because the statement might not be a loop! So just saying, "is this a loop
inside a pragma " would miss other cases where break could be valid (switch and
statement exprs, and a few others).
https://github.com/llvm/llvm-project/pull/82543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
@@ -3371,6 +3379,20 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope
*CurScope) {
if (S->isOpenMPLoopScope())
return StmtError(Diag(BreakLoc, diag::err_omp_loop_cannot_use_stmt)
<< "break");
+
+ // OpenACC doesn't allow 'break'ing from a compute construct, so diagnose if
+ // we are trying to do so. This can come in 2 flavors: 1-the break'able
thing
+ // (besides the compute construct) 'contains' the compute construct, at which
+ // point the 'break' scope will be the compute construct. Else it could be a
+ // loop of some sort that has a direct parent of the compute construct.
+ // However, a 'break' in a 'switch' marked as a compute construct doesn't
+ // count as 'branch out of' the compute construct.
+ if (S->isOpenACCComputeConstructScope() ||
+ (!S->isDirectlySwitchScope() && S->getParent() &&
erichkeane wrote:
>What is "ignorability"? Sorry, just asking to understand the context more
>correctly.
The idea is that we should be able to 'ignore' the pragmas and have the program
still have meaning. A big part of the 'theory' here is that you should be able
to add pragmas to existing code and get 'descriptive' messages that show what
you did wrong, and a pragma introducing a scope isnt really 'true' (so having
it observably act as a scope seems wrong).
>These constructs, if enabled, should be pretty similar to lambdas, so I assume
>it should be fine to diagnose dangling break/continue here. The diagnostics
>might be improved, though.
I think the difference is that the lambda introduces a lookup scope/etc,
whereas these pragmas aren't supposed to. For example, setting the 'function'
scope prevents something like:
```
auto foo(int i) {
#pragma acc parallel
int j = function_call();
return i + j;
}
```
from working. See : https://godbolt.org/z/Gq8Yen1vr (for the omp example).
This is actually something that is supposed to work in OpenACC, so introducing
the scope makes that not work.
https://github.com/llvm/llvm-project/pull/82543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
@@ -3371,6 +3379,20 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope
*CurScope) {
if (S->isOpenMPLoopScope())
return StmtError(Diag(BreakLoc, diag::err_omp_loop_cannot_use_stmt)
<< "break");
+
+ // OpenACC doesn't allow 'break'ing from a compute construct, so diagnose if
+ // we are trying to do so. This can come in 2 flavors: 1-the break'able
thing
+ // (besides the compute construct) 'contains' the compute construct, at which
+ // point the 'break' scope will be the compute construct. Else it could be a
+ // loop of some sort that has a direct parent of the compute construct.
+ // However, a 'break' in a 'switch' marked as a compute construct doesn't
+ // count as 'branch out of' the compute construct.
+ if (S->isOpenACCComputeConstructScope() ||
+ (!S->isDirectlySwitchScope() && S->getParent() &&
erichkeane wrote:
ALSO, it doesn't help the 'continue' case, where the inverse is the requirement
(a continue on the 'annotated loop' is perfectly fine, but not one that would
jump out of the pragma).
https://github.com/llvm/llvm-project/pull/82543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
@@ -3371,6 +3379,20 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope
*CurScope) {
if (S->isOpenMPLoopScope())
return StmtError(Diag(BreakLoc, diag::err_omp_loop_cannot_use_stmt)
<< "break");
+
+ // OpenACC doesn't allow 'break'ing from a compute construct, so diagnose if
+ // we are trying to do so. This can come in 2 flavors: 1-the break'able
thing
+ // (besides the compute construct) 'contains' the compute construct, at which
+ // point the 'break' scope will be the compute construct. Else it could be a
+ // loop of some sort that has a direct parent of the compute construct.
+ // However, a 'break' in a 'switch' marked as a compute construct doesn't
+ // count as 'branch out of' the compute construct.
+ if (S->isOpenACCComputeConstructScope() ||
+ (!S->isDirectlySwitchScope() && S->getParent() &&
erichkeane wrote:
> The first (dangling) case should be handled by getBreakParent() function.
> OpenMP set FnScope (IIRC) to nullify break parent and diagnose dangling
> break/continue and other similar constructs.
I saw that, but it ends up being much worse diagnostics, doesn't it? A case
like above just says "error: 'break' statement not in loop or switch
statement", which seems inaccurate for OpenACC given the 'ignorability'
requirement of the language.
https://github.com/llvm/llvm-project/pull/82543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
https://github.com/erichkeane updated
https://github.com/llvm/llvm-project/pull/82543
>From 5ea47a31eb0ce851441cb7f1851b13303732ca91 Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Wed, 21 Feb 2024 10:08:06 -0800
Subject: [PATCH 1/3] [OpenACC] Implement 'break' and 'continue' errors for
Compute Cnstrcts
OpenACC3.3 2.5.4 says: "A program may not branch into or out of a
compute construct". While some of this restriction isn't particularly
checkable, 'break' and 'continue' are possible and pretty trivial, so
this patch implements those limitations.
It IS unclear in the case of a 'break' in a 'switch' what should happen
(an antagonistic reading of the standard would prevent it from
appearing), however we're choosing to special-case the break-in-switch
to ensure that this works (albeit, a 'parallel' directive on a 'switch'
isn't particularly useful, though permitted).
Future implementations of this rule will be in a follow-up patch.
---
.../clang/Basic/DiagnosticSemaKinds.td| 2 +
clang/include/clang/Sema/Scope.h | 15 +++
clang/lib/Parse/ParseOpenACC.cpp | 17
clang/lib/Sema/SemaStmt.cpp | 22 +
clang/test/SemaOpenACC/no-branch-in-out.c | 95 +++
5 files changed, 151 insertions(+)
create mode 100644 clang/test/SemaOpenACC/no-branch-in-out.c
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 11411883e1bfc6..dbe6eecaa73df4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12201,4 +12201,6 @@ def warn_acc_clause_unimplemented
def err_acc_construct_appertainment
: Error<"OpenACC construct '%0' cannot be used here; it can only "
"be used in a statement context">;
+def err_acc_branch_in_out
+: Error<"invalid branch %select{out of|into}0 OpenACC region">;
} // end of sema component.
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index 9e81706cd2aa1d..3ffcc3590ae0e0 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -150,6 +150,9 @@ class Scope {
/// template scope in between), the outer scope does not increase the
/// depth of recursion.
LambdaScope = 0x800,
+/// This is the scope of an OpenACC Compute Construct, which restricts
+/// jumping into/out of it.
+OpenACCComputeConstructScope = 0x1000,
};
private:
@@ -469,6 +472,12 @@ class Scope {
return false;
}
+ /// Return true if this exact scope (and not one of it's parents) is a switch
+ /// scope.
+ bool isDirectlySwitchScope() const {
+return getFlags() & Scope::SwitchScope;
+ }
+
/// Determines whether this scope is the OpenMP directive scope
bool isOpenMPDirectiveScope() const {
return (getFlags() & Scope::OpenMPDirectiveScope);
@@ -504,6 +513,12 @@ class Scope {
return getFlags() & Scope::OpenMPOrderClauseScope;
}
+ /// Determine whether this scope is the statement associated with an OpenACC
+ /// Compute construct directive.
+ bool isOpenACCComputeConstructScope() const {
+return getFlags() & Scope::OpenACCComputeConstructScope;
+ }
+
/// Determine whether this scope is a while/do/for statement, which can have
/// continue statements embedded into it.
bool isContinueScope() const {
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index 50e78e8687aea1..4946a61fca007f 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -560,6 +560,21 @@ bool doesDirectiveHaveAssociatedStmt(OpenACCDirectiveKind
DirKind) {
llvm_unreachable("Unhandled directive->assoc stmt");
}
+unsigned getOpenACCScopeFlags(OpenACCDirectiveKind DirKind) {
+ switch (DirKind) {
+ case OpenACCDirectiveKind::Parallel:
+// Mark this as a BreakScope/ContinueScope as well as a compute construct
+// so that we can diagnose trying to 'break'/'continue' inside of one.
+return Scope::BreakScope | Scope::ContinueScope |
+ Scope::OpenACCComputeConstructScope;
+ case OpenACCDirectiveKind::Invalid:
+llvm_unreachable("Shouldn't be creating a scope for an invalid construct");
+ default:
+break;
+ }
+ return 0;
+}
+
} // namespace
// OpenACC 3.3, section 1.7:
@@ -1228,6 +1243,8 @@ StmtResult Parser::ParseOpenACCDirectiveStmt() {
if (doesDirectiveHaveAssociatedStmt(DirInfo.DirKind)) {
ParsingOpenACCDirectiveRAII DirScope(*this, /*Value=*/false);
+ParseScope ACCScope(this, getOpenACCScopeFlags(DirInfo.DirKind));
+
AssocStmt = getActions().ActOnOpenACCAssociatedStmt(DirInfo.DirKind,
ParseStatement());
}
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index dde3bd84e89f8b..b1025c05e5db50 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3356,6 +3356,14 @@
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
@@ -3371,6 +3379,20 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope
*CurScope) {
if (S->isOpenMPLoopScope())
return StmtError(Diag(BreakLoc, diag::err_omp_loop_cannot_use_stmt)
<< "break");
+
+ // OpenACC doesn't allow 'break'ing from a compute construct, so diagnose if
+ // we are trying to do so. This can come in 2 flavors: 1-the break'able
thing
+ // (besides the compute construct) 'contains' the compute construct, at which
+ // point the 'break' scope will be the compute construct. Else it could be a
+ // loop of some sort that has a direct parent of the compute construct.
+ // However, a 'break' in a 'switch' marked as a compute construct doesn't
+ // count as 'branch out of' the compute construct.
+ if (S->isOpenACCComputeConstructScope() ||
+ (!S->isDirectlySwitchScope() && S->getParent() &&
erichkeane wrote:
OpenACC compute constructs are NOT limited to loops unfortunately, they can be
any statement, so just checking the loop isn't sufficient. And checking the
'is loop with a parent openacc scope' doesn't work if the openacc-pragma is
inside the loop. See my 1st example above, the pragma is the 'immediate'
scope, but the 'switch' is above it (or the loop in that case).
https://github.com/llvm/llvm-project/pull/82543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
erichkeane wrote: > > Also note, this is missing Clang lit tests, and doesn't seem to be > > correctly handling dependent expressions for x,y, and z. > > What does it mean to "handle dependent expressions for x,y, and z"? Thanks! An expression can be 'dependent', which is a C++'ism for 'has some sort of dependence on a containing template' SO something like: ``` template [[clang::amdgpu_num_work_groups(T::value, std::is_aggregate_vhttps://github.com/llvm/llvm-project/pull/79035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
@@ -3371,6 +3379,20 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope
*CurScope) {
if (S->isOpenMPLoopScope())
return StmtError(Diag(BreakLoc, diag::err_omp_loop_cannot_use_stmt)
<< "break");
+
+ // OpenACC doesn't allow 'break'ing from a compute construct, so diagnose if
+ // we are trying to do so. This can come in 2 flavors: 1-the break'able
thing
+ // (besides the compute construct) 'contains' the compute construct, at which
+ // point the 'break' scope will be the compute construct. Else it could be a
+ // loop of some sort that has a direct parent of the compute construct.
+ // However, a 'break' in a 'switch' marked as a compute construct doesn't
+ // count as 'branch out of' the compute construct.
+ if (S->isOpenACCComputeConstructScope() ||
+ (!S->isDirectlySwitchScope() && S->getParent() &&
erichkeane wrote:
This is covering 2 separate cases.
First:
```
switch(F) { <-- Switch scope
case 1:
#pragma acc parallel <-- compute-construct scope
break;
}
```
In this case, this is the pre-'||' check, the pragma's scope itself (created in
ParseOpenACC.cpp) will be the 'break parent' of current, which cannot be broken
from (so it is diagnosed). This can NEVER be a 'switch' scope, as it is a
'pragma acc compute-construct' scope.
Second:
```
#pragma acc parallel <-- compute construct scope
switch(F) { <-- switch scope
case 1:
break;
}
```
This is how the OpenMP variant of this diagnostic works, the 'scope' here is
the 'switch scope'. Additionally, the 'parent' of this one is the Compute
Construct scope. So everything after the '||' handles this. (note that
`isOpenMPLoopScope` handles this by implicitly checking the 'parent'). Since
we have the 'switch' scope (or other loop/scope/etc), we have to check that it
is directly inside of the pragma scope. Since this one CAN be a 'switch'/any
other loop/scope/etc, we have to check it, as well as checking the 'parent' for
being an OpenACC Compute construct.
https://github.com/llvm/llvm-project/pull/82543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
https://github.com/erichkeane updated
https://github.com/llvm/llvm-project/pull/82543
>From 5ea47a31eb0ce851441cb7f1851b13303732ca91 Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Wed, 21 Feb 2024 10:08:06 -0800
Subject: [PATCH 1/2] [OpenACC] Implement 'break' and 'continue' errors for
Compute Cnstrcts
OpenACC3.3 2.5.4 says: "A program may not branch into or out of a
compute construct". While some of this restriction isn't particularly
checkable, 'break' and 'continue' are possible and pretty trivial, so
this patch implements those limitations.
It IS unclear in the case of a 'break' in a 'switch' what should happen
(an antagonistic reading of the standard would prevent it from
appearing), however we're choosing to special-case the break-in-switch
to ensure that this works (albeit, a 'parallel' directive on a 'switch'
isn't particularly useful, though permitted).
Future implementations of this rule will be in a follow-up patch.
---
.../clang/Basic/DiagnosticSemaKinds.td| 2 +
clang/include/clang/Sema/Scope.h | 15 +++
clang/lib/Parse/ParseOpenACC.cpp | 17
clang/lib/Sema/SemaStmt.cpp | 22 +
clang/test/SemaOpenACC/no-branch-in-out.c | 95 +++
5 files changed, 151 insertions(+)
create mode 100644 clang/test/SemaOpenACC/no-branch-in-out.c
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 11411883e1bfc6..dbe6eecaa73df4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12201,4 +12201,6 @@ def warn_acc_clause_unimplemented
def err_acc_construct_appertainment
: Error<"OpenACC construct '%0' cannot be used here; it can only "
"be used in a statement context">;
+def err_acc_branch_in_out
+: Error<"invalid branch %select{out of|into}0 OpenACC region">;
} // end of sema component.
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index 9e81706cd2aa1d..3ffcc3590ae0e0 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -150,6 +150,9 @@ class Scope {
/// template scope in between), the outer scope does not increase the
/// depth of recursion.
LambdaScope = 0x800,
+/// This is the scope of an OpenACC Compute Construct, which restricts
+/// jumping into/out of it.
+OpenACCComputeConstructScope = 0x1000,
};
private:
@@ -469,6 +472,12 @@ class Scope {
return false;
}
+ /// Return true if this exact scope (and not one of it's parents) is a switch
+ /// scope.
+ bool isDirectlySwitchScope() const {
+return getFlags() & Scope::SwitchScope;
+ }
+
/// Determines whether this scope is the OpenMP directive scope
bool isOpenMPDirectiveScope() const {
return (getFlags() & Scope::OpenMPDirectiveScope);
@@ -504,6 +513,12 @@ class Scope {
return getFlags() & Scope::OpenMPOrderClauseScope;
}
+ /// Determine whether this scope is the statement associated with an OpenACC
+ /// Compute construct directive.
+ bool isOpenACCComputeConstructScope() const {
+return getFlags() & Scope::OpenACCComputeConstructScope;
+ }
+
/// Determine whether this scope is a while/do/for statement, which can have
/// continue statements embedded into it.
bool isContinueScope() const {
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index 50e78e8687aea1..4946a61fca007f 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -560,6 +560,21 @@ bool doesDirectiveHaveAssociatedStmt(OpenACCDirectiveKind
DirKind) {
llvm_unreachable("Unhandled directive->assoc stmt");
}
+unsigned getOpenACCScopeFlags(OpenACCDirectiveKind DirKind) {
+ switch (DirKind) {
+ case OpenACCDirectiveKind::Parallel:
+// Mark this as a BreakScope/ContinueScope as well as a compute construct
+// so that we can diagnose trying to 'break'/'continue' inside of one.
+return Scope::BreakScope | Scope::ContinueScope |
+ Scope::OpenACCComputeConstructScope;
+ case OpenACCDirectiveKind::Invalid:
+llvm_unreachable("Shouldn't be creating a scope for an invalid construct");
+ default:
+break;
+ }
+ return 0;
+}
+
} // namespace
// OpenACC 3.3, section 1.7:
@@ -1228,6 +1243,8 @@ StmtResult Parser::ParseOpenACCDirectiveStmt() {
if (doesDirectiveHaveAssociatedStmt(DirInfo.DirKind)) {
ParsingOpenACCDirectiveRAII DirScope(*this, /*Value=*/false);
+ParseScope ACCScope(this, getOpenACCScopeFlags(DirInfo.DirKind));
+
AssocStmt = getActions().ActOnOpenACCAssociatedStmt(DirInfo.DirKind,
ParseStatement());
}
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index dde3bd84e89f8b..b1025c05e5db50 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3356,6 +3356,14 @@
[clang] [OpenACC] Implement 'break' and 'continue' errors for Compute Cnstrcts (PR #82543)
https://github.com/erichkeane created
https://github.com/llvm/llvm-project/pull/82543
OpenACC3.3 2.5.4 says: "A program may not branch into or out of a compute
construct". While some of this restriction isn't particularly checkable,
'break' and 'continue' are possible and pretty trivial, so this patch
implements those limitations.
It IS unclear in the case of a 'break' in a 'switch' what should happen (an
antagonistic reading of the standard would prevent it from
appearing), however we're choosing to special-case the break-in-switch
to ensure that this works (albeit, a 'parallel' directive on a 'switch' isn't
particularly useful, though permitted).
Future implementations of this rule will be in a follow-up patch.
>From 5ea47a31eb0ce851441cb7f1851b13303732ca91 Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Wed, 21 Feb 2024 10:08:06 -0800
Subject: [PATCH] [OpenACC] Implement 'break' and 'continue' errors for Compute
Cnstrcts
OpenACC3.3 2.5.4 says: "A program may not branch into or out of a
compute construct". While some of this restriction isn't particularly
checkable, 'break' and 'continue' are possible and pretty trivial, so
this patch implements those limitations.
It IS unclear in the case of a 'break' in a 'switch' what should happen
(an antagonistic reading of the standard would prevent it from
appearing), however we're choosing to special-case the break-in-switch
to ensure that this works (albeit, a 'parallel' directive on a 'switch'
isn't particularly useful, though permitted).
Future implementations of this rule will be in a follow-up patch.
---
.../clang/Basic/DiagnosticSemaKinds.td| 2 +
clang/include/clang/Sema/Scope.h | 15 +++
clang/lib/Parse/ParseOpenACC.cpp | 17
clang/lib/Sema/SemaStmt.cpp | 22 +
clang/test/SemaOpenACC/no-branch-in-out.c | 95 +++
5 files changed, 151 insertions(+)
create mode 100644 clang/test/SemaOpenACC/no-branch-in-out.c
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 11411883e1bfc6..dbe6eecaa73df4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12201,4 +12201,6 @@ def warn_acc_clause_unimplemented
def err_acc_construct_appertainment
: Error<"OpenACC construct '%0' cannot be used here; it can only "
"be used in a statement context">;
+def err_acc_branch_in_out
+: Error<"invalid branch %select{out of|into}0 OpenACC region">;
} // end of sema component.
diff --git a/clang/include/clang/Sema/Scope.h b/clang/include/clang/Sema/Scope.h
index 9e81706cd2aa1d..3ffcc3590ae0e0 100644
--- a/clang/include/clang/Sema/Scope.h
+++ b/clang/include/clang/Sema/Scope.h
@@ -150,6 +150,9 @@ class Scope {
/// template scope in between), the outer scope does not increase the
/// depth of recursion.
LambdaScope = 0x800,
+/// This is the scope of an OpenACC Compute Construct, which restricts
+/// jumping into/out of it.
+OpenACCComputeConstructScope = 0x1000,
};
private:
@@ -469,6 +472,12 @@ class Scope {
return false;
}
+ /// Return true if this exact scope (and not one of it's parents) is a switch
+ /// scope.
+ bool isDirectlySwitchScope() const {
+return getFlags() & Scope::SwitchScope;
+ }
+
/// Determines whether this scope is the OpenMP directive scope
bool isOpenMPDirectiveScope() const {
return (getFlags() & Scope::OpenMPDirectiveScope);
@@ -504,6 +513,12 @@ class Scope {
return getFlags() & Scope::OpenMPOrderClauseScope;
}
+ /// Determine whether this scope is the statement associated with an OpenACC
+ /// Compute construct directive.
+ bool isOpenACCComputeConstructScope() const {
+return getFlags() & Scope::OpenACCComputeConstructScope;
+ }
+
/// Determine whether this scope is a while/do/for statement, which can have
/// continue statements embedded into it.
bool isContinueScope() const {
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index 50e78e8687aea1..4946a61fca007f 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -560,6 +560,21 @@ bool doesDirectiveHaveAssociatedStmt(OpenACCDirectiveKind
DirKind) {
llvm_unreachable("Unhandled directive->assoc stmt");
}
+unsigned getOpenACCScopeFlags(OpenACCDirectiveKind DirKind) {
+ switch (DirKind) {
+ case OpenACCDirectiveKind::Parallel:
+// Mark this as a BreakScope/ContinueScope as well as a compute construct
+// so that we can diagnose trying to 'break'/'continue' inside of one.
+return Scope::BreakScope | Scope::ContinueScope |
+ Scope::OpenACCComputeConstructScope;
+ case OpenACCDirectiveKind::Invalid:
+llvm_unreachable("Shouldn't be creating a scope for an invalid construct");
+ default:
+break;
+ }
+ return 0;
+}
+
} // namespace
// OpenACC 3.3,
[clang] [attributes][analyzer] Generalize [[clang::suppress]] to declarations. (PR #80371)
erichkeane wrote: > Ok gotcha thanks! In any case, I'll do my best to handle this more gracefully > in the future. Your advice is always appreciated! Perfect! I'll try to be better about this in the future as well. https://github.com/llvm/llvm-project/pull/80371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Defer instantiation of exception specification until after partial ordering when determining primary template (PR #82417)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/82417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Defer instantiation of exception specification until after partial ordering when determining primary template (PR #82417)
https://github.com/erichkeane commented: 1 nit, else LGTM. We should probably have a release note as well. https://github.com/llvm/llvm-project/pull/82417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Defer instantiation of exception specification until after partial ordering when determining primary template (PR #82417)
@@ -9706,9 +9706,17 @@ bool Sema::CheckFunctionTemplateSpecialization( if (Result == Candidates.end()) return true; - // Ignore access information; it doesn't figure into redeclaration checking. FunctionDecl *Specialization = cast(*Result); + auto *SpecializationFPT = erichkeane wrote: Can you quote the standard part here in comments for future reference? https://github.com/llvm/llvm-project/pull/82417 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][analyzer] Generalize [[clang::suppress]] to declarations. (PR #80371)
erichkeane wrote: > Hmm, no, I landed it because I made an assumption that there's simply not > that much interest in this work (I'm quite depressed about this in general > lately) so as a code owner I just made a call that it's probably good enough > to go and rely on post-commit review. Now that you bring this up, it does > sound a lot like I should get myself out of this mindset and at least ping > people first. Especially Erich who I specifically invited as the code owner > of clang attributes. Absolutely my bad. I definitely see how this isn't great > moving forward and I will do better from now on. Should I also revert and > give you folks time to properly review and course-correct me? I hadn't noticed that you were the Analysis code owner. I don't really see anything in the Clang stuff (which IS attributes and my perview) that require a revert. Sorry for letting this drop off my backlog, I did one review on it at one point, then for some reason didn't come back to it. A ping would have been appreciated (for next time), but no need to revert this time. https://github.com/llvm/llvm-project/pull/80371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [attributes][analyzer] Generalize [[clang::suppress]] to declarations. (PR #80371)
erichkeane wrote: > did I miss something - it looks like this was committed without approval? It looks that way to me @haoNoQ : Did you commit this instead of something else? Can you revert this until we get approval? https://github.com/llvm/llvm-project/pull/80371 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix incorrect rejection default construction of union with nontrivial member (PR #82407)
@@ -188,3 +188,14 @@ static_assert(U2().b.x == 100, "");
static_assert(U3().b.x == 100, "");
} // namespace GH48416
+
+namespace GH81774 {
+struct Handle {
+Handle(int) {}
+};
+// Should be well-formed b/c NoState has a brace-or-equal-initializer
erichkeane wrote:
```suggestion
// Should be well-formed because NoState has a brace-or-equal-initializer.
```
https://github.com/llvm/llvm-project/pull/82407
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Fix incorrect rejection default construction of union with nontrivial member (PR #82407)
@@ -9442,9 +9442,21 @@ bool
SpecialMemberDeletionInfo::shouldDeleteForSubobjectCall(
int DiagKind = -1;
- if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::NoMemberOrDeleted)
-DiagKind = !Decl ? 0 : 1;
- else if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::Ambiguous)
+ if (SMOR.getKind() == Sema::SpecialMemberOverloadResult::NoMemberOrDeleted) {
+if (CSM == Sema::CXXDefaultConstructor && Field &&
+Field->getParent()->isUnion()) {
+ // [class.default.ctor]p2:
+ // A defaulted default constructor for class X is defined as deleted if
+ // - X is a union that has a variant member with a non-trivial default
+ // constructor and no variant member of X has a default member
+ // initializer
+ const auto *RD = cast(Field->getParent());
+ if (!RD->hasInClassInitializer())
+DiagKind = !Decl ? 0 : 1;
erichkeane wrote:
So what is going on here? this is the same value as its 'else' clause, and
leaves DiagKind as -1 otherwise? I might suggest making this a single 'if' and
getting rid of the 'else' here.
https://github.com/llvm/llvm-project/pull/82407
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
erichkeane wrote: Also note, this is missing Clang lit tests, and doesn't seem to be correctly handling dependent expressions for x,y, and z. https://github.com/llvm/llvm-project/pull/79035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -8069,6 +8069,26 @@ static void handleAMDGPUNumVGPRAttr(Sema , Decl *D,
const ParsedAttr ) {
D->addAttr(::new (S.Context) AMDGPUNumVGPRAttr(S.Context, AL, NumVGPR));
}
+static void handleAMDGPUMaxNumWorkGroupsAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+ uint32_t NumWGX = 0;
+ uint32_t NumWGY = 0;
+ uint32_t NumWGZ = 0;
+ Expr *NumWGXExpr = AL.getArgAsExpr(0);
+ Expr *NumWGYExpr = AL.getArgAsExpr(1);
+ Expr *NumWGZExpr = AL.getArgAsExpr(2);
+ if (!checkUInt32Argument(S, AL, NumWGXExpr, NumWGX))
+return;
+ if (!checkUInt32Argument(S, AL, NumWGYExpr, NumWGY))
+return;
+ if (!checkUInt32Argument(S, AL, NumWGZExpr, NumWGZ))
+return;
+
+ if (NumWGX != 0 && NumWGY != 0 && NumWGZ != 0)
erichkeane wrote:
Silently ignoring this is not acceptable. We need to diagnose this as an
error/warning in the compiler.
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Regroup declarations in `Sema` (PR #82217)
erichkeane wrote: > @erichkeane > > > That said, waiting until after 18 is perhaps a good diea. > > Resolving merge conflicts that will arise in the meantime is not going to be > trivial, but should be doable in a reasonable time. So I'm willing to wait. > I'm glad to hear that! I'm hopeful it isn't TOO much of a delay and not too much work. Sema.h changes are at least pretty innocuous/small. > > I MIGHT suggest private followed by public? It is a not-uncommon pattern > > I've seen to have a private 'helper' class(or data member) defined inline, > > that is then used by inline-defined public functions. > > I don't mind either way, as long as we don't inserting a couple of private > members in the middle of public section. > I agree with the goal as well! > > I wouldn't mind some sort of 'static_assert' to ensure that this doesn't > > accidentally increase the size of Sema > > Checking on Linux, `sizeof(Sema)` is 18824 bytes at moment. This patch > increases that by 24 bytes, to `18848`. I don't deem it significant enough to > care for an object this big, but I can claw it back if this is considered > important. > Urgh, and double-urgh. Yeah, not big enough/important enough, we only ever instantiate a handful of Sema objects anyway at most. > > or cause some sort of pessimization for layout. I realize we're not > > particularly concerned about the size, but I could imagine goofy things > > going on. > > If there was an intent to put some data members first to improve cache hits, > I think it has been lost by this point. Our first non-static data members are > `OpenCLOptions OpenCLFeatures` and `FPOptions CurFPFeatures` at the moment. > Since this patch puts generic Sema stuff (`Sema.cpp`) first, opportunity to > put widely used data members first is still there. As I mentioned in the > description, follow-up patches that improve ordering in each particular > section are expected. "Commonly used" stuff together perhaps has some value for cache-locality reasons (as well as any other "closely related" stuff with itself), but I don't have a good way to measure that so 'best effort' here is probably good enough/the changes you're intending a good enough attempt. https://github.com/llvm/llvm-project/pull/82217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Convert warning for extraneous template parameter lists to an extension warning (PR #82277)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/82277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Diagnose declarative nested-name-specifiers naming alias templates (PR #80842)
erichkeane wrote: I think we're good, feel free to resolve your conflict and commit. https://github.com/llvm/llvm-project/pull/80842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -8069,6 +8069,26 @@ static void handleAMDGPUNumVGPRAttr(Sema , Decl *D,
const ParsedAttr ) {
D->addAttr(::new (S.Context) AMDGPUNumVGPRAttr(S.Context, AL, NumVGPR));
}
+static void handleAMDGPUMaxNumWorkGroupsAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+ uint32_t NumWGX = 0;
+ uint32_t NumWGY = 0;
+ uint32_t NumWGZ = 0;
+ Expr *NumWGXExpr = AL.getArgAsExpr(0);
+ Expr *NumWGYExpr = AL.getArgAsExpr(1);
+ Expr *NumWGZExpr = AL.getArgAsExpr(2);
+ if (!checkUInt32Argument(S, AL, NumWGXExpr, NumWGX))
+return;
+ if (!checkUInt32Argument(S, AL, NumWGYExpr, NumWGY))
+return;
+ if (!checkUInt32Argument(S, AL, NumWGZExpr, NumWGZ))
+return;
+
+ if (NumWGX != 0 && NumWGY != 0 && NumWGZ != 0)
erichkeane wrote:
This needs to diagnose.
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -2705,6 +2705,33 @@ An error will be given if:
}];
}
+def AMDGPUMaxNumWorkGroupsDocs : Documentation {
+ let Category = DocCatAMDGPUAttributes;
+ let Content = [{
+This attribute specifies the max number of work groups when the kernel
+is dispatched.
+
+Clang supports the
+``__attribute__((amdgpu_max_num_work_groups(, , )))`` or
+``[[clang::amdgpu_max_num_work_groups(, , )]]`` attribute for the
+AMDGPU target. This attribute may be attached to HIP or OpenCL kernel function
+definitions and is an optimization hint.
+
+ parameter specifies the maximum number of work groups in the x
dimension.
erichkeane wrote:
```suggestion
The parameter specifies the maximum number of work groups in the x
dimension.
```
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -2705,6 +2705,33 @@ An error will be given if:
}];
}
+def AMDGPUMaxNumWorkGroupsDocs : Documentation {
+ let Category = DocCatAMDGPUAttributes;
+ let Content = [{
+This attribute specifies the max number of work groups when the kernel
+is dispatched.
+
+Clang supports the
+``__attribute__((amdgpu_max_num_work_groups(, , )))`` or
+``[[clang::amdgpu_max_num_work_groups(, , )]]`` attribute for the
+AMDGPU target. This attribute may be attached to HIP or OpenCL kernel function
+definitions and is an optimization hint.
+
+ parameter specifies the maximum number of work groups in the x
dimension.
+Similarly and are for the y and z dimensions respectively.
+Each of the three numbers must be >=1. The attribute is ignored if any of the
erichkeane wrote:
```suggestion
Each of the three values must be greater than zero. The attribute is ignored if
any of the
```
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -2705,6 +2705,33 @@ An error will be given if:
}];
}
+def AMDGPUMaxNumWorkGroupsDocs : Documentation {
+ let Category = DocCatAMDGPUAttributes;
+ let Content = [{
+This attribute specifies the max number of work groups when the kernel
+is dispatched.
+
+Clang supports the
+``__attribute__((amdgpu_max_num_work_groups(, , )))`` or
+``[[clang::amdgpu_max_num_work_groups(, , )]]`` attribute for the
+AMDGPU target. This attribute may be attached to HIP or OpenCL kernel function
+definitions and is an optimization hint.
+
+ parameter specifies the maximum number of work groups in the x
dimension.
+Similarly and are for the y and z dimensions respectively.
+Each of the three numbers must be >=1. The attribute is ignored if any of the
erichkeane wrote:
I don't think 'ignored' is the right semantics here: that should diagnose.
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
https://github.com/erichkeane commented: Reviewed the CFE component, didn't look at LLVM. https://github.com/llvm/llvm-project/pull/79035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/79035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Regroup declarations in `Sema` (PR #82217)
erichkeane wrote: > This is going to be rather disruptive on downstream projects. At least we > should wait until after the release of clang 18 to merge it, to avoid endless > merge conflicts For the most part, git will handle these pretty well on downstreams I think, and as they are declarations, I suspect this isn't going to be super tough of a merge conflict. That said, waiting until after 18 is perhaps a good diea. > Table of contents added at the very beginning of `Sema`. Grouping is > reflected in Doxygen commands, so structure of API reference of `Sema` is > also significantly improved ([example from official > documentation](https://www.doxygen.nl/manual/examples/memgrp/html/class_memgrp___test.html)). Neat! > > While grouping is intentional, as well as each group consisting of `public` > declarations followed by `private` ones (without changing access in-between), I MIGHT suggest private followed by public? It is a not-uncommon pattern I've seen to have a private 'helper' class(or data member) defined inline, that is then used by inline-defined public functions. >Data members and inline function definitions in `Sema.h` complicate the >matter, since it's not obvious which group they belong to. Further work is >expected to refine contents and order of declarations. This IS going to be complicated unfortunately. Many of the function definitions at least are going to be 1-liners that are just calling into a different definition. Data members you'll probably have to hunt down where they are used to see the relationship. > What is also intentional is some kind of layering, where Concepts group > follows template groups, and ObjC, code completion, CUDA, HLSL, OpenACC, > OpenMP, and SYCL are all placed at the end of the file, after C and C++ parts > of `Sema`. > Member initializer list of `Sema` in `Sema.cpp` is rewritten to reflect new > order of data members in order to avoid `-Wreorder-ctor`. I wouldn't mind some sort of 'static_assert' to ensure that this doesn't accidentally increase the size of Sema or cause some sort of pessimization for layout. I realize we're not particularly concerned about the size, but I could imagine goofy things going on. https://github.com/llvm/llvm-project/pull/82217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Diagnose misuse of the cleanup attribute (PR #80040)
erichkeane wrote: > what would be the reason for windows build failing , is it a CI issue or > specific to this PR & what can I do to resolve that. Thank you That appears to be a problem with the CI itself. I think we've fixed up a bunch of the CI, but it will require doing a 'merge' with main. That said, I think you're OK to just squash/merge as-is. https://github.com/llvm/llvm-project/pull/80040 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C23] Add __TYPE_FMTB__ and __TYPE_FMTb__ predefined macros (PR #82037)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/82037 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C11] Diagnose C11 keywords as being incompatible w/earlier standards (PR #82015)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/82015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C11] Diagnose C11 keywords as being incompatible w/earlier standards (PR #82015)
@@ -2742,6 +2742,19 @@ bool Parser::parseMisplacedModuleImport() {
return false;
}
+void Parser::diagnoseUseOfC11Keyword(const Token& Tok) {
+ // Warn that this is a C11 extension if in an older mode or if in C++.
+ // Otherwise, warn that it is incompatible with standards before C11 if in
+ // C11 or later.
+ unsigned DiagId;
+ if (!getLangOpts().C11) {
+DiagId = diag::ext_c11_feature;
+ } else {
+DiagId = diag::warn_c11_compat_keyword;
+ }
+ Diag(Tok, DiagId) << Tok.getName();
erichkeane wrote:
I dont think it is worth the DiagId pattern here, I think just a Diag + Ternary
is probably fine.
https://github.com/llvm/llvm-project/pull/82015
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [C11] Diagnose C11 keywords as being incompatible w/earlier standards (PR #82015)
@@ -2742,6 +2742,19 @@ bool Parser::parseMisplacedModuleImport() {
return false;
}
+void Parser::diagnoseUseOfC11Keyword(const Token& Tok) {
+ // Warn that this is a C11 extension if in an older mode or if in C++.
+ // Otherwise, warn that it is incompatible with standards before C11 if in
+ // C11 or later.
+ unsigned DiagId;
+ if (!getLangOpts().C11) {
erichkeane wrote:
Don't use curleys on 1 liners
https://github.com/llvm/llvm-project/pull/82015
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement beginning parts of the 'parallel' Sema impl (PR #81659)
https://github.com/erichkeane closed https://github.com/llvm/llvm-project/pull/81659 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement beginning parts of the 'parallel' Sema impl (PR #81659)
@@ -0,0 +1,132 @@
+//===--- SemaOpenACC.cpp - 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: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+/// \file
+/// This file implements semantic analysis for OpenACC constructs and
+/// clauses.
+///
+//===--===//
+
+#include "clang/Basic/DiagnosticSema.h"
+#include "clang/Basic/OpenACCKinds.h"
+#include "clang/Sema/Sema.h"
+
+using namespace clang;
+
+namespace {
+bool DiagnoseConstructAppertainment(Sema , OpenACCDirectiveKind K,
+SourceLocation StartLoc, bool IsStmt) {
+ switch (K) {
+ default:
+ case OpenACCDirectiveKind::Invalid:
+// Nothing to do here, both invalid and unimplemented don't really need to
+// do anything.
+break;
erichkeane wrote:
No, that would just end up crashing, we're calling into the 'act on start' even
when invalid, which should allow us to handle 'state' of bad pragmas better, so
'invalid' has to get through here without causing problems. The intent is that
we wont create an AST node here, but still allow 'checking' of everything else.
https://github.com/llvm/llvm-project/pull/81659
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor target attribute mangling. (PR #81893)
erichkeane wrote: > I wonder if this should be a part of the Targets in CodeGen instead of here? Hrm, this is a bad idea as the rest of mangling is in AST, but having mangling outside of AST seems to be the problem here. https://github.com/llvm/llvm-project/pull/81893 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor target attribute mangling. (PR #81893)
@@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_BASIC_TARGETINFO_H #define LLVM_CLANG_BASIC_TARGETINFO_H +#include "clang/AST/Attr.h" erichkeane wrote: Basic shouldn't be referencing AST (IIRC, it shouldn't reference anything but basic or llvm), this ends up being an unfortunate layering violation. https://github.com/llvm/llvm-project/pull/81893 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor target attribute mangling. (PR #81893)
https://github.com/erichkeane commented: I wonder if this should be a part of the Targets in CodeGen instead of here? https://github.com/llvm/llvm-project/pull/81893 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor target attribute mangling. (PR #81893)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/81893 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC][NFC] Implement basic OpenACC Sema infrastructure (PR #81874)
https://github.com/erichkeane closed https://github.com/llvm/llvm-project/pull/81874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC][NFC] Implement basic OpenACC Sema infrastructure (PR #81874)
https://github.com/erichkeane updated
https://github.com/llvm/llvm-project/pull/81874
>From 35700522a658571e2abad279f327f4160fdb6c9d Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Thu, 15 Feb 2024 08:41:58 -0800
Subject: [PATCH 1/3] [OpenACC][NFC] Implement basic OpenACC Sema
infrastructure
This patch is split off from #81659, and contains just the Sema
infrastructure that we can later use to implement semantic analysis of
OpenACC constructs.
---
clang/include/clang/Parse/Parser.h | 16 +-
clang/include/clang/Sema/Sema.h| 41 ++
clang/lib/Parse/ParseOpenACC.cpp | 47 +-
clang/lib/Sema/CMakeLists.txt | 1 +
clang/lib/Sema/SemaOpenACC.cpp | 47 ++
5 files changed, 143 insertions(+), 9 deletions(-)
create mode 100644 clang/lib/Sema/SemaOpenACC.cpp
diff --git a/clang/include/clang/Parse/Parser.h
b/clang/include/clang/Parse/Parser.h
index da18cf88edcc92..69b9e837fe8bef 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3572,7 +3572,21 @@ class Parser : public CodeCompletionHandler {
StmtResult ParseOpenACCDirectiveStmt();
private:
- void ParseOpenACCDirective();
+ /// A struct to hold the information that got parsed by
ParseOpenACCDirective,
+ /// so that the callers of it can use that to construct the appropriate AST
+ /// nodes.
+ struct OpenACCDirectiveParseInfo {
+OpenACCDirectiveKind DirKind;
+SourceLocation StartLoc;
+SourceLocation EndLoc;
+// TODO OpenACC: Add Clause list here once we have a type for that.
+// TODO OpenACC: As we implement support for the Atomic, Routine, Cache,
and
+// Wait constructs, we likely want to put that information in here as well.
+ };
+
+ /// Parses the OpenACC directive (the entire pragma) including the clause
+ /// list, but does not produce the main AST node.
+ OpenACCDirectiveParseInfo ParseOpenACCDirective();
/// Helper that parses an ID Expression based on the language options.
ExprResult ParseOpenACCIDExpression();
/// Parses the variable list for the `cache` construct.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ed933f27f8df6b..ec4c451c584612 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -41,6 +41,7 @@
#include "clang/Basic/DarwinSDKInfo.h"
#include "clang/Basic/ExpressionTraits.h"
#include "clang/Basic/Module.h"
+#include "clang/Basic/OpenACCKinds.h"
#include "clang/Basic/OpenCLOptions.h"
#include "clang/Basic/OpenMPKinds.h"
#include "clang/Basic/PragmaKinds.h"
@@ -12704,6 +12705,46 @@ class Sema final {
OMPClause *ActOnOpenMPXBareClause(SourceLocation StartLoc,
SourceLocation EndLoc);
+
//======//
+ // OpenACC directives and clauses.
+
+ /// Called after parsing an OpenACC Clause so that it can be checked.
+ bool ActOnOpenACCClause(OpenACCClauseKind ClauseKind,
+ SourceLocation StartLoc);
+
+ /// Called after the construct has been parsed, but clauses haven't been
+ /// parsed. This allows us to diagnose not-implemented, as well as set up
any
+ /// state required for parsing the clauses.
+ void ActOnOpenACCConstruct(OpenACCDirectiveKind K, SourceLocation StartLoc);
+
+ /// Called after the directive, including its clauses, have been parsed and
+ /// parsing has consumed the 'annot_pragma_openacc_end' token. This DOES
+ /// happen before any associated declarations or statements have been parsed.
+ /// This function is only called when we are parsing a 'statement' context.
+ bool ActOnStartOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc);
+
+ /// Called after the directive, including its clauses, have been parsed and
+ /// parsing has consumed the 'annot_pragma_openacc_end' token. This DOES
+ /// happen before any associated declarations or statements have been parsed.
+ /// This function is only called when we are parsing a 'Decl' context.
+ bool ActOnStartOpenACCDeclDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc);
+ /// Called when we encounter an associated statement for our construct, this
+ /// should check legality of the statement as it appertains to this
Construct.
+ StmtResult ActOnOpenACCAssociatedStmt(OpenACCDirectiveKind K,
+StmtResult AssocStmt);
+
+ /// Called after the directive has been completely parsed, including the
+ /// declaration group or associated statement.
+ StmtResult ActOnEndOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc,
+ SourceLocation EndLoc,
+ StmtResult AssocStmt);
+ /// Called after the
[clang] [OpenACC][NFC] Implement basic OpenACC Sema infrastructure (PR #81874)
https://github.com/erichkeane updated
https://github.com/llvm/llvm-project/pull/81874
>From 35700522a658571e2abad279f327f4160fdb6c9d Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Thu, 15 Feb 2024 08:41:58 -0800
Subject: [PATCH 1/2] [OpenACC][NFC] Implement basic OpenACC Sema
infrastructure
This patch is split off from #81659, and contains just the Sema
infrastructure that we can later use to implement semantic analysis of
OpenACC constructs.
---
clang/include/clang/Parse/Parser.h | 16 +-
clang/include/clang/Sema/Sema.h| 41 ++
clang/lib/Parse/ParseOpenACC.cpp | 47 +-
clang/lib/Sema/CMakeLists.txt | 1 +
clang/lib/Sema/SemaOpenACC.cpp | 47 ++
5 files changed, 143 insertions(+), 9 deletions(-)
create mode 100644 clang/lib/Sema/SemaOpenACC.cpp
diff --git a/clang/include/clang/Parse/Parser.h
b/clang/include/clang/Parse/Parser.h
index da18cf88edcc92..69b9e837fe8bef 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3572,7 +3572,21 @@ class Parser : public CodeCompletionHandler {
StmtResult ParseOpenACCDirectiveStmt();
private:
- void ParseOpenACCDirective();
+ /// A struct to hold the information that got parsed by
ParseOpenACCDirective,
+ /// so that the callers of it can use that to construct the appropriate AST
+ /// nodes.
+ struct OpenACCDirectiveParseInfo {
+OpenACCDirectiveKind DirKind;
+SourceLocation StartLoc;
+SourceLocation EndLoc;
+// TODO OpenACC: Add Clause list here once we have a type for that.
+// TODO OpenACC: As we implement support for the Atomic, Routine, Cache,
and
+// Wait constructs, we likely want to put that information in here as well.
+ };
+
+ /// Parses the OpenACC directive (the entire pragma) including the clause
+ /// list, but does not produce the main AST node.
+ OpenACCDirectiveParseInfo ParseOpenACCDirective();
/// Helper that parses an ID Expression based on the language options.
ExprResult ParseOpenACCIDExpression();
/// Parses the variable list for the `cache` construct.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ed933f27f8df6b..ec4c451c584612 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -41,6 +41,7 @@
#include "clang/Basic/DarwinSDKInfo.h"
#include "clang/Basic/ExpressionTraits.h"
#include "clang/Basic/Module.h"
+#include "clang/Basic/OpenACCKinds.h"
#include "clang/Basic/OpenCLOptions.h"
#include "clang/Basic/OpenMPKinds.h"
#include "clang/Basic/PragmaKinds.h"
@@ -12704,6 +12705,46 @@ class Sema final {
OMPClause *ActOnOpenMPXBareClause(SourceLocation StartLoc,
SourceLocation EndLoc);
+
//======//
+ // OpenACC directives and clauses.
+
+ /// Called after parsing an OpenACC Clause so that it can be checked.
+ bool ActOnOpenACCClause(OpenACCClauseKind ClauseKind,
+ SourceLocation StartLoc);
+
+ /// Called after the construct has been parsed, but clauses haven't been
+ /// parsed. This allows us to diagnose not-implemented, as well as set up
any
+ /// state required for parsing the clauses.
+ void ActOnOpenACCConstruct(OpenACCDirectiveKind K, SourceLocation StartLoc);
+
+ /// Called after the directive, including its clauses, have been parsed and
+ /// parsing has consumed the 'annot_pragma_openacc_end' token. This DOES
+ /// happen before any associated declarations or statements have been parsed.
+ /// This function is only called when we are parsing a 'statement' context.
+ bool ActOnStartOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc);
+
+ /// Called after the directive, including its clauses, have been parsed and
+ /// parsing has consumed the 'annot_pragma_openacc_end' token. This DOES
+ /// happen before any associated declarations or statements have been parsed.
+ /// This function is only called when we are parsing a 'Decl' context.
+ bool ActOnStartOpenACCDeclDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc);
+ /// Called when we encounter an associated statement for our construct, this
+ /// should check legality of the statement as it appertains to this
Construct.
+ StmtResult ActOnOpenACCAssociatedStmt(OpenACCDirectiveKind K,
+StmtResult AssocStmt);
+
+ /// Called after the directive has been completely parsed, including the
+ /// declaration group or associated statement.
+ StmtResult ActOnEndOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc,
+ SourceLocation EndLoc,
+ StmtResult AssocStmt);
+ /// Called after the
[clang] [OpenACC][NFC] Implement basic OpenACC Sema infrastructure (PR #81874)
@@ -1197,7 +1215,20 @@ StmtResult Parser::ParseOpenACCDirectiveStmt() {
ParsingOpenACCDirectiveRAII DirScope(*this);
ConsumeAnnotationToken();
- ParseOpenACCDirective();
+ OpenACCDirectiveParseInfo DirInfo = ParseOpenACCDirective();
+ if (getActions().ActOnStartOpenACCDeclDirective(DirInfo.DirKind,
+ DirInfo.StartLoc))
+return StmtError();
+
+ StmtResult AssocStmt;
+
+ if (doesDirectiveHaveAssociatedStmt(DirInfo.DirKind)) {
erichkeane wrote:
This section ends up being slightly worse without being able to check the AST
kind here, so we need a separate function to check whether we need to parse
one. However, this removes the ASTContext function that was troublesome.
https://github.com/llvm/llvm-project/pull/81874
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC][NFC] Implement basic OpenACC Sema infrastructure (PR #81874)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/81874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC][NFC] Implement basic OpenACC Sema infrastructure (PR #81874)
https://github.com/erichkeane commented: @alexey-bataev : as requested, I've split off the infrastructure bits. Once this is committed, I'll move the 'warning's as a Review-after-commit so that the other patch can be easier to review. https://github.com/llvm/llvm-project/pull/81874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC][NFC] Implement basic OpenACC Sema infrastructure (PR #81874)
https://github.com/erichkeane created
https://github.com/llvm/llvm-project/pull/81874
This patch is split off from #81659, and contains just the Sema infrastructure
that we can later use to implement semantic analysis of OpenACC constructs.
>From 35700522a658571e2abad279f327f4160fdb6c9d Mon Sep 17 00:00:00 2001
From: erichkeane
Date: Thu, 15 Feb 2024 08:41:58 -0800
Subject: [PATCH] [OpenACC][NFC] Implement basic OpenACC Sema infrastructure
This patch is split off from #81659, and contains just the Sema
infrastructure that we can later use to implement semantic analysis of
OpenACC constructs.
---
clang/include/clang/Parse/Parser.h | 16 +-
clang/include/clang/Sema/Sema.h| 41 ++
clang/lib/Parse/ParseOpenACC.cpp | 47 +-
clang/lib/Sema/CMakeLists.txt | 1 +
clang/lib/Sema/SemaOpenACC.cpp | 47 ++
5 files changed, 143 insertions(+), 9 deletions(-)
create mode 100644 clang/lib/Sema/SemaOpenACC.cpp
diff --git a/clang/include/clang/Parse/Parser.h
b/clang/include/clang/Parse/Parser.h
index da18cf88edcc92..69b9e837fe8bef 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3572,7 +3572,21 @@ class Parser : public CodeCompletionHandler {
StmtResult ParseOpenACCDirectiveStmt();
private:
- void ParseOpenACCDirective();
+ /// A struct to hold the information that got parsed by
ParseOpenACCDirective,
+ /// so that the callers of it can use that to construct the appropriate AST
+ /// nodes.
+ struct OpenACCDirectiveParseInfo {
+OpenACCDirectiveKind DirKind;
+SourceLocation StartLoc;
+SourceLocation EndLoc;
+// TODO OpenACC: Add Clause list here once we have a type for that.
+// TODO OpenACC: As we implement support for the Atomic, Routine, Cache,
and
+// Wait constructs, we likely want to put that information in here as well.
+ };
+
+ /// Parses the OpenACC directive (the entire pragma) including the clause
+ /// list, but does not produce the main AST node.
+ OpenACCDirectiveParseInfo ParseOpenACCDirective();
/// Helper that parses an ID Expression based on the language options.
ExprResult ParseOpenACCIDExpression();
/// Parses the variable list for the `cache` construct.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ed933f27f8df6b..ec4c451c584612 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -41,6 +41,7 @@
#include "clang/Basic/DarwinSDKInfo.h"
#include "clang/Basic/ExpressionTraits.h"
#include "clang/Basic/Module.h"
+#include "clang/Basic/OpenACCKinds.h"
#include "clang/Basic/OpenCLOptions.h"
#include "clang/Basic/OpenMPKinds.h"
#include "clang/Basic/PragmaKinds.h"
@@ -12704,6 +12705,46 @@ class Sema final {
OMPClause *ActOnOpenMPXBareClause(SourceLocation StartLoc,
SourceLocation EndLoc);
+
//======//
+ // OpenACC directives and clauses.
+
+ /// Called after parsing an OpenACC Clause so that it can be checked.
+ bool ActOnOpenACCClause(OpenACCClauseKind ClauseKind,
+ SourceLocation StartLoc);
+
+ /// Called after the construct has been parsed, but clauses haven't been
+ /// parsed. This allows us to diagnose not-implemented, as well as set up
any
+ /// state required for parsing the clauses.
+ void ActOnOpenACCConstruct(OpenACCDirectiveKind K, SourceLocation StartLoc);
+
+ /// Called after the directive, including its clauses, have been parsed and
+ /// parsing has consumed the 'annot_pragma_openacc_end' token. This DOES
+ /// happen before any associated declarations or statements have been parsed.
+ /// This function is only called when we are parsing a 'statement' context.
+ bool ActOnStartOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc);
+
+ /// Called after the directive, including its clauses, have been parsed and
+ /// parsing has consumed the 'annot_pragma_openacc_end' token. This DOES
+ /// happen before any associated declarations or statements have been parsed.
+ /// This function is only called when we are parsing a 'Decl' context.
+ bool ActOnStartOpenACCDeclDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc);
+ /// Called when we encounter an associated statement for our construct, this
+ /// should check legality of the statement as it appertains to this
Construct.
+ StmtResult ActOnOpenACCAssociatedStmt(OpenACCDirectiveKind K,
+StmtResult AssocStmt);
+
+ /// Called after the directive has been completely parsed, including the
+ /// declaration group or associated statement.
+ StmtResult ActOnEndOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc,
+
[clang] [OpenACC] Implement beginning parts of the 'parallel' Sema impl (PR #81659)
@@ -0,0 +1,132 @@
+//===--- SemaOpenACC.cpp - 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: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+/// \file
+/// This file implements semantic analysis for OpenACC constructs and
+/// clauses.
+///
+//===--===//
+
+#include "clang/Basic/DiagnosticSema.h"
+#include "clang/Basic/OpenACCKinds.h"
+#include "clang/Sema/Sema.h"
+
+using namespace clang;
+
+namespace {
+bool DiagnoseConstructAppertainment(Sema , OpenACCDirectiveKind K,
+SourceLocation StartLoc, bool IsStmt) {
+ switch (K) {
+ default:
+ case OpenACCDirectiveKind::Invalid:
+// Nothing to do here, both invalid and unimplemented don't really need to
+// do anything.
+break;
+ case OpenACCDirectiveKind::Parallel:
+if (!IsStmt)
+ return S.Diag(StartLoc, diag::err_acc_construct_appertainment) << K;
+break;
+ }
+ return false;
+}
+} // namespace
+
+bool Sema::ActOnOpenACCClause(OpenACCClauseKind ClauseKind,
+ SourceLocation StartLoc) {
+ // TODO OpenACC: this will probably want to take the Directive Kind as well
to
+ // help with legalization.
+ if (ClauseKind == OpenACCClauseKind::Invalid)
+return false;
+ // For now just diagnose that it is unsupported and leave the parsing to do
+ // whatever it can do. This function will eventually need to start returning
+ // some sort of Clause AST type, but for now just return true/false based on
+ // success.
+ return Diag(StartLoc, diag::warn_acc_clause_unimplemented) << ClauseKind;
+}
+
+void Sema::ActOnOpenACCConstruct(OpenACCDirectiveKind K,
+ SourceLocation StartLoc) {
+ switch (K) {
+ case OpenACCDirectiveKind::Invalid:
+// Nothing to do here, an invalid kind has nothing we can check here. We
+// want to continue parsing clauses as far as we can, so we will just
+// ensure that we can still work and don't check any construct-specific
+// rules anywhere.
+break;
+ case OpenACCDirectiveKind::Parallel:
+// Nothing to do here, there is no real legalization that needs to happen
+// here as these constructs do not take any arguments.
+break;
+ default:
+Diag(StartLoc, diag::warn_acc_construct_unimplemented) << K;
+break;
+ }
+}
+
+void Sema::ActOnStartOpenACCDeclDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc,
+ SourceLocation EndLoc) {
+ // TODO OpenACC: This should likely return something with the modified
+ // declaration. At the moment, only handle appertainment.
+ DiagnoseConstructAppertainment(*this, K, StartLoc, /*IsStmt=*/false);
+}
+
+void Sema::ActOnEndOpenACCDeclDirective() {
+ // TODO OpenACC: Should diagnose anything having to do with the associated
+ // statement, or any clause diagnostics that can only be done at the 'end' of
+ // the directive. We should also close any 'block' marking now that the decl
+ // parsing is complete.
+}
erichkeane wrote:
Ah, I think I see what you mean. I'll submit a patch that adds just the sema
functions and calls them properly, then can add the "change the warnings" as a
separate patch, then the "implement parallel" as a patch.
https://github.com/llvm/llvm-project/pull/81659
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement beginning parts of the 'parallel' Sema impl (PR #81659)
@@ -0,0 +1,132 @@
+//===--- SemaOpenACC.cpp - 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: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+/// \file
+/// This file implements semantic analysis for OpenACC constructs and
+/// clauses.
+///
+//===--===//
+
+#include "clang/Basic/DiagnosticSema.h"
+#include "clang/Basic/OpenACCKinds.h"
+#include "clang/Sema/Sema.h"
+
+using namespace clang;
+
+namespace {
+bool DiagnoseConstructAppertainment(Sema , OpenACCDirectiveKind K,
+SourceLocation StartLoc, bool IsStmt) {
+ switch (K) {
+ default:
+ case OpenACCDirectiveKind::Invalid:
+// Nothing to do here, both invalid and unimplemented don't really need to
+// do anything.
+break;
+ case OpenACCDirectiveKind::Parallel:
+if (!IsStmt)
+ return S.Diag(StartLoc, diag::err_acc_construct_appertainment) << K;
+break;
+ }
+ return false;
+}
+} // namespace
+
+bool Sema::ActOnOpenACCClause(OpenACCClauseKind ClauseKind,
+ SourceLocation StartLoc) {
+ // TODO OpenACC: this will probably want to take the Directive Kind as well
to
+ // help with legalization.
+ if (ClauseKind == OpenACCClauseKind::Invalid)
+return false;
+ // For now just diagnose that it is unsupported and leave the parsing to do
+ // whatever it can do. This function will eventually need to start returning
+ // some sort of Clause AST type, but for now just return true/false based on
+ // success.
+ return Diag(StartLoc, diag::warn_acc_clause_unimplemented) << ClauseKind;
+}
+
+void Sema::ActOnOpenACCConstruct(OpenACCDirectiveKind K,
+ SourceLocation StartLoc) {
+ switch (K) {
+ case OpenACCDirectiveKind::Invalid:
+// Nothing to do here, an invalid kind has nothing we can check here. We
+// want to continue parsing clauses as far as we can, so we will just
+// ensure that we can still work and don't check any construct-specific
+// rules anywhere.
+break;
+ case OpenACCDirectiveKind::Parallel:
+// Nothing to do here, there is no real legalization that needs to happen
+// here as these constructs do not take any arguments.
+break;
+ default:
+Diag(StartLoc, diag::warn_acc_construct_unimplemented) << K;
+break;
+ }
+}
+
+void Sema::ActOnStartOpenACCDeclDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc,
+ SourceLocation EndLoc) {
+ // TODO OpenACC: This should likely return something with the modified
+ // declaration. At the moment, only handle appertainment.
+ DiagnoseConstructAppertainment(*this, K, StartLoc, /*IsStmt=*/false);
+}
+
+void Sema::ActOnEndOpenACCDeclDirective() {
+ // TODO OpenACC: Should diagnose anything having to do with the associated
+ // statement, or any clause diagnostics that can only be done at the 'end' of
+ // the directive. We should also close any 'block' marking now that the decl
+ // parsing is complete.
+}
erichkeane wrote:
In this case, it ends up being a 'start' function without an 'end' if we do
that. That said, your suggestion above was to effectively no longer do the
'start' and 'end' by delaying construction until after parsing, so I might be
able to combine the 'ActOnStart' and 'ActOnEnd' into 1 anyway.
https://github.com/llvm/llvm-project/pull/81659
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement beginning parts of the 'parallel' Sema impl (PR #81659)
@@ -13662,3 +13663,8 @@ StringRef ASTContext::getCUIDHash() const {
CUIDHash = llvm::utohexstr(llvm::MD5Hash(LangOpts.CUID), /*LowerCase=*/true);
return CUIDHash;
}
+
+void ASTContext::setOpenACCStructuredBlock(OpenACCComputeConstruct *C,
+ Stmt *S) {
+ C->setStructuredBlock(S);
+}
erichkeane wrote:
So my concern with that, is it requires delaying the construction of the
Compute Construct until after we've parsed the Structured Block, which I
thought it would be useful to be able to refer to when enforcing other rules.
I'll work through to do it this patch, but we might have ourselves needing to
bring this back in the future.
https://github.com/llvm/llvm-project/pull/81659
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement beginning parts of the 'parallel' Sema impl (PR #81659)
@@ -745,9 +745,14 @@ bool Parser::ParseOpenACCClause(OpenACCDirectiveKind
DirKind) {
<< getCurToken().getIdentifierInfo();
// Consume the clause name.
- ConsumeToken();
+ SourceLocation ClauseLoc = ConsumeToken();
+
+ bool ParamsResult = ParseOpenACCClauseParams(DirKind, Kind);
- return ParseOpenACCClauseParams(DirKind, Kind);
+ // TODO OpenACC: this whole function should return a 'clause' type optional
+ // instead of bool, so we likely want to return the clause here.
+ getActions().ActOnOpenACCClause(Kind, ClauseLoc);
erichkeane wrote:
Sure, I can do that.
https://github.com/llvm/llvm-project/pull/81659
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][OpenACC] Fix copy-paste name in ParsingOpenACCDirectiveRAII (PR #81796)
https://github.com/erichkeane closed https://github.com/llvm/llvm-project/pull/81796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][OpenACC] Fix copy-paste name in ParsingOpenACCDirectiveRAII (PR #81796)
https://github.com/erichkeane approved this pull request. Ooof, thank you so much! https://github.com/llvm/llvm-project/pull/81796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Unify interface for accessing template arguments as written for class/variable template specializations (PR #81642)
@@ -6390,6 +6394,7 @@ TEST(HasTemplateArgumentLoc,
BindsToSpecializationWithDoubleArgument) {
0, hasTypeLoc(loc(asString("double")));
}
+#if 0
erichkeane wrote:
Same question here :)
https://github.com/llvm/llvm-project/pull/81642
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Unify interface for accessing template arguments as written for class/variable template specializations (PR #81642)
@@ -2545,10 +2545,12 @@ ASTDeclReader::VisitClassTemplateSpecializationDeclImpl(
}
// Explicit info.
- if (TypeSourceInfo *TyInfo = readTypeSourceInfo()) {
-auto *ExplicitInfo =
-new (C) ClassTemplateSpecializationDecl::ExplicitSpecializationInfo;
-ExplicitInfo->TypeAsWritten = TyInfo;
+ if (Record.readBool()) {
+// FIXME: We don't need to allocate this if ExternLoc and
TemplateKeywordLoc
erichkeane wrote:
This serialization is not expected to be stable. Could we just change the
'serialization' order here to be
`ExternLoc/TemplateKeywordLoc/TemplateArgsList` instead? That way we could
test this 'fixme' that way (in both places!).
Also likely value to extract this whole section into its own function so the
two can use the 'same' implementation here.
https://github.com/llvm/llvm-project/pull/81642
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Unify interface for accessing template arguments as written for class/variable template specializations (PR #81642)
@@ -6316,6 +6310,15 @@ TEST(HasAnyTemplateArgumentLoc,
BindsToSpecializationWithDoubleArgument) {
hasTypeLoc(loc(asString("double")));
}
+TEST(HasAnyTemplateArgumentLoc, BindsToExplicitSpecializationWithIntArgument) {
+ EXPECT_TRUE(
+ matches("template class A {}; template<> class A {};",
+ classTemplateSpecializationDecl(
+ hasName("A"), hasAnyTemplateArgument(templateArgument(
+refersToType(asString("int")));
+}
+
+#if 0
erichkeane wrote:
Still a WIP, or are these from local testing?
https://github.com/llvm/llvm-project/pull/81642
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Unify interface for accessing template arguments as written for class/variable template specializations (PR #81642)
@@ -285,30 +285,23 @@ template<> class SpecializationDecl; // CHECK: [[@LINE-1]]:7 | class(Gen,TS)/C++ | SpecializationDecl | c:@S@SpecializationDecl>#I | | Decl,RelSpecialization | rel: 1 // CHECK-NEXT: RelSpecialization | SpecializationDecl | c:@ST>1#T@SpecializationDecl -// CHECK: [[@LINE-3]]:7 | class(Gen,TS)/C++ | SpecializationDecl | c:@S@SpecializationDecl>#I | | Ref | rel: 0 erichkeane wrote: Can you explain what is going on here that these lines are being lost? https://github.com/llvm/llvm-project/pull/81642 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Unify interface for accessing template arguments as written for class/variable template specializations (PR #81642)
@@ -279,7 +279,7 @@ namespace dr727 { // dr727: partial
// cxx98-11-error@-1 {{variable templates are a C++14 extension}}
// cxx98-14-error@-2 {{inline variables are a C++17 extension}}
template<> static inline int v2; // #dr727-v2-T
-// cxx98-14-error@-1 {{inline variables are a C++17 extension}}
+// cxx98-14-error@-1 {{inline variables are a C++17 extension}}
erichkeane wrote:
unrelated change?
https://github.com/llvm/llvm-project/pull/81642
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Unify interface for accessing template arguments as written for class/variable template specializations (PR #81642)
@@ -222,7 +220,7 @@ int binTempl;
template
float binTempl = 1;
-// CHECK: VarTemplatePartialSpecializationDecl 0x{{[^ ]*}}
col:7 binTempl 'float' cinit
erichkeane wrote:
Why are we losing 'const-int' (Is that what cinit means?) here? Is that
perhaps important?
https://github.com/llvm/llvm-project/pull/81642
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Unify interface for accessing template arguments as written for class/variable template specializations (PR #81642)
@@ -1985,44 +1986,45 @@ class ClassTemplateSpecializationDecl
SpecializedTemplate = TemplDecl;
}
- /// Sets the type of this specialization as it was written by
- /// the user. This will be a class template specialization type.
- void setTypeAsWritten(TypeSourceInfo *T) {
-if (!ExplicitInfo)
- ExplicitInfo = new (getASTContext()) ExplicitSpecializationInfo;
-ExplicitInfo->TypeAsWritten = T;
- }
-
- /// Gets the type of this specialization as it was written by
- /// the user, if it was so written.
- TypeSourceInfo *getTypeAsWritten() const {
-return ExplicitInfo ? ExplicitInfo->TypeAsWritten : nullptr;
+ const ASTTemplateArgumentListInfo *getTemplateArgsAsWritten() const {
+if (auto *Info = ExplicitInfo.dyn_cast())
+ return Info->TemplateArgsAsWritten;
+return ExplicitInfo.get();
}
/// Gets the location of the extern keyword, if present.
SourceLocation getExternLoc() const {
-return ExplicitInfo ? ExplicitInfo->ExternLoc : SourceLocation();
+if (auto *Info = ExplicitInfo.dyn_cast())
+ return Info->ExternLoc;
+return SourceLocation();
}
- /// Sets the location of the extern keyword.
- void setExternLoc(SourceLocation Loc) {
-if (!ExplicitInfo)
- ExplicitInfo = new (getASTContext()) ExplicitSpecializationInfo;
-ExplicitInfo->ExternLoc = Loc;
+ /// Gets the location of the template keyword, if present.
+ SourceLocation getTemplateKeywordLoc() const {
+if (auto *Info = ExplicitInfo.dyn_cast())
+ return Info->TemplateKeywordLoc;
+return SourceLocation();
}
- /// Sets the location of the template keyword.
- void setTemplateKeywordLoc(SourceLocation Loc) {
-if (!ExplicitInfo)
- ExplicitInfo = new (getASTContext()) ExplicitSpecializationInfo;
-ExplicitInfo->TemplateKeywordLoc = Loc;
+ void
+ setTemplateArgsAsWritten(const ASTTemplateArgumentListInfo *ArgsWritten) {
+if (auto *Info = ExplicitInfo.dyn_cast())
+ Info->TemplateArgsAsWritten = ArgsWritten;
+else
+ ExplicitInfo = ArgsWritten;
}
- /// Gets the location of the template keyword, if present.
- SourceLocation getTemplateKeywordLoc() const {
-return ExplicitInfo ? ExplicitInfo->TemplateKeywordLoc : SourceLocation();
+ void setTemplateArgsAsWritten(const TemplateArgumentListInfo ) {
erichkeane wrote:
Can we 'pair' up the getters/setters? It makes it easier to read/review if
they are 'next' to eachother rather than alphabetized (or whatever order you're
going for).
https://github.com/llvm/llvm-project/pull/81642
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Unify interface for accessing template arguments as written for class/variable template specializations (PR #81642)
@@ -2596,23 +2582,24 @@ class VarTemplateSpecializationDecl : public VarDecl,
llvm::PointerUnion
SpecializedTemplate;
- /// Further info for explicit template specialization/instantiation.
- struct ExplicitSpecializationInfo {
-/// The type-as-written.
-TypeSourceInfo *TypeAsWritten = nullptr;
+ struct ExplicitInstantiationInfo {
erichkeane wrote:
This is the 2nd struct here that has the same structure. Should we instead
extract this to a 'details' NS and reuse it for all of them?
https://github.com/llvm/llvm-project/pull/81642
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Unify interface for accessing template arguments as written for class/variable template specializations (PR #81642)
erichkeane wrote: >The "most breaking" change is to AST Matchers, insofar that hasTypeLoc will no >longer match class template specializations (since they no longer store the >type as written). I wasn't entirely sure what to replace the tests for this >behavior with, so I just commented them out... guidance/feedback would be >appreciated on this. This part has implications that I'm not sure about, and likely needs to A: be in the 'breaking changes' doc, and B: Have Aaron comment on it. https://github.com/llvm/llvm-project/pull/81642 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -2705,6 +2705,30 @@ An error will be given if:
}];
}
+def AMDGPUNumWorkGroupsDocs : Documentation {
+ let Category = DocCatAMDGPUAttributes;
+ let Content = [{
+The number of work groups specifies the number of work groups when the kernel
+is dispatched.
+
+Clang supports the
+``__attribute__((amdgpu_num_work_groups(, , )))`` attribute for the
+AMDGPU target. This attribute may be attached to a kernel function definition
erichkeane wrote:
That would be preferential, yes
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Avoid printing overly large integer. (PR #75902)
=?utf-8?q?“Nhat?= , =?utf-8?q?“Nhat?= , =?utf-8?q?“Nhat?= , =?utf-8?q?“Nhat?= , =?utf-8?q?“Nhat?= ,Nhat Nguyen Message-ID: In-Reply-To: erichkeane wrote: > > Looks like all the tests disappeared? Only thing I see is the code change. > > Also, no release note is currently present. > > Hi I am still slowly working on it. I am not familiar with the release note. > Can you provide me some pointers so I can follow accordingly? Thanks a lot. See the clang/docs/ directory, particularly https://github.com/llvm/llvm-project/blob/main/clang/docs/ReleaseNotes.rst . You can put an entry into whichever section makes the most sense (probably improvements to clang's diagnostics), just write a sentence or two explaining the improvement. https://github.com/llvm/llvm-project/pull/75902 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] fix crash in codegen stage when an lambda expression declared in an unevaluated context (PR #80802)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/80802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [OpenACC] Implement AST for OpenACC Compute Constructs (PR #81188)
https://github.com/erichkeane closed https://github.com/llvm/llvm-project/pull/81188 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Diagnose friend declarations with enum elaborated-type-specifier in all language modes (PR #80171)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/80171 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Diagnose declarative nested-name-specifiers naming alias templates (PR #80842)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/80842 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Diagnose misuse of the cleanup attribute (PR #80040)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/80040 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Refactor `Sema::TemplateDeductionResult` (PR #81398)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/81398 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Refactor `Sema::TemplateDeductionResult` (PR #81398)
@@ -33,6 +33,7 @@ namespace clang {
class Decl;
struct DeducedPack;
class Sema;
+enum class TemplateDeductionResult;
erichkeane wrote:
Ok then, I'm ok sticking to our existing practice if this is a policy that was
already made.
https://github.com/llvm/llvm-project/pull/81398
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Refactor `Sema::TemplateDeductionResult` (PR #81398)
@@ -11445,11 +11452,16 @@ static void DiagnoseBadDeduction(Sema , NamedDecl
*Found, Decl *Templated,
(ParamD = Param.dyn_cast()) ||
(ParamD = Param.dyn_cast()) ||
(ParamD = Param.dyn_cast());
- switch (DeductionFailure.Result) {
- case Sema::TDK_Success:
-llvm_unreachable("TDK_success while diagnosing bad deduction");
+ switch (DeductionFailure.getResult()) {
+ case TemplateDeductionResult::Success:
+llvm_unreachable(
+"TemplateDeductionResult::Success while diagnosing bad deduction");
+ case TemplateDeductionResult::Invalid:
+ case TemplateDeductionResult::NonDependentConversionFailure:
+ case TemplateDeductionResult::AlreadyDiagnosed:
+return;
erichkeane wrote:
Cool. It'll be interesting to see if we ever end up here, and the assert
message is good enough we can figure out what to do pretty easily based on bug
reports. Thanks!
https://github.com/llvm/llvm-project/pull/81398
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] fix crash in codegen stage when an lambda expression declared in an unevaluated context (PR #80802)
@@ -1614,7 +1614,19 @@ bool TemplateInstantiator::AlreadyTransformed(QualType
T) {
if (T.isNull())
return true;
- if (T->isInstantiationDependentType() || T->isVariablyModifiedType())
+ bool DependentLambdaType = false;
+ QualType DesugaredType = T.getDesugaredType(SemaRef.getASTContext());
erichkeane wrote:
Is this step necessary? `getAsCXXRecordDecl` should at least canonicalize.
https://github.com/llvm/llvm-project/pull/80802
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] fix crash in codegen stage when an lambda expression declared in an unevaluated context (PR #80802)
@@ -1614,7 +1614,19 @@ bool TemplateInstantiator::AlreadyTransformed(QualType
T) {
if (T.isNull())
return true;
- if (T->isInstantiationDependentType() || T->isVariablyModifiedType())
+ bool DependentLambdaType = false;
+ QualType DesugaredType = T.getDesugaredType(SemaRef.getASTContext());
+ CXXRecordDecl *RD = DesugaredType->getAsCXXRecordDecl();
+ if (RD && RD->isLambda()) {
erichkeane wrote:
`if (auto *RD = T->getAsCXXRecordDecl(); RD && RD->isLambda())`
??
https://github.com/llvm/llvm-project/pull/80802
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] fix crash in codegen stage when an lambda expression declared in an unevaluated context (PR #80802)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/80802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] fix crash in codegen stage when an lambda expression declared in an unevaluated context (PR #80802)
https://github.com/erichkeane commented: A few suggestions, but I REALLY would like @cor3ntin to take a final pass, it mostly LGTM. https://github.com/llvm/llvm-project/pull/80802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Don't consider top-level cv-qualifiers in template partial orderings (PR #81449)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/81449 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -356,6 +356,19 @@ void AMDGPUTargetCodeGenInfo::setFunctionDeclAttributes(
if (NumVGPR != 0)
F->addFnAttr("amdgpu-num-vgpr", llvm::utostr(NumVGPR));
}
+
+ if (const auto *Attr = FD->getAttr()) {
+uint32_t X = Attr->getNumWorkGroupsX();
+uint32_t Y = Attr->getNumWorkGroupsY();
+uint32_t Z = Attr->getNumWorkGroupsZ();
+
+if (X != 0 && Y != 0 && Z != 0) {
erichkeane wrote:
Yeah, this seems like a 'bad' user interface. If '0' means 'nothing', we
should probably reject that in Sema.
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -2705,6 +2705,30 @@ An error will be given if:
}];
}
+def AMDGPUNumWorkGroupsDocs : Documentation {
+ let Category = DocCatAMDGPUAttributes;
+ let Content = [{
+The number of work groups specifies the number of work groups when the kernel
+is dispatched.
+
+Clang supports the
+``__attribute__((amdgpu_num_work_groups(, , )))`` attribute for the
+AMDGPU target. This attribute may be attached to a kernel function definition
erichkeane wrote:
'kernel function definition' probably needs more elaboration as well, we now
have ~3 different 'kinds' of kernel function definitions.
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AMDGPU] Adding the amdgpu-num-work-groups function attribute (PR #79035)
@@ -8069,6 +8069,25 @@ static void handleAMDGPUNumVGPRAttr(Sema , Decl *D,
const ParsedAttr ) {
D->addAttr(::new (S.Context) AMDGPUNumVGPRAttr(S.Context, AL, NumVGPR));
}
+static void handleAMDGPUNumWorkGroupsAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+ uint32_t NumWGX = 0;
+ uint32_t NumWGY = 0;
+ uint32_t NumWGZ = 0;
+ Expr *NumWGXExpr = AL.getArgAsExpr(0);
+ Expr *NumWGYExpr = AL.getArgAsExpr(1);
+ Expr *NumWGZExpr = AL.getArgAsExpr(2);
+ if (!checkUInt32Argument(S, AL, NumWGXExpr, NumWGX))
+return;
+ if (!checkUInt32Argument(S, AL, NumWGYExpr, NumWGY))
+return;
+ if (!checkUInt32Argument(S, AL, NumWGZExpr, NumWGZ))
+return;
+
+ D->addAttr(::new (S.Context) AMDGPUNumWorkGroupsAttr(S.Context, AL, NumWGX,
erichkeane wrote:
Based on the above, if any of these are zero, this attribute has no effect. We
should diagnose based on the value of X, Y, and Z, then only create it in the
AST if it has an effect.
https://github.com/llvm/llvm-project/pull/79035
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
