https://github.com/yronglin updated
https://github.com/llvm/llvm-project/pull/90240
>From 131783211a23d007b5b6f1d5691306df4dec716b Mon Sep 17 00:00:00 2001
From: yronglin <yronglin...@gmail.com>
Date: Sat, 27 Apr 2024 01:38:34 +0800
Subject: [PATCH 1/3] [Clang] Fix incorrect handling of #pragma {GCC} unroll N
in dependent context
Signed-off-by: yronglin <yronglin...@gmail.com>
---
clang/lib/Sema/SemaStmtAttr.cpp | 23 ++++++++++--------
clang/test/Parser/pragma-unroll.cpp | 37 +++++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 10 deletions(-)
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index 9d44c22c8ddcc3..4dfdfd7aec425f 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -109,16 +109,19 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const
ParsedAttr &A,
SetHints(LoopHintAttr::Unroll, LoopHintAttr::Disable);
} else if (PragmaName == "unroll") {
// #pragma unroll N
- if (ValueExpr && !ValueExpr->isValueDependent()) {
- 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");
- (void)R;
- // The values of 0 and 1 block any unrolling of the loop.
- if (ValueAPS.isZero() || ValueAPS.isOne())
- SetHints(LoopHintAttr::UnrollCount, LoopHintAttr::Disable);
- else
+ if (ValueExpr) {
+ if (!ValueExpr->isValueDependent()) {
+ 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");
+ (void)R;
+ // 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::UnrollCount, LoopHintAttr::Numeric);
} else
SetHints(LoopHintAttr::Unroll, LoopHintAttr::Enable);
diff --git a/clang/test/Parser/pragma-unroll.cpp
b/clang/test/Parser/pragma-unroll.cpp
index f41bd7a18d5a41..e84f6ea9ad1035 100644
--- a/clang/test/Parser/pragma-unroll.cpp
+++ b/clang/test/Parser/pragma-unroll.cpp
@@ -124,3 +124,40 @@ void test(int *List, int Length) {
#pragma unroll
/* expected-error {{expected statement}} */ }
+
+using size_t = unsigned long long;
+
+template <bool Flag>
+int FailToBuild(int n) {
+ constexpr int N = 100;
+ auto init = [=]() { return Flag ? n : 0UL; };
+ auto cond = [=](size_t ix) { return Flag ? ix != 0 : ix < 10; };
+ auto iter = [=](size_t ix) {
+ return Flag ? ix & ~(1ULL << __builtin_clzll(ix)) : ix + 1;
+ };
+#pragma unroll Flag ? 1 : N
+ for (size_t ix = init(); cond(ix); ix = iter(ix)) {
+ n *= n;
+ }
+#pragma unroll Flag ? 0 : N
+ for (size_t ix = init(); cond(ix); ix = iter(ix)) {
+ n *= n;
+ }
+#pragma GCC unroll Flag ? 1 : N
+ for (size_t ix = init(); cond(ix); ix = iter(ix)) {
+ n *= n;
+ }
+#pragma GCC unroll Flag ? 0 : N
+ for (size_t ix = init(); cond(ix); ix = iter(ix)) {
+ n *= n;
+ }
+ return n;
+}
+
+int foo(int n) {
+ return FailToBuild<true>(n);
+}
+
+int bar(int n) {
+ return FailToBuild<false>(n);
+}
>From 3e5ef859c3d55830199a366edf48a8f536dc1208 Mon Sep 17 00:00:00 2001
From: yronglin <yronglin...@gmail.com>
Date: Sat, 27 Apr 2024 01:52:41 +0800
Subject: [PATCH 2/3] Format
Signed-off-by: yronglin <yronglin...@gmail.com>
---
clang/lib/Sema/SemaStmtAttr.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index 4dfdfd7aec425f..d3e76f282974e9 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -113,8 +113,9 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const
ParsedAttr &A,
if (!ValueExpr->isValueDependent()) {
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");
+ assert(!R.isInvalid() &&
+ "unroll count value must be a valid value, it's "
+ "should be checked in Sema::CheckLoopHintExpr");
(void)R;
// The values of 0 and 1 block any unrolling of the loop.
if (ValueAPS.isZero() || ValueAPS.isOne())
>From 35b63c6df6409a4e9dde07d8c658583614964856 Mon Sep 17 00:00:00 2001
From: yronglin <yronglin...@gmail.com>
Date: Sat, 27 Apr 2024 23:20:55 +0800
Subject: [PATCH 3/3] Address comments and add more test
Signed-off-by: yronglin <yronglin...@gmail.com>
---
clang/lib/CodeGen/CGLoopInfo.cpp | 2 -
clang/lib/Sema/SemaStmtAttr.cpp | 19 ++------
clang/lib/Sema/SemaTemplateInstantiate.cpp | 18 +++++--
clang/test/AST/ast-dump-pragma-unroll.cpp | 28 +++++++++++
clang/test/CodeGenCXX/pragma-gcc-unroll.cpp | 30 ++++++++++++
clang/test/CodeGenCXX/pragma-unroll.cpp | 52 +++++++++++++++++++++
clang/test/Parser/pragma-unroll.cpp | 12 +----
7 files changed, 132 insertions(+), 29 deletions(-)
create mode 100644 clang/test/AST/ast-dump-pragma-unroll.cpp
diff --git a/clang/lib/CodeGen/CGLoopInfo.cpp b/clang/lib/CodeGen/CGLoopInfo.cpp
index 72d1471021ac02..0d4800b90a2f26 100644
--- a/clang/lib/CodeGen/CGLoopInfo.cpp
+++ b/clang/lib/CodeGen/CGLoopInfo.cpp
@@ -673,8 +673,6 @@ 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/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index d3e76f282974e9..1565b31271f961 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -109,20 +109,11 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const
ParsedAttr &A,
SetHints(LoopHintAttr::Unroll, LoopHintAttr::Disable);
} else if (PragmaName == "unroll") {
// #pragma unroll N
- if (ValueExpr) {
- if (!ValueExpr->isValueDependent()) {
- 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");
- (void)R;
- // 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
+ if (ValueExpr && !ValueExpr->isValueDependent()) {
+ auto Value = ValueExpr->EvaluateKnownConstInt(S.getASTContext());
+ if (Value.isZero() || Value.isOne())
+ SetHints(LoopHintAttr::Unroll, LoopHintAttr::Disable);
+ else
SetHints(LoopHintAttr::UnrollCount, LoopHintAttr::Numeric);
} else
SetHints(LoopHintAttr::Unroll, LoopHintAttr::Enable);
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 98d5c7cb3a8a80..971cc2b3722bda 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2151,13 +2151,25 @@ TemplateInstantiator::TransformLoopHintAttr(const
LoopHintAttr *LH) {
// Generate error if there is a problem with the value.
if (getSema().CheckLoopHintExpr(TransformedExpr, LH->getLocation(),
- LH->getOption() ==
LoopHintAttr::UnrollCount))
+ LH->getSemanticSpelling() ==
+ LoopHintAttr::Pragma_unroll))
return LH;
+ LoopHintAttr::OptionType Option = LH->getOption();
+ LoopHintAttr::LoopHintState State = LH->getState();
+
+ llvm::APSInt ValueAPS =
+ TransformedExpr->EvaluateKnownConstInt(getSema().getASTContext());
+ // The values of 0 and 1 block any unrolling of the loop.
+ if (ValueAPS.isZero() || ValueAPS.isOne()) {
+ Option = LoopHintAttr::Unroll;
+ State = LoopHintAttr::Disable;
+ }
+
// Create new LoopHintValueAttr with integral expression in place of the
// non-type template parameter.
- return LoopHintAttr::CreateImplicit(getSema().Context, LH->getOption(),
- LH->getState(), TransformedExpr, *LH);
+ return LoopHintAttr::CreateImplicit(getSema().Context, Option,
+ State, TransformedExpr, *LH);
}
const NoInlineAttr *TemplateInstantiator::TransformStmtNoInlineAttr(
const Stmt *OrigS, const Stmt *InstS, const NoInlineAttr *A) {
diff --git a/clang/test/AST/ast-dump-pragma-unroll.cpp
b/clang/test/AST/ast-dump-pragma-unroll.cpp
new file mode 100644
index 00000000000000..6efe3072477e86
--- /dev/null
+++ b/clang/test/AST/ast-dump-pragma-unroll.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck %s
+
+using size_t = unsigned long long;
+
+template <bool Flag>
+int value_dependent(int n) {
+ constexpr int N = 100;
+ auto init = [=]() { return Flag ? n : 0UL; };
+ auto cond = [=](size_t ix) { return Flag ? ix != 0 : ix < 10; };
+ auto iter = [=](size_t ix) {
+ return Flag ? ix & ~(1ULL << __builtin_clzll(ix)) : ix + 1;
+ };
+ // CHECK: LoopHintAttr {{.*}} Implicit unroll Unroll Disable
+#pragma unroll Flag ? 1 : N
+ for (size_t ix = init(); cond(ix); ix = iter(ix)) {
+ n *= n;
+ }
+ // CHECK: LoopHintAttr {{.*}} Implicit unroll Unroll Disable
+#pragma unroll Flag ? 0 : N
+ for (size_t ix = init(); cond(ix); ix = iter(ix)) {
+ n *= n;
+ }
+ return n;
+}
+
+void test_value_dependent(int n) {
+ value_dependent<true>(n);
+}
diff --git a/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp
b/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp
index 8a94a5cc91e239..85f10fcdff14dd 100644
--- a/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp
+++ b/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp
@@ -116,6 +116,34 @@ void while_unroll_zero_test(int *List, int Length) {
}
}
+using size_t = unsigned long long;
+
+template <bool Flag>
+int value_dependent(int n) {
+ // CHECK: define {{.*}} @_Z15value_dependentILb1EEii
+ constexpr int N = 100;
+ auto init = [=]() { return Flag ? n : 0UL; };
+ auto cond = [=](size_t ix) { return Flag ? ix != 0 : ix < 10; };
+ auto iter = [=](size_t ix) {
+ return Flag ? ix & ~(1ULL << __builtin_clzll(ix)) : ix + 1;
+ };
+#pragma GCC unroll Flag ? 1 : N
+ for (size_t ix = init(); cond(ix); ix = iter(ix)) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_16:.*]]
+ n *= n;
+ }
+#pragma GCC unroll Flag ? 0 : N
+ for (size_t ix = init(); cond(ix); ix = iter(ix)) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_17:.*]]
+ n *= n;
+ }
+ return n;
+}
+
+void test_value_dependent(int n) {
+ value_dependent<true>(n);
+}
+
// 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:.*]]}
@@ -129,3 +157,5 @@ void while_unroll_zero_test(int *List, int Length) {
// 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:.*]]}
+// CHECK: ![[LOOP_16]] = distinct !{![[LOOP_16]], [[MP]],
![[UNROLL_DISABLE:.*]]}
+// CHECK: ![[LOOP_17]] = distinct !{![[LOOP_17]], [[MP]],
![[UNROLL_DISABLE:.*]]}
diff --git a/clang/test/CodeGenCXX/pragma-unroll.cpp
b/clang/test/CodeGenCXX/pragma-unroll.cpp
index 02d9bad7148d36..6754788b72436f 100644
--- a/clang/test/CodeGenCXX/pragma-unroll.cpp
+++ b/clang/test/CodeGenCXX/pragma-unroll.cpp
@@ -96,6 +96,54 @@ 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 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 unroll(0)
+ while (i < Length) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_15:.*]]
+ List[i] = i * 2;
+ i++;
+ }
+}
+
+using size_t = unsigned long long;
+
+template <bool Flag>
+int value_dependent(int n) {
+ // CHECK: define {{.*}} @_Z15value_dependentILb1EEii
+ constexpr int N = 100;
+ auto init = [=]() { return Flag ? n : 0UL; };
+ auto cond = [=](size_t ix) { return Flag ? ix != 0 : ix < 10; };
+ auto iter = [=](size_t ix) {
+ return Flag ? ix & ~(1ULL << __builtin_clzll(ix)) : ix + 1;
+ };
+#pragma unroll Flag ? 1 : N
+ for (size_t ix = init(); cond(ix); ix = iter(ix)) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_16:.*]]
+ n *= n;
+ }
+#pragma unroll Flag ? 0 : N
+ for (size_t ix = init(); cond(ix); ix = iter(ix)) {
+ // CHECK: br label {{.*}}, !llvm.loop ![[LOOP_17:.*]]
+ n *= n;
+ }
+ return n;
+}
+
+void test_value_dependent(int n) {
+ value_dependent<true>(n);
+}
+
// 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 +155,7 @@ 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:.*]]}
+// CHECK: ![[LOOP_16]] = distinct !{![[LOOP_16]], [[MP]],
![[UNROLL_DISABLE:.*]]}
+// CHECK: ![[LOOP_17]] = distinct !{![[LOOP_17]], [[MP]],
![[UNROLL_DISABLE:.*]]}
diff --git a/clang/test/Parser/pragma-unroll.cpp
b/clang/test/Parser/pragma-unroll.cpp
index e84f6ea9ad1035..19066acddcef10 100644
--- a/clang/test/Parser/pragma-unroll.cpp
+++ b/clang/test/Parser/pragma-unroll.cpp
@@ -135,19 +135,11 @@ int FailToBuild(int n) {
auto iter = [=](size_t ix) {
return Flag ? ix & ~(1ULL << __builtin_clzll(ix)) : ix + 1;
};
-#pragma unroll Flag ? 1 : N
+#pragma unroll Flag ? 0 : N // Ok, allow 0.
for (size_t ix = init(); cond(ix); ix = iter(ix)) {
n *= n;
}
-#pragma unroll Flag ? 0 : N
- for (size_t ix = init(); cond(ix); ix = iter(ix)) {
- n *= n;
- }
-#pragma GCC unroll Flag ? 1 : N
- for (size_t ix = init(); cond(ix); ix = iter(ix)) {
- n *= n;
- }
-#pragma GCC unroll Flag ? 0 : N
+#pragma GCC unroll Flag ? 0 : N // Ok, allow 0.
for (size_t ix = init(); cond(ix); ix = iter(ix)) {
n *= n;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
