[clang] [OpenMP] Add diagnostic for 'factor' width mismatch in 'unroll partial' (PR #139986)
ALBIN BABU VARGHESE via cfe-commits
cfe-commits at lists.llvm.org
Fri May 23 00:28:20 PDT 2025
https://github.com/albus-droid updated https://github.com/llvm/llvm-project/pull/139986
>From 3cc8334092853442f85c5a17a3bd31e373f30da8 Mon Sep 17 00:00:00 2001
From: albus-droid <albinbabuvarghese20 at gmail.com>
Date: Wed, 14 May 2025 15:49:11 -0400
Subject: [PATCH 1/8] Added two conditions to check for width of the factor
variable
---
clang/lib/Sema/SemaOpenMP.cpp | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index f16f841d62edd..334fd1ef4370d 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -14924,8 +14924,18 @@ StmtResult SemaOpenMP::ActOnOpenMPUnrollDirective(ArrayRef<OMPClause *> Clauses,
SourceLocation FactorLoc;
if (Expr *FactorVal = PartialClause->getFactor();
FactorVal && !FactorVal->containsErrors()) {
+ if (!VerifyPositiveIntegerConstantInClause(FactorVal,OMPC_partial,/*StrictlyPositive=*/true,/*SuppressExprDiags=*/false).isUsable()) {
+ return StmtError();
+ }
+ // Checking if Itertor Variable Type can hold the Factor Width
+ if (FactorVal->EvaluateKnownConstInt(Context).getBitWidth() > Context.getTypeSize(IVTy)) {
+ Diag(FactorVal->getExprLoc(), diag::err_omp_hint_clause_no_name);
+ return StmtError();
+ }
+
Factor = FactorVal->getIntegerConstantExpr(Context)->getZExtValue();
FactorLoc = FactorVal->getExprLoc();
+
} else {
// TODO: Use a better profitability model.
Factor = 2;
>From 0f77ad36a4256e0d5d24d133fa8d0af4a4348ce8 Mon Sep 17 00:00:00 2001
From: albus-droid <albinbabuvarghese20 at gmail.com>
Date: Wed, 14 May 2025 16:33:34 -0400
Subject: [PATCH 2/8] Added diag error message and also formatted using
clang-format
---
clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 ++
clang/lib/Sema/SemaOpenMP.cpp | 16 +++++++++++-----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6e940a318b61d..3cc757f72ed27 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11540,6 +11540,8 @@ def err_omp_negative_expression_in_clause : Error<
"argument to '%0' clause must be a %select{non-negative|strictly positive}1 integer value">;
def err_omp_large_expression_in_clause : Error<
"argument to '%0' clause requires a value that can be represented by a 64-bit">;
+def err_omp_unroll_factor_width_mismatch : Error<
+ "unroll factor has width %0 but the iteration variable %1 is only %2 bits wide">;
def err_omp_not_integral : Error<
"expression must have integral or unscoped enumeration "
"type, not %0">;
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 334fd1ef4370d..35d7bfba1bb9a 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -14924,13 +14924,19 @@ StmtResult SemaOpenMP::ActOnOpenMPUnrollDirective(ArrayRef<OMPClause *> Clauses,
SourceLocation FactorLoc;
if (Expr *FactorVal = PartialClause->getFactor();
FactorVal && !FactorVal->containsErrors()) {
- if (!VerifyPositiveIntegerConstantInClause(FactorVal,OMPC_partial,/*StrictlyPositive=*/true,/*SuppressExprDiags=*/false).isUsable()) {
- return StmtError();
+ if (!VerifyPositiveIntegerConstantInClause(FactorVal, OMPC_partial,
+ /*StrictlyPositive=*/true,
+ /*SuppressExprDiags=*/false)
+ .isUsable()) {
+ return StmtError();
}
// Checking if Itertor Variable Type can hold the Factor Width
- if (FactorVal->EvaluateKnownConstInt(Context).getBitWidth() > Context.getTypeSize(IVTy)) {
- Diag(FactorVal->getExprLoc(), diag::err_omp_hint_clause_no_name);
- return StmtError();
+ if (FactorVal->getIntegerConstantExpr(Context)->getBitWidth() >
+ Context.getTypeSize(IVTy)) {
+ Diag(FactorVal->getExprLoc(), diag::err_omp_unroll_factor_width_mismatch)
+ << FactorVal->getIntegerConstantExpr(Context)->getBitWidth() << IVTy
+ << Context.getTypeSize(IVTy);
+ return StmtError();
}
Factor = FactorVal->getIntegerConstantExpr(Context)->getZExtValue();
>From 3fafd6bc2af31a929e4919864b8d280e10147852 Mon Sep 17 00:00:00 2001
From: albus-droid <albinbabuvarghese20 at gmail.com>
Date: Wed, 14 May 2025 20:11:15 -0400
Subject: [PATCH 3/8] Added two test cases
---
clang/test/OpenMP/unroll_messages.cpp | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/clang/test/OpenMP/unroll_messages.cpp b/clang/test/OpenMP/unroll_messages.cpp
index 17d5ed83e162f..2903639b602a6 100644
--- a/clang/test/OpenMP/unroll_messages.cpp
+++ b/clang/test/OpenMP/unroll_messages.cpp
@@ -58,8 +58,17 @@ void func(int n) {
// expected-error at +1 {{argument to 'partial' clause must be a strictly positive integer value}}
#pragma omp unroll partial(0)
for (int i = 0; i < n; ++i) {}
-
- // expected-error at +1 {{directive '#pragma omp unroll' cannot contain more than one 'partial' clause}}
+
+ // expected-error at +1 {{unroll factor has width 64 but the iteration variable 'int' is only 32 bits wide}}
+ #pragma omp unroll partial(0xFFFFFFFFF)
+ for (int i = 0; i < 10; i++)
+
+ // expected-error at +2 {{integer literal is too large to be represented in any integer type}}
+ // expected-error at +1 {{argument to 'partial' clause must be a strictly positive integer value}}
+ #pragma omp unroll partial(0x10000000000000000)
+ for (int i = 0; i < 10; i++)
+
+ // expected-error at +1 {{directive '#pragma omp unroll' cannot contain more than one 'partial' clause}}
#pragma omp unroll partial partial
for (int i = 0; i < n; ++i) {}
>From 7a99f51ac974f10ae53ecf5b9eaaed8a84c63f67 Mon Sep 17 00:00:00 2001
From: albus-droid <albinbabuvarghese20 at gmail.com>
Date: Thu, 15 May 2025 02:39:58 -0400
Subject: [PATCH 4/8] Changed the Factor variable and Iterator Variable
---
clang/lib/Sema/SemaOpenMP.cpp | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 35d7bfba1bb9a..4d37ad2aba4a7 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -14930,12 +14930,13 @@ StmtResult SemaOpenMP::ActOnOpenMPUnrollDirective(ArrayRef<OMPClause *> Clauses,
.isUsable()) {
return StmtError();
}
- // Checking if Itertor Variable Type can hold the Factor Width
- if (FactorVal->getIntegerConstantExpr(Context)->getBitWidth() >
- Context.getTypeSize(IVTy)) {
+ // Checking if Iterator Variable Type can hold the Factor Width
+ auto FactorValWidth = FactorVal->getIntegerConstantExpr(Context)->getActiveBits();
+ auto IteratorVWidth = Context.getTypeSize(OrigVar->getType());
+ if ( FactorValWidth > IteratorVWidth ) {
Diag(FactorVal->getExprLoc(), diag::err_omp_unroll_factor_width_mismatch)
- << FactorVal->getIntegerConstantExpr(Context)->getBitWidth() << IVTy
- << Context.getTypeSize(IVTy);
+ << FactorValWidth << OrigVar->getType()
+ << IteratorVWidth;
return StmtError();
}
>From 1ad91629b8fd01109c5e4de03a8043298d9d8be5 Mon Sep 17 00:00:00 2001
From: albus-droid <albinbabuvarghese20 at gmail.com>
Date: Thu, 15 May 2025 03:51:55 -0400
Subject: [PATCH 5/8] updated and added some new test cases
---
clang/test/OpenMP/unroll_messages.cpp | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/clang/test/OpenMP/unroll_messages.cpp b/clang/test/OpenMP/unroll_messages.cpp
index 2903639b602a6..ba9ec35899dcd 100644
--- a/clang/test/OpenMP/unroll_messages.cpp
+++ b/clang/test/OpenMP/unroll_messages.cpp
@@ -59,14 +59,22 @@ void func(int n) {
#pragma omp unroll partial(0)
for (int i = 0; i < n; ++i) {}
- // expected-error at +1 {{unroll factor has width 64 but the iteration variable 'int' is only 32 bits wide}}
+ // expected-error at +1 {{unroll factor has width 36 but the iteration variable 'int' is only 32 bits wide}}
#pragma omp unroll partial(0xFFFFFFFFF)
- for (int i = 0; i < 10; i++)
+ for (int i = 0; i < 10; i++) {}
// expected-error at +2 {{integer literal is too large to be represented in any integer type}}
// expected-error at +1 {{argument to 'partial' clause must be a strictly positive integer value}}
#pragma omp unroll partial(0x10000000000000000)
- for (int i = 0; i < 10; i++)
+ for (int i = 0; i < 10; i++) {}
+
+ // expected-error at +1 {{unroll factor has width 12 but the iteration variable 'char' is only 8 bits wide}}
+ #pragma omp unroll partial(0xFFF)
+ for (char i = 0; i < 10; i++) {}
+
+ // expected-error at +1 {{unroll factor has width 20 but the iteration variable 'short' is only 16 bits wide}}
+ #pragma omp unroll partial(0xFFFFF)
+ for (short i = 0; i < 10; i++) {}
// expected-error at +1 {{directive '#pragma omp unroll' cannot contain more than one 'partial' clause}}
#pragma omp unroll partial partial
>From 94b76de4456f1443ccd15a38bfa3e6616edbcd92 Mon Sep 17 00:00:00 2001
From: albus-droid <albinbabuvarghese20 at gmail.com>
Date: Thu, 15 May 2025 15:39:15 -0400
Subject: [PATCH 6/8] Added the requested changes
---
clang/lib/Sema/SemaOpenMP.cpp | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 4d37ad2aba4a7..b13a744591a0f 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -14927,12 +14927,11 @@ StmtResult SemaOpenMP::ActOnOpenMPUnrollDirective(ArrayRef<OMPClause *> Clauses,
if (!VerifyPositiveIntegerConstantInClause(FactorVal, OMPC_partial,
/*StrictlyPositive=*/true,
/*SuppressExprDiags=*/false)
- .isUsable()) {
+ .isUsable())
return StmtError();
- }
// Checking if Iterator Variable Type can hold the Factor Width
- auto FactorValWidth = FactorVal->getIntegerConstantExpr(Context)->getActiveBits();
- auto IteratorVWidth = Context.getTypeSize(OrigVar->getType());
+ unsigned FactorValWidth = FactorVal->getIntegerConstantExpr(Context)->getActiveBits();
+ unsigned IteratorVWidth = Context.getTypeSize(OrigVar->getType());
if ( FactorValWidth > IteratorVWidth ) {
Diag(FactorVal->getExprLoc(), diag::err_omp_unroll_factor_width_mismatch)
<< FactorValWidth << OrigVar->getType()
@@ -14942,7 +14941,6 @@ StmtResult SemaOpenMP::ActOnOpenMPUnrollDirective(ArrayRef<OMPClause *> Clauses,
Factor = FactorVal->getIntegerConstantExpr(Context)->getZExtValue();
FactorLoc = FactorVal->getExprLoc();
-
} else {
// TODO: Use a better profitability model.
Factor = 2;
>From 935bedb0452c5340a34970f709ec2dbaa40ef993 Mon Sep 17 00:00:00 2001
From: albus-droid <albinbabuvarghese20 at gmail.com>
Date: Thu, 15 May 2025 16:07:27 -0400
Subject: [PATCH 7/8] Added the requested formatting changes
---
clang/lib/Sema/SemaOpenMP.cpp | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index b13a744591a0f..cc4d98faa2c56 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -14929,13 +14929,13 @@ StmtResult SemaOpenMP::ActOnOpenMPUnrollDirective(ArrayRef<OMPClause *> Clauses,
/*SuppressExprDiags=*/false)
.isUsable())
return StmtError();
- // Checking if Iterator Variable Type can hold the Factor Width
- unsigned FactorValWidth = FactorVal->getIntegerConstantExpr(Context)->getActiveBits();
- unsigned IteratorVWidth = Context.getTypeSize(OrigVar->getType());
- if ( FactorValWidth > IteratorVWidth ) {
+ // Check that the iterator variable’s type can hold the factor’s bit-width
+ unsigned factorValWidth =
+ FactorVal->getIntegerConstantExpr(Context)->getActiveBits();
+ unsigned iteratorVWidth = Context.getTypeSize(OrigVar->getType());
+ if (factorValWidth > iteratorVWidth) {
Diag(FactorVal->getExprLoc(), diag::err_omp_unroll_factor_width_mismatch)
- << FactorValWidth << OrigVar->getType()
- << IteratorVWidth;
+ << factorValWidth << OrigVar->getType() << iteratorVWidth;
return StmtError();
}
>From cdfe95f9dfc07aecbafae0c536f74c1636ba1404 Mon Sep 17 00:00:00 2001
From: ALBIN BABU VARGHESE <albinbabuvarghese20 at gmail.com>
Date: Fri, 23 May 2025 03:28:08 -0400
Subject: [PATCH 8/8] Update SemaOpenMP.cpp
Co-authored-by: Aaron Ballman <aaron at aaronballman.com>
---
clang/lib/Sema/SemaOpenMP.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index cc4d98faa2c56..078e8dcdfa316 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -14930,12 +14930,12 @@ StmtResult SemaOpenMP::ActOnOpenMPUnrollDirective(ArrayRef<OMPClause *> Clauses,
.isUsable())
return StmtError();
// Check that the iterator variable’s type can hold the factor’s bit-width
- unsigned factorValWidth =
+ unsigned FactorValWidth =
FactorVal->getIntegerConstantExpr(Context)->getActiveBits();
- unsigned iteratorVWidth = Context.getTypeSize(OrigVar->getType());
- if (factorValWidth > iteratorVWidth) {
+ unsigned IteratorVWidth = Context.getTypeSize(OrigVar->getType());
+ if (FactorValWidth > IteratorVWidth) {
Diag(FactorVal->getExprLoc(), diag::err_omp_unroll_factor_width_mismatch)
- << factorValWidth << OrigVar->getType() << iteratorVWidth;
+ << FactorValWidth << OrigVar->getType() << IteratorVWidth;
return StmtError();
}
More information about the cfe-commits
mailing list