[clang] fix issue: [Clang][OpenMP] Implicit conversion with `pragma omp taskloop` #100536 (PR #101812)

via cfe-commits cfe-commits at lists.llvm.org
Sat Aug 3 04:17:22 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: None (HenryZ16)

<details>
<summary>Changes</summary>

This PR fixes #<!-- -->100536
I've noticed that this issue was caused by setting `VType` and `StrideVType` to a specified type:
```cpp
if (isOpenMPTaskLoopDirective(DKind)) {
    VType =
        SemaRef.Context.getIntTypeForBitwidth(/*DestWidth=*/64, /*Signed=*/0);
    StrideVType =
        SemaRef.Context.getIntTypeForBitwidth(/*DestWidth=*/64, /*Signed=*/1);
  }
```
Then performed type-checking on these types. So I avoided the corresponding type-checkings to these types.
To test whether this fix can avoid the failure under `-Werror -Wconversion`, you can simply use the code in #<!-- -->100536:
```cpp
int main(void)
{
	#pragma omp parallel
	#pragma omp single
	{
		#pragma omp taskloop
		for (int i = 0; i < 10000; i++) {
			#pragma omp task
			{}
		}

		#pragma omp taskloop
		for (long i = 0L; i < 10000L; i++) {
			#pragma omp task
			{}
		}

		#pragma omp taskloop
		for (unsigned long i = 0UL; i < 10000UL; i++) {
			#pragma omp task
			{}
		}
	}
	return 0;
}
```
It used to fail, but now it can be compiled without any errors.

---
Full diff: https://github.com/llvm/llvm-project/pull/101812.diff


3 Files Affected:

- (modified) clang/include/clang/Sema/Sema.h (+8-1) 
- (modified) clang/lib/Sema/SemaExprCXX.cpp (+4-2) 
- (modified) clang/lib/Sema/SemaOpenMP.cpp (+61-15) 


``````````diff
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 2ec6367eccea0..c62919cb13962 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8279,9 +8279,16 @@ class Sema final : public SemaBase {
     return ActOnFinishFullExpr(
         Expr, Expr ? Expr->getExprLoc() : SourceLocation(), DiscardedValue);
   }
+  ExprResult ActOnFinishFullExprNoCheckExpr(Expr *Expr, bool DiscardedValue) {
+    return ActOnFinishFullExpr(
+        Expr, Expr ? Expr->getExprLoc() : SourceLocation(), DiscardedValue,
+        /*IsConstexpr=*/false, /*IsTemplateArgument=*/false,
+        /*PerformsCheckCompletedExpr=*/false);
+  }
   ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
                                  bool DiscardedValue, bool IsConstexpr = false,
-                                 bool IsTemplateArgument = false);
+                                 bool IsTemplateArgument = false,
+                                 bool PerformsCheckCompletedExpr = true);
   StmtResult ActOnFinishFullStmt(Stmt *Stmt);
 
   /// Process the expression contained within a decltype. For such expressions,
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index c5003d9ac0254..8d47602712d3a 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -9083,7 +9083,8 @@ Sema::CorrectDelayedTyposInExpr(Expr *E, VarDecl *InitDecl,
 
 ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC,
                                      bool DiscardedValue, bool IsConstexpr,
-                                     bool IsTemplateArgument) {
+                                     bool IsTemplateArgument,
+                                     bool PerformsCheckCompletedExpr) {
   ExprResult FullExpr = FE;
 
   if (!FullExpr.get())
@@ -9117,7 +9118,8 @@ ExprResult Sema::ActOnFinishFullExpr(Expr *FE, SourceLocation CC,
   if (FullExpr.isInvalid())
     return ExprError();
 
-  CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
+  if (PerformsCheckCompletedExpr)
+    CheckCompletedExpr(FullExpr.get(), CC, IsConstexpr);
 
   // At the end of this full expression (which could be a deeply nested
   // lambda), if there is a potential capture within the nested lambda,
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index cecb80f8fb7fd..87a5457664fd1 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -9758,7 +9758,11 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr,
         LastIteration.get(), UB.get());
     EUB = SemaRef.BuildBinOp(CurScope, InitLoc, BO_Assign, UB.get(),
                              CondOp.get());
-    EUB = SemaRef.ActOnFinishFullExpr(EUB.get(), /*DiscardedValue*/ false);
+    EUB =
+        isOpenMPTaskLoopDirective(DKind)
+            ? SemaRef.ActOnFinishFullExprNoCheckExpr(EUB.get(),
+                                                     /*DiscardedValue*/ false)
+            : SemaRef.ActOnFinishFullExpr(EUB.get(), /*DiscardedValue*/ false);
 
     // If we have a combined directive that combines 'distribute', 'for' or
     // 'simd' we need to be able to access the bounds of the schedule of the
@@ -9788,7 +9792,11 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr,
       CombEUB = SemaRef.BuildBinOp(CurScope, InitLoc, BO_Assign, CombUB.get(),
                                    CombCondOp.get());
       CombEUB =
-          SemaRef.ActOnFinishFullExpr(CombEUB.get(), /*DiscardedValue*/ false);
+          isOpenMPTaskLoopDirective(DKind)
+              ? SemaRef.ActOnFinishFullExprNoCheckExpr(CombEUB.get(),
+                                                       /*DiscardedValue*/ false)
+              : SemaRef.ActOnFinishFullExpr(CombEUB.get(),
+                                            /*DiscardedValue*/ false);
 
       const CapturedDecl *CD = cast<CapturedStmt>(AStmt)->getCapturedDecl();
       // We expect to have at least 2 more parameters than the 'parallel'
@@ -9857,7 +9865,13 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr,
                         SemaRef.ActOnIntegerConstant(SourceLocation(), 1).get())
             .get();
     BoundUB =
-        SemaRef.ActOnFinishFullExpr(BoundUB, /*DiscardedValue*/ false).get();
+        isOpenMPTaskLoopDirective(DKind)
+            ? SemaRef
+                  .ActOnFinishFullExprNoCheckExpr(BoundUB,
+                                                  /*DiscardedValue*/ false)
+                  .get()
+            : SemaRef.ActOnFinishFullExpr(BoundUB, /*DiscardedValue*/ false)
+                  .get();
   }
   ExprResult Cond =
       (isOpenMPWorksharingDirective(DKind) ||
@@ -9886,8 +9900,14 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr,
                   SemaRef.ActOnIntegerConstant(SourceLocation(), 1).get())
               .get();
       BoundCombUB =
-          SemaRef.ActOnFinishFullExpr(BoundCombUB, /*DiscardedValue*/ false)
-              .get();
+          isOpenMPTaskLoopDirective(DKind)
+              ? SemaRef
+                    .ActOnFinishFullExprNoCheckExpr(BoundCombUB,
+                                                    /*DiscardedValue*/ false)
+                    .get()
+              : SemaRef
+                    .ActOnFinishFullExpr(BoundCombUB, /*DiscardedValue*/ false)
+                    .get();
     }
     CombCond =
         SemaRef.BuildBinOp(CurScope, CondLoc, UseStrictCompare ? BO_LT : BO_LE,
@@ -9922,7 +9942,11 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr,
     NextLB =
         SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, LB.get(), NextLB.get());
     NextLB =
-        SemaRef.ActOnFinishFullExpr(NextLB.get(), /*DiscardedValue*/ false);
+        isOpenMPTaskLoopDirective(DKind)
+            ? SemaRef.ActOnFinishFullExprNoCheckExpr(NextLB.get(),
+                                                     /*DiscardedValue*/ false)
+            : SemaRef.ActOnFinishFullExpr(NextLB.get(),
+                                          /*DiscardedValue*/ false);
     if (!NextLB.isUsable())
       return 0;
     // UB + ST
@@ -9933,7 +9957,11 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr,
     NextUB =
         SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, UB.get(), NextUB.get());
     NextUB =
-        SemaRef.ActOnFinishFullExpr(NextUB.get(), /*DiscardedValue*/ false);
+        isOpenMPTaskLoopDirective(DKind)
+            ? SemaRef.ActOnFinishFullExprNoCheckExpr(NextUB.get(),
+                                                     /*DiscardedValue*/ false)
+            : SemaRef.ActOnFinishFullExpr(NextUB.get(),
+                                          /*DiscardedValue*/ false);
     if (!NextUB.isUsable())
       return 0;
     if (isOpenMPLoopBoundSharingDirective(DKind)) {
@@ -9944,8 +9972,12 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr,
       // LB = LB + ST
       CombNextLB = SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, CombLB.get(),
                                       CombNextLB.get());
-      CombNextLB = SemaRef.ActOnFinishFullExpr(CombNextLB.get(),
-                                               /*DiscardedValue*/ false);
+      CombNextLB =
+          isOpenMPTaskLoopDirective(DKind)
+              ? SemaRef.ActOnFinishFullExprNoCheckExpr(CombNextLB.get(),
+                                                       /*DiscardedValue*/ false)
+              : SemaRef.ActOnFinishFullExpr(CombNextLB.get(),
+                                            /*DiscardedValue*/ false);
       if (!CombNextLB.isUsable())
         return 0;
       // UB + ST
@@ -9956,8 +9988,12 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr,
       // UB = UB + ST
       CombNextUB = SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, CombUB.get(),
                                       CombNextUB.get());
-      CombNextUB = SemaRef.ActOnFinishFullExpr(CombNextUB.get(),
-                                               /*DiscardedValue*/ false);
+      CombNextUB =
+          isOpenMPTaskLoopDirective(DKind)
+              ? SemaRef.ActOnFinishFullExprNoCheckExpr(CombNextUB.get(),
+                                                       /*DiscardedValue*/ false)
+              : SemaRef.ActOnFinishFullExpr(CombNextUB.get(),
+                                            /*DiscardedValue*/ false);
       if (!CombNextUB.isUsable())
         return 0;
     }
@@ -10003,7 +10039,11 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr,
     PrevEUB = SemaRef.BuildBinOp(CurScope, DistIncLoc, BO_Assign, UB.get(),
                                  CondOp.get());
     PrevEUB =
-        SemaRef.ActOnFinishFullExpr(PrevEUB.get(), /*DiscardedValue*/ false);
+        isOpenMPTaskLoopDirective(DKind)
+            ? SemaRef.ActOnFinishFullExprNoCheckExpr(PrevEUB.get(),
+                                                     /*DiscardedValue*/ false)
+            : SemaRef.ActOnFinishFullExpr(PrevEUB.get(),
+                                          /*DiscardedValue*/ false);
 
     // Build IV <= PrevUB or IV < PrevUB + 1 for unsigned IV to be used in
     // parallel for is in combination with a distribute directive with
@@ -10016,9 +10056,15 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr,
                   CurScope, CondLoc, BO_Add, BoundPrevUB,
                   SemaRef.ActOnIntegerConstant(SourceLocation(), 1).get())
               .get();
-      BoundPrevUB =
-          SemaRef.ActOnFinishFullExpr(BoundPrevUB, /*DiscardedValue*/ false)
-              .get();
+      BoundPrevUB = isOpenMPTaskLoopDirective(DKind)
+                        ? SemaRef
+                              .ActOnFinishFullExprNoCheckExpr(
+                                  BoundPrevUB, /*DiscardedValue*/ false)
+                              .get()
+                        : SemaRef
+                              .ActOnFinishFullExpr(BoundPrevUB,
+                                                   /*DiscardedValue*/ false)
+                              .get();
     }
     ParForInDistCond =
         SemaRef.BuildBinOp(CurScope, CondLoc, UseStrictCompare ? BO_LT : BO_LE,

``````````

</details>


https://github.com/llvm/llvm-project/pull/101812


More information about the cfe-commits mailing list