https://github.com/yronglin updated https://github.com/llvm/llvm-project/pull/88666
>From 8d48a0bd1cf15b9cf00bc294912b674b5f94a11c Mon Sep 17 00:00:00 2001 From: yronglin <yronglin...@gmail.com> Date: Mon, 15 Apr 2024 00:36:06 +0800 Subject: [PATCH] [Clang] Allow the value of unroll count to be zero in '#pragma GCC unroll' and '#pragma unroll' Signed-off-by: yronglin <yronglin...@gmail.com> --- clang/docs/ReleaseNotes.rst | 4 ++++ clang/include/clang/Sema/Sema.h | 3 ++- clang/lib/CodeGen/CGLoopInfo.cpp | 2 ++ clang/lib/Parse/ParsePragma.cpp | 7 ++++--- clang/lib/Sema/SemaExpr.cpp | 12 +++++++++-- clang/lib/Sema/SemaStmtAttr.cpp | 19 +++++++++++++----- clang/lib/Sema/SemaTemplateInstantiate.cpp | 3 ++- clang/test/CodeGenCXX/pragma-gcc-unroll.cpp | 22 +++++++++++++++++++++ clang/test/Parser/pragma-unroll.cpp | 8 ++++++-- 9 files changed, 66 insertions(+), 14 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ade8f4e93d5a0c..5183edcf01b1cc 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -423,6 +423,10 @@ Bug Fixes in This Version - Fixed a regression in CTAD that a friend declaration that befriends itself may cause incorrect constraint substitution. (#GH86769). +- Clang now allowed the value of unroll count to be zero in ``#pragma GCC unroll`` and ``#pragma unroll``. + The values of 0 and 1 block any unrolling of the loop. This keeps the same behavior with GCC. + Fixes (`#88624 <https://github.com/llvm/llvm-project/issues/88624>`_). + Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index c6e0332c3176b3..3b2f3a6d82675c 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -5469,7 +5469,8 @@ class Sema final : public SemaBase { ExprResult ActOnPredefinedExpr(SourceLocation Loc, tok::TokenKind Kind); ExprResult ActOnIntegerConstant(SourceLocation Loc, uint64_t Val); - bool CheckLoopHintExpr(Expr *E, SourceLocation Loc); + bool CheckLoopHintExpr(Expr *E, SourceLocation Loc, + const IdentifierInfo *PragmaNameInfo); ExprResult ActOnNumericConstant(const Token &Tok, Scope *UDLScope = nullptr); ExprResult ActOnCharacterConstant(const Token &Tok, diff --git a/clang/lib/CodeGen/CGLoopInfo.cpp b/clang/lib/CodeGen/CGLoopInfo.cpp index 0d4800b90a2f26..72d1471021ac02 100644 --- a/clang/lib/CodeGen/CGLoopInfo.cpp +++ b/clang/lib/CodeGen/CGLoopInfo.cpp @@ -673,6 +673,8 @@ void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx, setPipelineDisabled(true); break; case LoopHintAttr::UnrollCount: + setUnrollState(LoopAttributes::Disable); + break; case LoopHintAttr::UnrollAndJamCount: case LoopHintAttr::VectorizeWidth: case LoopHintAttr::InterleaveCount: diff --git a/clang/lib/Parse/ParsePragma.cpp b/clang/lib/Parse/ParsePragma.cpp index 3979f75b6020db..8fed9e70f7a56e 100644 --- a/clang/lib/Parse/ParsePragma.cpp +++ b/clang/lib/Parse/ParsePragma.cpp @@ -1569,7 +1569,8 @@ bool Parser::HandlePragmaLoopHint(LoopHint &Hint) { ConsumeToken(); // Consume the constant expression eof terminator. if (Arg2Error || R.isInvalid() || - Actions.CheckLoopHintExpr(R.get(), Toks[0].getLocation())) + Actions.CheckLoopHintExpr(R.get(), Toks[0].getLocation(), + PragmaNameInfo)) return false; // Argument is a constant expression with an integer type. @@ -1593,8 +1594,8 @@ bool Parser::HandlePragmaLoopHint(LoopHint &Hint) { ConsumeToken(); // Consume the constant expression eof terminator. - if (R.isInvalid() || - Actions.CheckLoopHintExpr(R.get(), Toks[0].getLocation())) + if (R.isInvalid() || Actions.CheckLoopHintExpr( + R.get(), Toks[0].getLocation(), PragmaNameInfo)) return false; // Argument is a constant expression with an integer type. diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 505d068ac42ebe..437b31716ed30c 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -3885,7 +3885,8 @@ static Expr *BuildFloatingLiteral(Sema &S, NumericLiteralParser &Literal, return FloatingLiteral::Create(S.Context, Val, isExact, Ty, Loc); } -bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc) { +bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc, + const IdentifierInfo *PragmaNameInfo) { assert(E && "Invalid expression"); if (E->isValueDependent()) @@ -3903,7 +3904,14 @@ bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc) { if (R.isInvalid()) return true; - bool ValueIsPositive = ValueAPS.isStrictlyPositive(); + // GCC allows the value of unroll count to be 0. + // https://gcc.gnu.org/onlinedocs/gcc/Loop-Specific-Pragmas.html says + // "The values of 0 and 1 block any unrolling of the loop." + // The values doesn't have to be strictly positive in '#pragma GCC unroll' and + // '#pragma unroll' cases. + bool ValueIsPositive = PragmaNameInfo->isStr("unroll") + ? ValueAPS.isNonNegative() + : ValueAPS.isStrictlyPositive(); if (!ValueIsPositive || ValueAPS.getActiveBits() > 31) { Diag(E->getExprLoc(), diag::err_pragma_loop_invalid_argument_value) << toString(ValueAPS, 10) << ValueIsPositive; diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp index a0339273a0ba35..403843daa4616e 100644 --- a/clang/lib/Sema/SemaStmtAttr.cpp +++ b/clang/lib/Sema/SemaStmtAttr.cpp @@ -109,9 +109,17 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const ParsedAttr &A, SetHints(LoopHintAttr::Unroll, LoopHintAttr::Disable); } else if (PragmaName == "unroll") { // #pragma unroll N - if (ValueExpr) - SetHints(LoopHintAttr::UnrollCount, LoopHintAttr::Numeric); - else + if (ValueExpr) { + llvm::APSInt ValueAPS; + ExprResult R = S.VerifyIntegerConstantExpression(ValueExpr, &ValueAPS); + assert(!R.isInvalid() && "unroll count value must be a valid value, it's " + "should be checked in Sema::CheckLoopHintExpr"); + // The values of 0 and 1 block any unrolling of the loop. + if (ValueAPS.isZero() || ValueAPS.isOne()) + SetHints(LoopHintAttr::UnrollCount, LoopHintAttr::Disable); + else + SetHints(LoopHintAttr::UnrollCount, LoopHintAttr::Numeric); + } else SetHints(LoopHintAttr::Unroll, LoopHintAttr::Enable); } else if (PragmaName == "nounroll_and_jam") { SetHints(LoopHintAttr::UnrollAndJam, LoopHintAttr::Disable); @@ -142,7 +150,8 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const ParsedAttr &A, if (Option == LoopHintAttr::VectorizeWidth) { assert((ValueExpr || (StateLoc && StateLoc->Ident)) && "Attribute must have a valid value expression or argument."); - if (ValueExpr && S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc())) + if (ValueExpr && + S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc(), OptionLoc->Ident)) return nullptr; if (StateLoc && StateLoc->Ident && StateLoc->Ident->isStr("scalable")) State = LoopHintAttr::ScalableWidth; @@ -152,7 +161,7 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const ParsedAttr &A, Option == LoopHintAttr::UnrollCount || Option == LoopHintAttr::PipelineInitiationInterval) { assert(ValueExpr && "Attribute must have a valid value expression."); - if (S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc())) + if (S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc(), OptionLoc->Ident)) return nullptr; State = LoopHintAttr::Numeric; } else if (Option == LoopHintAttr::Vectorize || diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 7cd428de0bb32d..c98b44e0c4a227 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -2150,7 +2150,8 @@ TemplateInstantiator::TransformLoopHintAttr(const LoopHintAttr *LH) { return LH; // Generate error if there is a problem with the value. - if (getSema().CheckLoopHintExpr(TransformedExpr, LH->getLocation())) + if (getSema().CheckLoopHintExpr(TransformedExpr, LH->getLocation(), + LH->getAttrName())) return LH; // Create new LoopHintValueAttr with integral expression in place of the diff --git a/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp b/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp index ed75e0b6e3c364..8a94a5cc91e239 100644 --- a/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp +++ b/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp @@ -96,6 +96,26 @@ void template_test(double *List, int Length) { for_template_define_test<double>(List, Length, Value); } +void for_unroll_zero_test(int *List, int Length) { + // CHECK: define {{.*}} @_Z20for_unroll_zero_testPii + #pragma GCC unroll 0 + for (int i = 0; i < Length; i++) { + // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_14:.*]] + List[i] = i * 2; + } +} + +void while_unroll_zero_test(int *List, int Length) { + // CHECK: define {{.*}} @_Z22while_unroll_zero_testPii + int i = 0; +#pragma GCC unroll(0) + while (i < Length) { + // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_15:.*]] + List[i] = i * 2; + i++; + } +} + // CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], [[MP:![0-9]+]], ![[UNROLL_ENABLE:.*]]} // CHECK: ![[UNROLL_ENABLE]] = !{!"llvm.loop.unroll.enable"} // CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2:.*]], ![[UNROLL_DISABLE:.*]]} @@ -107,3 +127,5 @@ void template_test(double *List, int Length) { // CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNROLL_8:.*]]} // CHECK: ![[LOOP_6]] = distinct !{![[LOOP_6]], ![[UNROLL_8:.*]]} // CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[UNROLL_8:.*]]} +// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], [[MP]], ![[UNROLL_DISABLE:.*]]} +// CHECK: ![[LOOP_15]] = distinct !{![[LOOP_15]], [[MP]], ![[UNROLL_DISABLE:.*]]} diff --git a/clang/test/Parser/pragma-unroll.cpp b/clang/test/Parser/pragma-unroll.cpp index c89cf49a002065..f41bd7a18d5a41 100644 --- a/clang/test/Parser/pragma-unroll.cpp +++ b/clang/test/Parser/pragma-unroll.cpp @@ -40,14 +40,18 @@ void test(int *List, int Length) { /* expected-error {{expected ')'}} */ #pragma unroll(() /* expected-error {{expected expression}} */ #pragma unroll - -/* expected-error {{invalid value '0'; must be positive}} */ #pragma unroll(0) -/* expected-error {{invalid value '0'; must be positive}} */ #pragma unroll 0 +/* The values of 0 and 1 block any unrolling of the loop. */ #pragma unroll 0 /* expected-error {{value '3000000000' is too large}} */ #pragma unroll(3000000000) /* expected-error {{value '3000000000' is too large}} */ #pragma unroll 3000000000 while (i-8 < Length) { List[i] = i; } +/* The values of 0 and 1 block any unrolling of the loop. */ #pragma unroll(0) + while (i-8 < Length) { + List[i] = i; + } + #pragma unroll /* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll'}} */ int j = Length; #pragma unroll 4 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
