Skip to content

Commit c554816

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 d488b22 commit c554816

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
@@ -420,6 +420,10 @@ Bug Fixes in This Version
420420
- Fixed a regression in CTAD that a friend declaration that befriends itself may cause
421421
incorrect constraint substitution. (#GH86769).
422422

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

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

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

5452-
bool CheckLoopHintExpr(Expr *E, SourceLocation Loc);
5452+
bool CheckLoopHintExpr(Expr *E, SourceLocation Loc,
5453+
const IdentifierInfo *PragmaNameInfo);
54535454

54545455
ExprResult ActOnNumericConstant(const Token &Tok, Scope *UDLScope = nullptr);
54555456
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
@@ -1568,7 +1568,8 @@ bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
15681568
ConsumeToken(); // Consume the constant expression eof terminator.
15691569

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

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

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

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

15991600
// 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
@@ -3880,7 +3880,8 @@ static Expr *BuildFloatingLiteral(Sema &S, NumericLiteralParser &Literal,
38803880
return FloatingLiteral::Create(S.Context, Val, isExact, Ty, Loc);
38813881
}
38823882

3883-
bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc) {
3883+
bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc,
3884+
const IdentifierInfo *PragmaNameInfo) {
38843885
assert(E && "Invalid expression");
38853886

38863887
if (E->isValueDependent())
@@ -3898,7 +3899,14 @@ bool Sema::CheckLoopHintExpr(Expr *E, SourceLocation Loc) {
38983899
if (R.isInvalid())
38993900
return true;
39003901

3901-
bool ValueIsPositive = ValueAPS.isStrictlyPositive();
3902+
// GCC allows the value of unroll count to be 0.
3903+
// https://gcc.gnu.org/onlinedocs/gcc/Loop-Specific-Pragmas.html says
3904+
// "The values of 0 and 1 block any unrolling of the loop."
3905+
// The values doesn't have to be strictly positive in '#pragma GCC unroll' and
3906+
// '#pragma unroll' cases.
3907+
bool ValueIsPositive = PragmaNameInfo->isStr("unroll")
3908+
? ValueAPS.isNonNegative()
3909+
: ValueAPS.isStrictlyPositive();
39023910
if (!ValueIsPositive || ValueAPS.getActiveBits() > 31) {
39033911
Diag(E->getExprLoc(), diag::err_pragma_loop_invalid_argument_value)
39043912
<< 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
@@ -2147,7 +2147,8 @@ TemplateInstantiator::TransformLoopHintAttr(const LoopHintAttr *LH) {
21472147
return LH;
21482148

21492149
// Generate error if there is a problem with the value.
2150-
if (getSema().CheckLoopHintExpr(TransformedExpr, LH->getLocation()))
2150+
if (getSema().CheckLoopHintExpr(TransformedExpr, LH->getLocation(),
2151+
LH->getAttrName()))
21512152
return LH;
21522153

21532154
// 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)