Skip to content

Commit 8d48a0b

Browse files
yronglinyronglin
committed
[Clang] Allow the value of unroll count to be zero in '#pragma GCC unroll' and '#pragma unroll'
Signed-off-by: yronglin <[email protected]>
1 parent b8d0cba commit 8d48a0b

File tree

9 files changed

+66
-14
lines changed

9 files changed

+66
-14
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,10 @@ Bug Fixes in This Version
423423
- Fixed a regression in CTAD that a friend declaration that befriends itself may cause
424424
incorrect constraint substitution. (#GH86769).
425425

426+
- Clang now allowed the value of unroll count to be zero in ``#pragma GCC unroll`` and ``#pragma unroll``.
427+
The values of 0 and 1 block any unrolling of the loop. This keeps the same behavior with GCC.
428+
Fixes (`#88624 <https://github.com/llvm/llvm-project/issues/88624>`_).
429+
426430
Bug Fixes to Compiler Builtins
427431
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
428432

‎clang/include/clang/Sema/Sema.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5469,7 +5469,8 @@ class Sema final : public SemaBase {
54695469
ExprResult ActOnPredefinedExpr(SourceLocation Loc, tok::TokenKind Kind);
54705470
ExprResult ActOnIntegerConstant(SourceLocation Loc, uint64_t Val);
54715471

5472-
bool CheckLoopHintExpr(Expr *E, SourceLocation Loc);
5472+
bool CheckLoopHintExpr(Expr *E, SourceLocation Loc,
5473+
const IdentifierInfo *PragmaNameInfo);
54735474

54745475
ExprResult ActOnNumericConstant(const Token &Tok, Scope *UDLScope = nullptr);
54755476
ExprResult ActOnCharacterConstant(const Token &Tok,

‎clang/lib/CodeGen/CGLoopInfo.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,8 @@ void LoopInfoStack::push(BasicBlock *Header, clang::ASTContext &Ctx,
673673
setPipelineDisabled(true);
674674
break;
675675
case LoopHintAttr::UnrollCount:
676+
setUnrollState(LoopAttributes::Disable);
677+
break;
676678
case LoopHintAttr::UnrollAndJamCount:
677679
case LoopHintAttr::VectorizeWidth:
678680
case LoopHintAttr::InterleaveCount:

‎clang/lib/Parse/ParsePragma.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1569,7 +1569,8 @@ bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
15691569
ConsumeToken(); // Consume the constant expression eof terminator.
15701570

15711571
if (Arg2Error || R.isInvalid() ||
1572-
Actions.CheckLoopHintExpr(R.get(), Toks[0].getLocation()))
1572+
Actions.CheckLoopHintExpr(R.get(), Toks[0].getLocation(),
1573+
PragmaNameInfo))
15731574
return false;
15741575

15751576
// Argument is a constant expression with an integer type.
@@ -1593,8 +1594,8 @@ bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
15931594

15941595
ConsumeToken(); // Consume the constant expression eof terminator.
15951596

1596-
if (R.isInvalid() ||
1597-
Actions.CheckLoopHintExpr(R.get(), Toks[0].getLocation()))
1597+
if (R.isInvalid() || Actions.CheckLoopHintExpr(
1598+
R.get(), Toks[0].getLocation(), PragmaNameInfo))
15981599
return false;
15991600

16001601
// Argument is a constant expression with an integer type.

‎clang/lib/Sema/SemaExpr.cpp

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3885,7 +3885,8 @@ static Expr *BuildFloatingLiteral(Sema &S, NumericLiteralParser &Literal,
38853885
return FloatingLiteral::Create(S.Context, Val, isExact, Ty, Loc);
38863886
}
38873887

3888-
bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc) {
3888+
bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc,
3889+
const IdentifierInfo *PragmaNameInfo) {
38893890
assert(E && "Invalid expression");
38903891

38913892
if (E->isValueDependent())
@@ -3903,7 +3904,14 @@ bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc) {
39033904
if (R.isInvalid())
39043905
return true;
39053906

3906-
bool ValueIsPositive = ValueAPS.isStrictlyPositive();
3907+
// GCC allows the value of unroll count to be 0.
3908+
// https://gcc.gnu.org/onlinedocs/gcc/Loop-Specific-Pragmas.html says
3909+
// "The values of 0 and 1 block any unrolling of the loop."
3910+
// The values doesn't have to be strictly positive in '#pragma GCC unroll' and
3911+
// '#pragma unroll' cases.
3912+
bool ValueIsPositive = PragmaNameInfo->isStr("unroll")
3913+
? ValueAPS.isNonNegative()
3914+
: ValueAPS.isStrictlyPositive();
39073915
if (!ValueIsPositive || ValueAPS.getActiveBits() > 31) {
39083916
Diag(E->getExprLoc(), diag::err_pragma_loop_invalid_argument_value)
39093917
<< toString(ValueAPS, 10) << ValueIsPositive;

‎clang/lib/Sema/SemaStmtAttr.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,17 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const ParsedAttr &A,
109109
SetHints(LoopHintAttr::Unroll, LoopHintAttr::Disable);
110110
} else if (PragmaName == "unroll") {
111111
// #pragma unroll N
112-
if (ValueExpr)
113-
SetHints(LoopHintAttr::UnrollCount, LoopHintAttr::Numeric);
114-
else
112+
if (ValueExpr) {
113+
llvm::APSInt ValueAPS;
114+
ExprResult R = S.VerifyIntegerConstantExpression(ValueExpr, &ValueAPS);
115+
assert(!R.isInvalid() && "unroll count value must be a valid value, it's "
116+
"should be checked in Sema::CheckLoopHintExpr");
117+
// The values of 0 and 1 block any unrolling of the loop.
118+
if (ValueAPS.isZero() || ValueAPS.isOne())
119+
SetHints(LoopHintAttr::UnrollCount, LoopHintAttr::Disable);
120+
else
121+
SetHints(LoopHintAttr::UnrollCount, LoopHintAttr::Numeric);
122+
} else
115123
SetHints(LoopHintAttr::Unroll, LoopHintAttr::Enable);
116124
} else if (PragmaName == "nounroll_and_jam") {
117125
SetHints(LoopHintAttr::UnrollAndJam, LoopHintAttr::Disable);
@@ -142,7 +150,8 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const ParsedAttr &A,
142150
if (Option == LoopHintAttr::VectorizeWidth) {
143151
assert((ValueExpr || (StateLoc && StateLoc->Ident)) &&
144152
"Attribute must have a valid value expression or argument.");
145-
if (ValueExpr && S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc()))
153+
if (ValueExpr &&
154+
S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc(), OptionLoc->Ident))
146155
return nullptr;
147156
if (StateLoc && StateLoc->Ident && StateLoc->Ident->isStr("scalable"))
148157
State = LoopHintAttr::ScalableWidth;
@@ -152,7 +161,7 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const ParsedAttr &A,
152161
Option == LoopHintAttr::UnrollCount ||
153162
Option == LoopHintAttr::PipelineInitiationInterval) {
154163
assert(ValueExpr && "Attribute must have a valid value expression.");
155-
if (S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc()))
164+
if (S.CheckLoopHintExpr(ValueExpr, St->getBeginLoc(), OptionLoc->Ident))
156165
return nullptr;
157166
State = LoopHintAttr::Numeric;
158167
} else if (Option == LoopHintAttr::Vectorize ||

‎clang/lib/Sema/SemaTemplateInstantiate.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2150,7 +2150,8 @@ TemplateInstantiator::TransformLoopHintAttr(const LoopHintAttr *LH) {
21502150
return LH;
21512151

21522152
// Generate error if there is a problem with the value.
2153-
if (getSema().CheckLoopHintExpr(TransformedExpr, LH->getLocation()))
2153+
if (getSema().CheckLoopHintExpr(TransformedExpr, LH->getLocation(),
2154+
LH->getAttrName()))
21542155
return LH;
21552156

21562157
// Create new LoopHintValueAttr with integral expression in place of the

‎clang/test/CodeGenCXX/pragma-gcc-unroll.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,26 @@ void template_test(double *List, int Length) {
9696
for_template_define_test<double>(List, Length, Value);
9797
}
9898

99+
void for_unroll_zero_test(int *List, int Length) {
100+
// CHECK: define {{.*}} @_Z20for_unroll_zero_testPii
101+
#pragma GCC unroll 0
102+
for (int i = 0; i < Length; i++) {
103+
// CHECK: br label {{.*}}, !llvm.loop ![[LOOP_14:.*]]
104+
List[i] = i * 2;
105+
}
106+
}
107+
108+
void while_unroll_zero_test(int *List, int Length) {
109+
// CHECK: define {{.*}} @_Z22while_unroll_zero_testPii
110+
int i = 0;
111+
#pragma GCC unroll(0)
112+
while (i < Length) {
113+
// CHECK: br label {{.*}}, !llvm.loop ![[LOOP_15:.*]]
114+
List[i] = i * 2;
115+
i++;
116+
}
117+
}
118+
99119
// CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], [[MP:![0-9]+]], ![[UNROLL_ENABLE:.*]]}
100120
// CHECK: ![[UNROLL_ENABLE]] = !{!"llvm.loop.unroll.enable"}
101121
// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2:.*]], ![[UNROLL_DISABLE:.*]]}
@@ -107,3 +127,5 @@ void template_test(double *List, int Length) {
107127
// CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNROLL_8:.*]]}
108128
// CHECK: ![[LOOP_6]] = distinct !{![[LOOP_6]], ![[UNROLL_8:.*]]}
109129
// CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[UNROLL_8:.*]]}
130+
// CHECK: ![[LOOP_14]] = distinct !{![[LOOP_14]], [[MP]], ![[UNROLL_DISABLE:.*]]}
131+
// CHECK: ![[LOOP_15]] = distinct !{![[LOOP_15]], [[MP]], ![[UNROLL_DISABLE:.*]]}

‎clang/test/Parser/pragma-unroll.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,18 @@ void test(int *List, int Length) {
4040

4141
/* expected-error {{expected ')'}} */ #pragma unroll(()
4242
/* expected-error {{expected expression}} */ #pragma unroll -
43-
/* expected-error {{invalid value '0'; must be positive}} */ #pragma unroll(0)
44-
/* expected-error {{invalid value '0'; must be positive}} */ #pragma unroll 0
43+
/* The values of 0 and 1 block any unrolling of the loop. */ #pragma unroll 0
4544
/* expected-error {{value '3000000000' is too large}} */ #pragma unroll(3000000000)
4645
/* expected-error {{value '3000000000' is too large}} */ #pragma unroll 3000000000
4746
while (i-8 < Length) {
4847
List[i] = i;
4948
}
5049

50+
/* The values of 0 and 1 block any unrolling of the loop. */ #pragma unroll(0)
51+
while (i-8 < Length) {
52+
List[i] = i;
53+
}
54+
5155
#pragma unroll
5256
/* expected-error {{expected a for, while, or do-while loop to follow '#pragma unroll'}} */ int j = Length;
5357
#pragma unroll 4

0 commit comments

Comments
 (0)