https://github.com/brunodf-snps updated https://github.com/llvm/llvm-project/pull/172289
>From 0e2ed4b9b2ffe0763c0de82cb688636490e10cbe Mon Sep 17 00:00:00 2001 From: Bruno De Fraine <[email protected]> Date: Mon, 15 Dec 2025 11:43:13 +0100 Subject: [PATCH 1/4] [clang] Fix TemplateInstantiator crash transforming loop hint argument In cases where multiple template instations occur (e.g. with a generic lambda), the loop hint argument expression may still be value dependent. (It is also not needed to do the constant evaluation when the loop hint is not an unroll count anyway.) --- clang/lib/Sema/SemaTemplateInstantiate.cpp | 18 +++++++------ .../test/Sema/unroll-template-value-crash.cpp | 25 +++++++++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 35205f40cbcef..dcbedb2c2ee22 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -2197,19 +2197,23 @@ TemplateInstantiator::TransformLoopHintAttr(const LoopHintAttr *LH) { // Generate error if there is a problem with the value. if (getSema().CheckLoopHintExpr(TransformedExpr, LH->getLocation(), - LH->getSemanticSpelling() == + /*AllowZero=*/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; + if (Option == LoopHintAttr::UnrollCount && + !TransformedExpr->isValueDependent()) { + llvm::APSInt ValueAPS = + TransformedExpr->EvaluateKnownConstInt(getSema().getASTContext()); + // The values of 0 and 1 block any unrolling of the loop (also see + // handleLoopHintAttr in SemaStmtAttr). + if (ValueAPS.isZero() || ValueAPS.isOne()) { + Option = LoopHintAttr::Unroll; + State = LoopHintAttr::Disable; + } } // Create new LoopHintValueAttr with integral expression in place of the diff --git a/clang/test/Sema/unroll-template-value-crash.cpp b/clang/test/Sema/unroll-template-value-crash.cpp index d8953c4845c26..cda4b897509b9 100644 --- a/clang/test/Sema/unroll-template-value-crash.cpp +++ b/clang/test/Sema/unroll-template-value-crash.cpp @@ -8,3 +8,28 @@ template <int Unroll> void foo() { #pragma GCC unroll Unroll for (int i = 0; i < Unroll; ++i); } + +struct val { + constexpr operator int() const { return 1; } +}; + +// generic lambda (using double template instantiation) + +template<typename T> +void use(T t) { + auto lam = [](auto N) { + #pragma clang loop unroll_count(N+1) + for (int i = 0; i < 10; ++i); + + #pragma unroll N+1 + for (int i = 0; i < 10; ++i); + + #pragma GCC unroll N+1 + for (int i = 0; i < 10; ++i); + }; + lam(t); +} + +void test() { + use(val()); +} >From e7efc38d5ac57b73953bc0b2e955554d6397c46c Mon Sep 17 00:00:00 2001 From: Bruno De Fraine <[email protected]> Date: Fri, 2 Jan 2026 13:55:09 +0100 Subject: [PATCH 2/4] Add code comment describing why we protect against value dependent expr --- clang/lib/Sema/SemaTemplateInstantiate.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index dcbedb2c2ee22..27d73a8555202 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -2204,6 +2204,11 @@ TemplateInstantiator::TransformLoopHintAttr(const LoopHintAttr *LH) { LoopHintAttr::OptionType Option = LH->getOption(); LoopHintAttr::LoopHintState State = LH->getState(); + // Since C++ does not have partial instantiation, we would expect a + // transformed loop hint expression to not be value dependent. However, at + // the time of writing, the use of a generic lambda inside a template + // triggers a double instantiation, so we must protect against this event. + // This provision may become unneeded in the future. if (Option == LoopHintAttr::UnrollCount && !TransformedExpr->isValueDependent()) { llvm::APSInt ValueAPS = >From 7fd8afdbcca87ce35e7ada286aa9cc4fa0f411be Mon Sep 17 00:00:00 2001 From: Bruno De Fraine <[email protected]> Date: Wed, 7 Jan 2026 12:14:11 +0100 Subject: [PATCH 3/4] [clang][CodeGenCXX] Fix pragma unroll test patterns (NFC) These tests were not testing the loop metadata that they suggest to be testing. They would define FileCheck variables such as UNROLL_8 for metadata nodes and then later redefine instead of use them (redefinition always succeeds and thus checks nothing). The error was likely introduced because the earlier loop metadata nodes, which must define the variables at the first occurrence, were copy-pasted for later loop metadata nodes, without realizing that the variable definitions in the FileCheck pattern must be changed to uses to match later occurrences of the same metadata node. By matching the metadata section with a block of DAG patterns, we can define variables such as UNROLL_8 at the metadata node itself, even if it occurs later, and the loop metadata nodes can be matched with uniform patterns. This system is also used in the pragma-loop.cpp test. --- clang/test/CodeGenCXX/pragma-gcc-unroll.cpp | 33 +++++++++++---------- clang/test/CodeGenCXX/pragma-unroll.cpp | 33 +++++++++++---------- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp b/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp index 85f10fcdff14d..0c7fc527eca65 100644 --- a/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp +++ b/clang/test/CodeGenCXX/pragma-gcc-unroll.cpp @@ -144,18 +144,21 @@ 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:.*]]} -// CHECK: ![[UNROLL_DISABLE]] = !{!"llvm.loop.unroll.disable"} -// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], [[MP]], ![[UNROLL_8:.*]]} -// CHECK: ![[UNROLL_8]] = !{!"llvm.loop.unroll.count", i32 8} -// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[UNROLL_4:.*]]} -// CHECK: ![[UNROLL_4]] = !{!"llvm.loop.unroll.count", i32 4} -// 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:.*]]} +// CHECK-DAG: ![[MP:[0-9]+]] = !{!"llvm.loop.mustprogress"} +// +// CHECK-DAG: ![[UNROLL_ENABLE:[0-9]+]] = !{!"llvm.loop.unroll.enable"} +// CHECK-DAG: ![[UNROLL_DISABLE:[0-9]+]] = !{!"llvm.loop.unroll.disable"} +// CHECK-DAG: ![[UNROLL_4:[0-9]+]] = !{!"llvm.loop.unroll.count", i32 4} +// CHECK-DAG: ![[UNROLL_8:[0-9]+]] = !{!"llvm.loop.unroll.count", i32 8} +// +// CHECK-DAG: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[MP]], ![[UNROLL_ENABLE]]} +// CHECK-DAG: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[MP]], ![[UNROLL_DISABLE]]} +// CHECK-DAG: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[MP]], ![[UNROLL_8]]} +// CHECK-DAG: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[UNROLL_4]]} +// CHECK-DAG: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[MP]], ![[UNROLL_8]]} +// CHECK-DAG: ![[LOOP_6]] = distinct !{![[LOOP_6]], ![[MP]], ![[UNROLL_8]]} +// CHECK-DAG: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[MP]], ![[UNROLL_8]]} +// CHECK-DAG: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[MP]], ![[UNROLL_DISABLE]]} +// CHECK-DAG: ![[LOOP_15]] = distinct !{![[LOOP_15]], ![[MP]], ![[UNROLL_DISABLE]]} +// CHECK-DAG: ![[LOOP_16]] = distinct !{![[LOOP_16]], ![[MP]], ![[UNROLL_DISABLE]]} +// CHECK-DAG: ![[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 6754788b72436..75f0c54ed9131 100644 --- a/clang/test/CodeGenCXX/pragma-unroll.cpp +++ b/clang/test/CodeGenCXX/pragma-unroll.cpp @@ -144,18 +144,21 @@ 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:.*]]} -// CHECK: ![[UNROLL_DISABLE]] = !{!"llvm.loop.unroll.disable"} -// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], [[MP]], ![[UNROLL_8:.*]]} -// CHECK: ![[UNROLL_8]] = !{!"llvm.loop.unroll.count", i32 8} -// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[UNROLL_4:.*]]} -// CHECK: ![[UNROLL_4]] = !{!"llvm.loop.unroll.count", i32 4} -// 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:.*]]} +// CHECK-DAG: ![[MP:[0-9]+]] = !{!"llvm.loop.mustprogress"} +// +// CHECK-DAG: ![[UNROLL_ENABLE:[0-9]+]] = !{!"llvm.loop.unroll.enable"} +// CHECK-DAG: ![[UNROLL_DISABLE:[0-9]+]] = !{!"llvm.loop.unroll.disable"} +// CHECK-DAG: ![[UNROLL_4:[0-9]+]] = !{!"llvm.loop.unroll.count", i32 4} +// CHECK-DAG: ![[UNROLL_8:[0-9]+]] = !{!"llvm.loop.unroll.count", i32 8} +// +// CHECK-DAG: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[MP]], ![[UNROLL_ENABLE]]} +// CHECK-DAG: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[MP]], ![[UNROLL_DISABLE]]} +// CHECK-DAG: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[MP]], ![[UNROLL_8]]} +// CHECK-DAG: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[UNROLL_4]]} +// CHECK-DAG: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[MP]], ![[UNROLL_8]]} +// CHECK-DAG: ![[LOOP_6]] = distinct !{![[LOOP_6]], ![[MP]], ![[UNROLL_8]]} +// CHECK-DAG: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[MP]], ![[UNROLL_8]]} +// CHECK-DAG: ![[LOOP_14]] = distinct !{![[LOOP_14]], ![[MP]], ![[UNROLL_DISABLE]]} +// CHECK-DAG: ![[LOOP_15]] = distinct !{![[LOOP_15]], ![[MP]], ![[UNROLL_DISABLE]]} +// CHECK-DAG: ![[LOOP_16]] = distinct !{![[LOOP_16]], ![[MP]], ![[UNROLL_DISABLE]]} +// CHECK-DAG: ![[LOOP_17]] = distinct !{![[LOOP_17]], ![[MP]], ![[UNROLL_DISABLE]]} >From 3868ec2e27797f0e638181f5600cddb1a4724817 Mon Sep 17 00:00:00 2001 From: Bruno De Fraine <[email protected]> Date: Wed, 7 Jan 2026 22:30:43 +0100 Subject: [PATCH 4/4] Add test to verify that special treatment of #pragma unroll 0|1 does not affect other loop pragmas --- clang/test/CodeGenCXX/pragma-unroll.cpp | 32 ++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/clang/test/CodeGenCXX/pragma-unroll.cpp b/clang/test/CodeGenCXX/pragma-unroll.cpp index 75f0c54ed9131..c89b58907701e 100644 --- a/clang/test/CodeGenCXX/pragma-unroll.cpp +++ b/clang/test/CodeGenCXX/pragma-unroll.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,OTHER %s // Check that passing -fno-unroll-loops does not impact the decision made using pragmas. // RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - -O1 -disable-llvm-optzns -fno-unroll-loops %s | FileCheck %s @@ -144,6 +144,31 @@ void test_value_dependent(int n) { value_dependent<true>(n); } +// Verify that the special treatment of #pragma unroll 0|1 does not affect +// other loop pragmas with an integer argument, such as vectorize_width or +// interleave_count. + +void for_vectorize_one_test(int *List, int Length) { + // OTHER: define {{.*}} @_Z22for_vectorize_one_testPii + #pragma clang loop vectorize_width(1) + for (int i = 0; i < Length; i++) { + // OTHER: br label {{.*}}, !llvm.loop ![[LOOP_18:.*]] + List[i] = i * 2; + } +} + +template <int V> +void for_vectorize_value_dependent(int *List, int Length) { + // OTHER: define {{.*}} @_Z29for_vectorize_value_dependentILi1EEvPii + #pragma clang loop vectorize_width(V) + for (int i = 0; i < Length; i++) { + // OTHER: br label {{.*}}, !llvm.loop ![[LOOP_19:.*]] + List[i] = i * 2; + } +} + +template void for_vectorize_value_dependent<1>(int *List, int Length); + // CHECK-DAG: ![[MP:[0-9]+]] = !{!"llvm.loop.mustprogress"} // // CHECK-DAG: ![[UNROLL_ENABLE:[0-9]+]] = !{!"llvm.loop.unroll.enable"} @@ -151,6 +176,9 @@ void test_value_dependent(int n) { // CHECK-DAG: ![[UNROLL_4:[0-9]+]] = !{!"llvm.loop.unroll.count", i32 4} // CHECK-DAG: ![[UNROLL_8:[0-9]+]] = !{!"llvm.loop.unroll.count", i32 8} // +// OTHER-DAG: ![[VEC_1:[0-9]+]] = !{!"llvm.loop.vectorize.width", i32 1} +// OTHER-DAG: ![[VEC_FIXED:[0-9]+]] = !{!"llvm.loop.vectorize.scalable.enable", i1 false} +// // CHECK-DAG: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[MP]], ![[UNROLL_ENABLE]]} // CHECK-DAG: ![[LOOP_2]] = distinct !{![[LOOP_2]], ![[MP]], ![[UNROLL_DISABLE]]} // CHECK-DAG: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[MP]], ![[UNROLL_8]]} @@ -162,3 +190,5 @@ void test_value_dependent(int n) { // CHECK-DAG: ![[LOOP_15]] = distinct !{![[LOOP_15]], ![[MP]], ![[UNROLL_DISABLE]]} // CHECK-DAG: ![[LOOP_16]] = distinct !{![[LOOP_16]], ![[MP]], ![[UNROLL_DISABLE]]} // CHECK-DAG: ![[LOOP_17]] = distinct !{![[LOOP_17]], ![[MP]], ![[UNROLL_DISABLE]]} +// OTHER-DAG: ![[LOOP_18]] = distinct !{![[LOOP_18]], ![[MP]], ![[VEC_1]], ![[VEC_FIXED]]} +// OTHER-DAG: ![[LOOP_19]] = distinct !{![[LOOP_19]], ![[MP]], ![[VEC_1]], ![[VEC_FIXED]]} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
