[clang] [OpenMP] Add assertion for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

https://github.com/albus-droid created 
https://github.com/llvm/llvm-project/pull/139986

This fixes #139268





  



Rate limit · GitHub


  body {
background-color: #f6f8fa;
color: #24292e;
font-family: -apple-system,BlinkMacSystemFont,Segoe 
UI,Helvetica,Arial,sans-serif,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol;
font-size: 14px;
line-height: 1.5;
margin: 0;
  }

  .container { margin: 50px auto; max-width: 600px; text-align: center; 
padding: 0 24px; }

  a { color: #0366d6; text-decoration: none; }
  a:hover { text-decoration: underline; }

  h1 { line-height: 60px; font-size: 48px; font-weight: 300; margin: 0px; 
text-shadow: 0 1px 0 #fff; }
  p { color: rgba(0, 0, 0, 0.5); margin: 20px 0 40px; }

  ul { list-style: none; margin: 25px 0; padding: 0; }
  li { display: table-cell; font-weight: bold; width: 1%; }

  .logo { display: inline-block; margin-top: 35px; }
  .logo-img-2x { display: none; }
  @media
  only screen and (-webkit-min-device-pixel-ratio: 2),
  only screen and (   min--moz-device-pixel-ratio: 2),
  only screen and ( -o-min-device-pixel-ratio: 2/1),
  only screen and (min-device-pixel-ratio: 2),
  only screen and (min-resolution: 192dpi),
  only screen and (min-resolution: 2dppx) {
.logo-img-1x { display: none; }
.logo-img-2x { display: inline-block; }
  }

  #suggestions {
margin-top: 35px;
color: #ccc;
  }
  #suggestions a {
color: #66;
font-weight: 200;
font-size: 14px;
margin: 0 10px;
  }


  
  



  Whoa there!
  You have exceeded a secondary rate limit.
Please wait a few minutes before you try again;
in some cases this may take up to an hour.
  
  
https://support.github.com/contact";>Contact Support —
https://githubstatus.com";>GitHub Status —
https://twitter.com/githubstatus";>@githubstatus
  

  

  

  

  

  


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Add assertion for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

@@ -58,8 +58,17 @@ void func(int n) {
   // expected-error@+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@+1 {{directive '#pragma omp unroll' cannot contain more 
than one 'partial' clause}} 
+
+  // expected-error@+1 {{unroll factor has width 64 but the iteration variable 
'int' is only 32 bits wide}}
+  #pragma omp unroll partial(0xF)
+  for (int i = 0; i < 10; i++)

albus-droid wrote:

I realized I was using the wrong loop variable: the type still shows as i32 
even after I changed it to char. I should be using OrigVar instead of IVTy. 
I’ll include that fix along with the other changes.

https://github.com/llvm/llvm-project/pull/139986
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Add assertion for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

https://github.com/albus-droid edited 
https://github.com/llvm/llvm-project/pull/139986
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Add assertion for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

https://github.com/albus-droid edited 
https://github.com/llvm/llvm-project/pull/139986
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Add assertion for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

@@ -14924,8 +14924,24 @@ StmtResult 
SemaOpenMP::ActOnOpenMPUnrollDirective(ArrayRef 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->getIntegerConstantExpr(Context)->getBitWidth() >

albus-droid wrote:

I created two local variables `FactorValWidth` and `IteratorVWidth` to use in 
both places.

https://github.com/llvm/llvm-project/pull/139986
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Add assertion for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

@@ -58,8 +58,17 @@ void func(int n) {
   // expected-error@+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@+1 {{directive '#pragma omp unroll' cannot contain more 
than one 'partial' clause}} 
+
+  // expected-error@+1 {{unroll factor has width 64 but the iteration variable 
'int' is only 32 bits wide}}
+  #pragma omp unroll partial(0xF)
+  for (int i = 0; i < 10; i++)

albus-droid wrote:

Also I have added new test to cover cases with short and char and revised the 
existing ones.

https://github.com/llvm/llvm-project/pull/139986
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Add assertion for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

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 
Date: Wed, 14 May 2025 15:49:11 -0400
Subject: [PATCH 1/5] 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 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 
Date: Wed, 14 May 2025 16:33:34 -0400
Subject: [PATCH 2/5] 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 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 
Date: Wed, 14 May 2025 20:11:15 -0400
Subject: [PATCH 3/5] 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@+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@+1 {{di

[clang] [OpenMP] Add diagnostic for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

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 
Date: Wed, 14 May 2025 15:49:11 -0400
Subject: [PATCH 1/6] 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 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 
Date: Wed, 14 May 2025 16:33:34 -0400
Subject: [PATCH 2/6] 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 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 
Date: Wed, 14 May 2025 20:11:15 -0400
Subject: [PATCH 3/6] 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@+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@+1 {{di

[clang] [OpenMP] Add assertion for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

https://github.com/albus-droid edited 
https://github.com/llvm/llvm-project/pull/139986
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Add assertion for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

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 
Date: Wed, 14 May 2025 15:49:11 -0400
Subject: [PATCH 1/4] 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 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 
Date: Wed, 14 May 2025 16:33:34 -0400
Subject: [PATCH 2/4] 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 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 
Date: Wed, 14 May 2025 20:11:15 -0400
Subject: [PATCH 3/4] 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@+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@+1 {{di

[clang] [OpenMP] Add diagnostic for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

https://github.com/albus-droid updated 
https://github.com/llvm/llvm-project/pull/139986



  



Rate limit · GitHub


  body {
background-color: #f6f8fa;
color: #24292e;
font-family: -apple-system,BlinkMacSystemFont,Segoe 
UI,Helvetica,Arial,sans-serif,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol;
font-size: 14px;
line-height: 1.5;
margin: 0;
  }

  .container { margin: 50px auto; max-width: 600px; text-align: center; 
padding: 0 24px; }

  a { color: #0366d6; text-decoration: none; }
  a:hover { text-decoration: underline; }

  h1 { line-height: 60px; font-size: 48px; font-weight: 300; margin: 0px; 
text-shadow: 0 1px 0 #fff; }
  p { color: rgba(0, 0, 0, 0.5); margin: 20px 0 40px; }

  ul { list-style: none; margin: 25px 0; padding: 0; }
  li { display: table-cell; font-weight: bold; width: 1%; }

  .logo { display: inline-block; margin-top: 35px; }
  .logo-img-2x { display: none; }
  @media
  only screen and (-webkit-min-device-pixel-ratio: 2),
  only screen and (   min--moz-device-pixel-ratio: 2),
  only screen and ( -o-min-device-pixel-ratio: 2/1),
  only screen and (min-device-pixel-ratio: 2),
  only screen and (min-resolution: 192dpi),
  only screen and (min-resolution: 2dppx) {
.logo-img-1x { display: none; }
.logo-img-2x { display: inline-block; }
  }

  #suggestions {
margin-top: 35px;
color: #ccc;
  }
  #suggestions a {
color: #66;
font-weight: 200;
font-size: 14px;
margin: 0 10px;
  }


  
  



  Whoa there!
  You have exceeded a secondary rate limit.
Please wait a few minutes before you try again;
in some cases this may take up to an hour.
  
  
https://support.github.com/contact";>Contact Support —
https://githubstatus.com";>GitHub Status —
https://twitter.com/githubstatus";>@githubstatus
  

  

  

  

  

  


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Add diagnostic for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

albus-droid wrote:

Thanks for the review. I’ve implemented all the requested changes but still 
have a few questions:

* Originally, using `IVTy` to query the iteration variable’s width always 
returned “32-bit int,” even if the loop was declared as `char` or `short`. As a 
result, errors only appeared when the factor exceeded 32 bits, and the 
diagnostic always said “int (32 bits).” However, when the loop variable was 
`long`, the code compiled successfully, so larger types were recognized. I’m 
not sure whether this was due to how I was reading the type or something else.

* In the current implementation I switched to using `OrigVar->getType()` to get 
the variable’s `QualType`. Now `getTypeSize()` correctly returns 8 bits for 
`char`, 16 for `short`, 64 for `long`, etc. That means code like

  ```cpp
  #pragma omp unroll partial(0xF)
  for (char i = 0; …) { }
  ```

  now rejects, whereas it used to compile.

* One remaining concern: the bit-width check treats all iteration variables as 
unsigned. It doesn’t account for signed types. Should I add a separate 
signed-range check, as suggested here in this 
[comment](https://github.com/llvm/llvm-project/issues/139268#issuecomment-2867068205)?

https://github.com/llvm/llvm-project/pull/139986
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Add diagnostic for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

albus-droid wrote:

Ping @AaronBallman — could you take a look at this when you have a moment? 
Thanks!

https://github.com/llvm/llvm-project/pull/139986
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [OpenMP] Add diagnostic for 'factor' width mismatch in 'unroll partial' (PR #139986)

[ALBIN BABU VARGHESE via cfe-commits]

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 
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 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 
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 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 
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@+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@+1 {{di