https://github.com/HenryZ16 created https://github.com/llvm/llvm-project/pull/101307
This patch is to fix [issue: Implicit conversion with `pragma omp taskloop`](https://github.com/llvm/llvm-project/issues/100536) #100536 As is seen in the issue, clang will report error while compiling with `-Werror -Wconversion`: ```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; } ``` During my revision of the code, I find out that the code in clang/lib/Sema/SemaOpenMP.cpp seems to automatically set the type of the iteration variable declared in the for loop to unsigned long. Note that clang has performed the type checking on the for loop before parsing & semantic checking on the OpenMP part. So I deleted the duplicated function `ActOnFinishFullExpr`, which will perform the type checking on the for loop again. Thus, clang no longer report such an error, and it can still pass `make clang-test`. However, this can only be a temporary expedient. When I compare the x86_64 assembly code generated by clang and gcc for this code, I find out that, for each "for" loop, clang always uses unsigned number judgment statement, while gcc can automatically choose the type of number judgment statement according to the sign of the iteration variable. I don't know whether it's a potential error. >From 75879fa7a7e163ea350c11f37b24c0b2f5231c35 Mon Sep 17 00:00:00 2001 From: HenryZ16 <1546169...@qq.com> Date: Wed, 31 Jul 2024 02:34:04 -0600 Subject: [PATCH] fix issue #100536: Implicit conversion with `pragma omp taskloop` --- clang/lib/Sema/SemaOpenMP.cpp | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 4f50efda155fb..ddf1dd9824e04 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -9758,7 +9758,6 @@ 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); // 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 @@ -9787,8 +9786,6 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, LastIteration.get(), CombUB.get()); CombEUB = SemaRef.BuildBinOp(CurScope, InitLoc, BO_Assign, CombUB.get(), CombCondOp.get()); - CombEUB = - 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' @@ -9824,7 +9821,6 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, ? LB.get() : SemaRef.ActOnIntegerConstant(SourceLocation(), 0).get(); Init = SemaRef.BuildBinOp(CurScope, InitLoc, BO_Assign, IV.get(), RHS); - Init = SemaRef.ActOnFinishFullExpr(Init.get(), /*DiscardedValue*/ false); if (isOpenMPLoopBoundSharingDirective(DKind)) { Expr *CombRHS = @@ -9836,8 +9832,6 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, : SemaRef.ActOnIntegerConstant(SourceLocation(), 0).get(); CombInit = SemaRef.BuildBinOp(CurScope, InitLoc, BO_Assign, IV.get(), CombRHS); - CombInit = - SemaRef.ActOnFinishFullExpr(CombInit.get(), /*DiscardedValue*/ false); } } @@ -9856,8 +9850,6 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, .BuildBinOp(CurScope, CondLoc, BO_Add, BoundUB, SemaRef.ActOnIntegerConstant(SourceLocation(), 1).get()) .get(); - BoundUB = - SemaRef.ActOnFinishFullExpr(BoundUB, /*DiscardedValue*/ false).get(); } ExprResult Cond = (isOpenMPWorksharingDirective(DKind) || @@ -9885,9 +9877,6 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, CurScope, CondLoc, BO_Add, BoundCombUB, SemaRef.ActOnIntegerConstant(SourceLocation(), 1).get()) .get(); - BoundCombUB = - SemaRef.ActOnFinishFullExpr(BoundCombUB, /*DiscardedValue*/ false) - .get(); } CombCond = SemaRef.BuildBinOp(CurScope, CondLoc, UseStrictCompare ? BO_LT : BO_LE, @@ -9901,7 +9890,6 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, if (!Inc.isUsable()) return 0; Inc = SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, IV.get(), Inc.get()); - Inc = SemaRef.ActOnFinishFullExpr(Inc.get(), /*DiscardedValue*/ false); if (!Inc.isUsable()) return 0; @@ -9921,8 +9909,6 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, // LB = LB + ST NextLB = SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, LB.get(), NextLB.get()); - NextLB = - SemaRef.ActOnFinishFullExpr(NextLB.get(), /*DiscardedValue*/ false); if (!NextLB.isUsable()) return 0; // UB + ST @@ -9932,8 +9918,6 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, // UB = UB + ST NextUB = SemaRef.BuildBinOp(CurScope, IncLoc, BO_Assign, UB.get(), NextUB.get()); - NextUB = - SemaRef.ActOnFinishFullExpr(NextUB.get(), /*DiscardedValue*/ false); if (!NextUB.isUsable()) return 0; if (isOpenMPLoopBoundSharingDirective(DKind)) { @@ -9944,8 +9928,6 @@ 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); if (!CombNextLB.isUsable()) return 0; // UB + ST @@ -9956,8 +9938,6 @@ 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); if (!CombNextUB.isUsable()) return 0; } @@ -9979,8 +9959,6 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, assert(DistInc.isUsable() && "distribute inc expr was not built"); DistInc = SemaRef.BuildBinOp(CurScope, DistIncLoc, BO_Assign, IV.get(), DistInc.get()); - DistInc = - SemaRef.ActOnFinishFullExpr(DistInc.get(), /*DiscardedValue*/ false); assert(DistInc.isUsable() && "distribute inc expr was not built"); // Build expression: UB = min(UB, prevUB) for #for in composite or combined @@ -10002,8 +9980,6 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, DistEUBLoc, DistEUBLoc, IsUBGreater.get(), NewPrevUB.get(), UB.get()); PrevEUB = SemaRef.BuildBinOp(CurScope, DistIncLoc, BO_Assign, UB.get(), CondOp.get()); - PrevEUB = - 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 +9992,6 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, CurScope, CondLoc, BO_Add, BoundPrevUB, SemaRef.ActOnIntegerConstant(SourceLocation(), 1).get()) .get(); - BoundPrevUB = - SemaRef.ActOnFinishFullExpr(BoundPrevUB, /*DiscardedValue*/ false) - .get(); } ParForInDistCond = SemaRef.BuildBinOp(CurScope, CondLoc, UseStrictCompare ? BO_LT : BO_LE, @@ -10144,10 +10117,7 @@ checkOpenMPLoop(OpenMPDirectiveKind DKind, Expr *CollapseLoopCountExpr, Built.IterationVarRef = IV.get(); Built.LastIteration = LastIteration.get(); Built.NumIterations = NumIterations.get(); - Built.CalcLastIteration = SemaRef - .ActOnFinishFullExpr(CalcLastIteration.get(), - /*DiscardedValue=*/false) - .get(); + Built.CalcLastIteration = CalcLastIteration.get(); Built.PreCond = PreCond.get(); Built.PreInits = buildPreInits(C, Captures); Built.Cond = Cond.get(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
