[clang] [clang] Support `#pragma clang loop pipeline(enable)` (PR #112501)
https://github.com/kasuga-fj created
https://github.com/llvm/llvm-project/pull/112501
Previously `#pragma clang loop pipeline` only accepted `disable`. This patch
adds `enable` as a valid argument for this pragma. This allows Software
Pipelining optimization to be applied to some loops instead of all loops.
This is clang part of the fix.
>From 04f0f22178272dbf2ebe8a74569245f97a2f644b Mon Sep 17 00:00:00 2001
From: Ryotaro Kasuga
Date: Thu, 10 Oct 2024 09:08:00 +
Subject: [PATCH] [clang] Support `#pragma clang loop pipeline(enable)`
Previously `#pragma clang loop pipeline` only accepted `disable`. This
patch adds `enable` as a valid argument for this pragma. This allows
Software Pipelining optimization to be applied to some loops instead of
all loops.
This is clang part of the fix.
---
clang/include/clang/Basic/Attr.td | 6 ++--
clang/include/clang/Basic/AttrDocs.td | 12 ++-
.../clang/Basic/DiagnosticParseKinds.td | 2 --
clang/lib/CodeGen/CGLoopInfo.cpp | 36 ---
clang/lib/CodeGen/CGLoopInfo.h| 11 +++---
clang/lib/Parse/ParsePragma.cpp | 34 --
clang/lib/Sema/SemaStmtAttr.cpp | 8 ++---
clang/test/CodeGenCXX/pragma-pipeline.cpp | 12 +++
clang/test/Parser/pragma-loop.cpp | 2 +-
clang/test/Parser/pragma-pipeline.cpp | 8 +++--
10 files changed, 82 insertions(+), 49 deletions(-)
diff --git a/clang/include/clang/Basic/Attr.td
b/clang/include/clang/Basic/Attr.td
index ec3d6e0079f630..3d5d5f6ca99f1e 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -4190,7 +4190,7 @@ def LoopHint : Attr {
/// unroll_and_jam: attempt to unroll and jam loop if State == Enable.
/// unroll_and_jam_count: unroll and jams loop 'Value' times.
/// distribute: attempt to distribute loop if State == Enable.
- /// pipeline: disable pipelining loop if State == Disable.
+ /// pipeline: enable pipelining loop if State == Enable.
/// pipeline_initiation_interval: create loop schedule with initiation
interval equal to 'Value'.
/// #pragma unroll directive
@@ -4210,7 +4210,7 @@ def LoopHint : Attr {
"vectorize_predicate"],
["Vectorize", "VectorizeWidth", "Interleave",
"InterleaveCount",
"Unroll", "UnrollCount", "UnrollAndJam",
"UnrollAndJamCount",
- "PipelineDisabled", "PipelineInitiationInterval",
"Distribute",
+ "Pipeline", "PipelineInitiationInterval",
"Distribute",
"VectorizePredicate"]>,
EnumArgument<"State", "LoopHintState", /*is_string=*/false,
["enable", "disable", "numeric", "fixed_width",
@@ -4230,7 +4230,7 @@ def LoopHint : Attr {
case UnrollCount: return "unroll_count";
case UnrollAndJam: return "unroll_and_jam";
case UnrollAndJamCount: return "unroll_and_jam_count";
-case PipelineDisabled: return "pipeline";
+case Pipeline: return "pipeline";
case PipelineInitiationInterval: return "pipeline_initiation_interval";
case Distribute: return "distribute";
case VectorizePredicate: return "vectorize_predicate";
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index b1512e22ee2dd4..e2591c7be7905a 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3968,8 +3968,18 @@ def PipelineHintDocs : Documentation {
placed immediately before a for, while, do-while, or a C++11 range-based for
loop.
+ Using ``#pragma clang loop pipeline(enable)`` applies the software pipelining
+ optimization if possible:
+
+ .. code-block:: c++
+
+ #pragma clang loop pipeline(enable)
+ for (...) {
+...
+ }
+
Using ``#pragma clang loop pipeline(disable)`` avoids the software pipelining
- optimization. The disable state can only be specified:
+ optimization:
.. code-block:: c++
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td
b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 78510e61a639fa..1bf82923aa26bd 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1676,8 +1676,6 @@ def err_pragma_fp_invalid_argument : Error<
def err_pragma_invalid_keyword : Error<
"invalid argument; expected 'enable'%select{|, 'full'}0%select{|,
'assume_safety'}1 or 'disable'">;
-def err_pragma_pipeline_invalid_keyword : Error<
-"invalid argument; expected 'disable'">;
// API notes.
def err_type_unparsed : Error<"unparsed tokens following type">;
diff --git a/clang/lib/CodeGen/CGLoopInfo.cpp b/clang/lib/CodeGen/CGLoopInfo.cpp
index 6b886bd6b6d2cf..04b229da2013e3 100644
--- a/clang/lib/CodeGen/CGLoopInfo.cpp
+++ b/clang/lib/CodeGen/CGLoopInfo.cpp
@@ -39,9 +39,10 @@ MDNode *LoopInfo::createPipeliningM
[clang] [clang] Support `#pragma clang loop pipeline(enable)` (PR #112501)
https://github.com/kasuga-fj edited https://github.com/llvm/llvm-project/pull/112501 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][Driver] Add an option to control loop-interchange (PR #125830)
https://github.com/kasuga-fj approved this pull request. https://github.com/llvm/llvm-project/pull/125830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [Clang][Driver] Add an option to control loop-interchange (PR #125830)
kasuga-fj wrote: Thanks for the comments. I'll try to work on it in the near future. https://github.com/llvm/llvm-project/pull/125830 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][CodeGen] Make pragma-loop test more rboust (NFC) (PR #133707)
https://github.com/kasuga-fj created
https://github.com/llvm/llvm-project/pull/133707
pragma-loop.cpp contains tests for loop metadata generated via pragma
directives. These tests were not working as (perhaps) expected. This is because
the regex `.*` consumes multiple elements in metadata. For example, there was a
check directive as follows.
```
// CHECK: ![[LOOP_9]] = distinct !{![[LOOP_9]], ![[WIDTH_8:.*]],
![[FIXED_VEC]], ...}
```
In the above case, `[[WIDTH_8]]` would have been expected to match a node like
`[[WIDTH_8]] = !{!"llvm.loop.vectorize.width", i32 8}`. However, since there is
no check directive to verify the contents of `[[WIDTH_8]]`, the regex `.*`
consumed more than one element. There were other similar cases.
This patch fixes this problem by not using regex matcher in the metadata
contents (except for follow-up metadata). Instead, it uses string variables
whose contents are validated elsewhere.
Related:
https://github.com/llvm/llvm-project/pull/131985#discussion_r2014369699
>From b3bd68366dd71f11d0629c16584b1b9d4057d3f7 Mon Sep 17 00:00:00 2001
From: Ryotaro Kasuga
Date: Mon, 31 Mar 2025 11:36:35 +
Subject: [PATCH] [clang][CodeGen] Make pragma-loop test more rboust (NFC)
pragma-loop.cpp contains tests for loop metadata generated via pragma
directives. These tests were not working as (perhaps) expected. This is
because the regex `.*` consumes multiple elements in metadata. For
example,
```
![[LOOP_9]] = distinct !{![[LOOP_9]], ![[WIDTH_8:.*]], ![[FIXED_VEC]], ...}
```
`[[WIDTH_8]]` would have been expected to match a node like
`[[WIDTH_8]] = !{!"llvm.loop.vectorize.width", i32 8}`. However, since
there is no check directive to verify the contents of `[[WIDTH_8]]`,
`[[WIDTH_8:.*]]` consumed more than one element. There were other
similar cases.
This patch fixes this problem by not using regex matcher in the metadata
contents. Instead, it uses string variables whose contents are validated
elsewhere.
Related:
https://github.com/llvm/llvm-project/pull/131985#discussion_r2014369699
---
clang/test/CodeGenCXX/pragma-loop.cpp | 92 +++
1 file changed, 51 insertions(+), 41 deletions(-)
diff --git a/clang/test/CodeGenCXX/pragma-loop.cpp
b/clang/test/CodeGenCXX/pragma-loop.cpp
index 76bdcc4a5a9c9..4857299f1c037 100644
--- a/clang/test/CodeGenCXX/pragma-loop.cpp
+++ b/clang/test/CodeGenCXX/pragma-loop.cpp
@@ -203,60 +203,70 @@ void for_test_scalable_1(int *List, int Length) {
}
}
-// CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], [[MP:![0-9]+]],
![[UNROLL_FULL:.*]]}
-// CHECK: ![[UNROLL_FULL]] = !{!"llvm.loop.unroll.full"}
+// CHECK-DAG: ![[MP:[0-9]+]] = !{!"llvm.loop.mustprogress"}
-// CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2]], [[MP]],
![[UNROLL_DISABLE:.*]], ![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_8:.*]],
![[FIXED_VEC:.*]], ![[INTERLEAVE_4:.*]], ![[VECTORIZE_ENABLE:.*]]}
-// CHECK: ![[UNROLL_DISABLE]] = !{!"llvm.loop.unroll.disable"}
-// CHECK: ![[DISTRIBUTE_DISABLE]] = !{!"llvm.loop.distribute.enable", i1 false}
-// CHECK: ![[WIDTH_8]] = !{!"llvm.loop.vectorize.width", i32 8}
-// CHECK: ![[FIXED_VEC]] = !{!"llvm.loop.vectorize.scalable.enable", i1 false}
-// CHECK: ![[INTERLEAVE_4]] = !{!"llvm.loop.interleave.count", i32 4}
-// CHECK: ![[VECTORIZE_ENABLE]] = !{!"llvm.loop.vectorize.enable", i1 true}
+// CHECK-DAG: ![[UNROLL_DISABLE:[0-9]+]] = !{!"llvm.loop.unroll.disable"}
+// CHECK-DAG: ![[UNROLL_8:[0-9]+]] = !{!"llvm.loop.unroll.count", i32 8}
+// CHECK-DAG: ![[UNROLL_24:[0-9]+]] = !{!"llvm.loop.unroll.count", i32 24}
+// CHECK-DAG: ![[UNROLL_32:[0-9]+]] = !{!"llvm.loop.unroll.count", i32 32}
+// CHECK-DAG: ![[UNROLL_FULL:[0-9]+]] = !{!"llvm.loop.unroll.full"}
-// CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], [[MP]], ![[INTERLEAVE_4:.*]],
![[VECTORIZE_ENABLE]], ![[FOLLOWUP_VECTOR_3:.*]]}
-// CHECK: ![[FOLLOWUP_VECTOR_3]] = !{!"llvm.loop.vectorize.followup_all",
[[MP]], ![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
-// CHECK: ![[ISVECTORIZED]] = !{!"llvm.loop.isvectorized"}
-// CHECK: ![[UNROLL_8]] = !{!"llvm.loop.unroll.count", i32 8}
+// CHECK-DAG: ![[DISTRIBUTE_DISABLE:[0-9]+]] =
!{!"llvm.loop.distribute.enable", i1 false}
-// CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[WIDTH_2:.*]],
![[FIXED_VEC]], ![[INTERLEAVE_2:.*]], ![[VECTORIZE_ENABLE]]}
-// CHECK: ![[WIDTH_2]] = !{!"llvm.loop.vectorize.width", i32 2}
-// CHECK: ![[INTERLEAVE_2]] = !{!"llvm.loop.interleave.count", i32 2}
+// CHECK-DAG: ![[INTERLEAVE_2:[0-9]+]] = !{!"llvm.loop.interleave.count", i32
2}
+// CHECK-DAG: ![[INTERLEAVE_4:[0-9]+]] = !{!"llvm.loop.interleave.count", i32
4}
+// CHECK-DAG: ![[INTERLEAVE_8:[0-9]+]] = !{!"llvm.loop.interleave.count", i32
8}
+// CHECK-DAG: ![[INTERLEAVE_10:[0-9]+]] = !{!"llvm.loop.interleave.count", i32
10}
+// CHECK-DAG: ![[INTERLEAVE_16:[0-9]+]] = !{!"llvm.loop.interleave.count", i32
16}
-// CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNROLL_DISABLE:.*]],
![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_1:.*]]}
-// CHECK: ![[WIDTH_1]]
[clang] [clang][CodeGen] Make pragma-loop test more rboust (NFC) (PR #133707)
https://github.com/kasuga-fj edited https://github.com/llvm/llvm-project/pull/133707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
@@ -44,23 +44,19 @@ MDNode *LoopInfo::createPipeliningMetadata(const
LoopAttributes &Attrs,
else if (Attrs.PipelineInitiationInterval != 0)
Enabled = true;
+ SmallVector Args;
+ Args.append(LoopProperties.begin(), LoopProperties.end());
+
if (Enabled != true) {
-SmallVector NewLoopProperties;
if (Enabled == false) {
- NewLoopProperties.append(LoopProperties.begin(), LoopProperties.end());
kasuga-fj wrote:
> I think this is due to the regex .* being too greedy, so e.g.
> ![[ISVECTORIZED:.*]] consumes multiple metadata nodes.
That makes sense! I got it, thank you!
> Another one is that by default `CHECK: pet store` will match `carpet store`.
Ugh, that's a tricky problem.
https://github.com/llvm/llvm-project/pull/131985
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
@@ -44,23 +44,19 @@ MDNode *LoopInfo::createPipeliningMetadata(const
LoopAttributes &Attrs,
else if (Attrs.PipelineInitiationInterval != 0)
Enabled = true;
+ SmallVector Args;
+ Args.append(LoopProperties.begin(), LoopProperties.end());
+
if (Enabled != true) {
-SmallVector NewLoopProperties;
if (Enabled == false) {
- NewLoopProperties.append(LoopProperties.begin(), LoopProperties.end());
- NewLoopProperties.push_back(
+ Args.push_back(
MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.pipeline.disable"),
ConstantAsMetadata::get(ConstantInt::get(
llvm::Type::getInt1Ty(Ctx), 1))}));
- LoopProperties = NewLoopProperties;
kasuga-fj wrote:
What about other transformations, e.g., vectorization?
https://github.com/llvm/llvm-project/blob/ce8febb0befe41694b9d83c14dcfb831a82489ff/clang/lib/CodeGen/CGLoopInfo.cpp#L213-L220
I just looked for it and found an issue that might be caused by this.
https://github.com/llvm/llvm-project/issues/75839
https://github.com/llvm/llvm-project/pull/131985
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
@@ -44,23 +44,19 @@ MDNode *LoopInfo::createPipeliningMetadata(const
LoopAttributes &Attrs,
else if (Attrs.PipelineInitiationInterval != 0)
Enabled = true;
+ SmallVector Args;
+ Args.append(LoopProperties.begin(), LoopProperties.end());
+
if (Enabled != true) {
-SmallVector NewLoopProperties;
if (Enabled == false) {
- NewLoopProperties.append(LoopProperties.begin(), LoopProperties.end());
kasuga-fj wrote:
When `Enabled == std::nullopt`, `LoopProperties` was used as is, not
`NewProperties`. So I think the cause is elsewhere. Anyway, it's enough to know
that `llvm.mustprogress` should be appended unconditionally, thanks.
https://github.com/llvm/llvm-project/pull/131985
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
https://github.com/kasuga-fj commented: Thanks for your review! https://github.com/llvm/llvm-project/pull/131985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
https://github.com/kasuga-fj closed https://github.com/llvm/llvm-project/pull/131985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
https://github.com/kasuga-fj edited https://github.com/llvm/llvm-project/pull/131985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
@@ -44,23 +44,19 @@ MDNode *LoopInfo::createPipeliningMetadata(const
LoopAttributes &Attrs,
else if (Attrs.PipelineInitiationInterval != 0)
Enabled = true;
+ SmallVector Args;
+ Args.append(LoopProperties.begin(), LoopProperties.end());
+
if (Enabled != true) {
-SmallVector NewLoopProperties;
if (Enabled == false) {
- NewLoopProperties.append(LoopProperties.begin(), LoopProperties.end());
kasuga-fj wrote:
> In principle, LoopVectorize should know that if the original loop had a
> progress guarantee, then the vectorized loop will as well, so it should add
> `llvm.loop.must_progress` no matter what.
I think this is completely correct. What I didn't understand is, why the
followup metadata of `LOOP_6` (`FOLLOW_VECTOR_6`) didn't have
`llvm.mustprogress` before this patch, but now it (`FOLLOWUP_VECTOR_3`) does.
I investigated a little deeper and found the cause; `FOLLOWUP_VECTOR_6`
actually had `mustprogress` (?!). That is, the test passed for both of the
following directives.
```
// Original.
// CHECK: ![[AFTER_VECTOR_6]] = distinct !{![[AFTER_VECTOR_6]],
![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
// This was also fine.
// CHECK: ![[AFTER_VECTOR_6]] = distinct !{![[AFTER_VECTOR_6]], [[MP]],
![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
```
Maybe FileCheck has a problem?
https://github.com/llvm/llvm-project/pull/131985
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
@@ -44,23 +44,19 @@ MDNode *LoopInfo::createPipeliningMetadata(const
LoopAttributes &Attrs,
else if (Attrs.PipelineInitiationInterval != 0)
Enabled = true;
+ SmallVector Args;
+ Args.append(LoopProperties.begin(), LoopProperties.end());
+
if (Enabled != true) {
-SmallVector NewLoopProperties;
if (Enabled == false) {
- NewLoopProperties.append(LoopProperties.begin(), LoopProperties.end());
kasuga-fj wrote:
The same thing seems to happen elsewhere, e.g. `LOOP_6` actually has
`vectorize.enable` but is not included in the CHECK directive.
https://github.com/llvm/llvm-project/pull/131985
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
@@ -227,36 +226,26 @@ void for_test_scalable_1(int *List, int Length) {
// CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNROLL_DISABLE:.*]],
![[DISTRIBUTE_DISABLE:.*]], ![[WIDTH_1:.*]]}
// CHECK: ![[WIDTH_1]] = !{!"llvm.loop.vectorize.width", i32 1}
-// CHECK: ![[LOOP_6]] = distinct !{![[LOOP_6]], [[MP]], ![[WIDTH_2:.*]],
![[FIXED_VEC]], ![[INTERLEAVE_2:.*]], ![[FOLLOWUP_VECTOR_6:.*]]}
-// CHECK: ![[FOLLOWUP_VECTOR_6]] = !{!"llvm.loop.vectorize.followup_all",
![[AFTER_VECTOR_6:.*]]}
-// CHECK: ![[AFTER_VECTOR_6]] = distinct !{![[AFTER_VECTOR_6]],
![[ISVECTORIZED:.*]], ![[UNROLL_8:.*]]}
+// CHECK: ![[LOOP_6]] = distinct !{![[LOOP_6]], [[MP]], ![[WIDTH_2:.*]],
![[FIXED_VEC]], ![[INTERLEAVE_2:.*]], ![[FOLLOWUP_VECTOR_3]]}
kasuga-fj wrote:
I don't fully understand why `[[MP]]` was not in `![[FOLLOWUP_VECTOR_6]]`
before this patch. Is it intentional?
https://github.com/llvm/llvm-project/pull/131985
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
@@ -170,11 +161,10 @@ LoopInfo::createUnrollAndJamMetadata(const LoopAttributes &Attrs, MDNode::get(Ctx, MDString::get(Ctx, "llvm.loop.unroll_and_jam.disable"))); bool FollowupHasTransforms = false; - MDNode *Followup = createPartialUnrollMetadata(Attrs, FollowupLoopProperties, - FollowupHasTransforms); + SmallVector Followup = createPartialUnrollMetadata( + Attrs, FollowupLoopProperties, FollowupHasTransforms); SmallVector Args; - Args.push_back(nullptr); kasuga-fj wrote: `nullptr` is used as a placeholder for LoopID, so this is no longer necessary, right? https://github.com/llvm/llvm-project/pull/131985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
@@ -44,23 +44,19 @@ MDNode *LoopInfo::createPipeliningMetadata(const
LoopAttributes &Attrs,
else if (Attrs.PipelineInitiationInterval != 0)
Enabled = true;
+ SmallVector Args;
+ Args.append(LoopProperties.begin(), LoopProperties.end());
+
if (Enabled != true) {
-SmallVector NewLoopProperties;
if (Enabled == false) {
- NewLoopProperties.append(LoopProperties.begin(), LoopProperties.end());
- NewLoopProperties.push_back(
+ Args.push_back(
MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.pipeline.disable"),
ConstantAsMetadata::get(ConstantInt::get(
llvm::Type::getInt1Ty(Ctx), 1))}));
- LoopProperties = NewLoopProperties;
kasuga-fj wrote:
Different from this PR, but I think we should set `HasUserTransforms` to true
here (same for other `create*Metadata`). If not, the user-specified `disable'
attributes would not be generated properly.
https://github.com/llvm/llvm-project/pull/131985
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
https://github.com/kasuga-fj commented: > Possibly the `create*Metadata` should only return a list of properties > instead of an MDNode, with the MDNode only to be created when applying the > properties to a Loop. However, the easier fix would probably be to extract > the properties from the MDNode and append to the `followup_*` MDNode, with > the returned MDNode becoming garbage-collected at some point. Fixed `create*Metadata` to return a list of properties instead of an MDNode. I believe unnecessary MDNodes would not be generated. https://github.com/llvm/llvm-project/pull/131985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
https://github.com/kasuga-fj edited https://github.com/llvm/llvm-project/pull/131985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [LoopUtils] Fix metadata generated by makeFollowupLoopID (PR #131985)
https://github.com/kasuga-fj updated
https://github.com/llvm/llvm-project/pull/131985
>From 889f40c5570af8a02e301c2bf3c6382f69210140 Mon Sep 17 00:00:00 2001
From: Ryotaro Kasuga
Date: Mon, 17 Mar 2025 11:24:47 +
Subject: [PATCH 1/3] [LoopUtils] Fix metadata generated by makeFollowupLoopID
When multiple pragma for loop transformations are specified, such as:
```c
for (...) {
}
```
The generated metadata would look like this:
```
!0 = distinct !{!0, !1, !2}
!1 = !{"llvm.loop.vectorize.enable", i1 true}
!2 = !{"llvm.loop.vectorize.followup_all", !3}
!3 = distinct !{!3, !4, !5}
!4 = !{"llvm.loop.isvectorized"}
!5 = !{"llvm.loop.unroll_count", i32, 8}
```
For a loop with `!0` as its LoopID, the new LoopID after vectorized
should be like as below, so that we can know that this loop is already
vectorized and should be unrolled with specified count:
```
!6 = distinct !{!6, !4, !5}
```
However, the current implementation creates new LoopID like:
```
!7 = distinct !{!7, !3}
```
Therefore subsequent passes like LoopUnroll fails to recognize the
attributes of this loop correctly.
This patch fixes `makeFollowupLoopID`, which creates a new LoopID after
each transformation. If the follow-up metadata (`!3` in the above case)
is a LoopID, the new LoopID will contain its operands (`!4` and `!5`)
instead of the metadata itself.
---
llvm/lib/Transforms/Utils/LoopUtils.cpp | 29 +
.../LoopVectorize/make-followup-loop-id.ll| 102 ++
2 files changed, 111 insertions(+), 20 deletions(-)
diff --git a/llvm/lib/Transforms/Utils/LoopUtils.cpp
b/llvm/lib/Transforms/Utils/LoopUtils.cpp
index 84c08556f8a25..4a6105add953f 100644
--- a/llvm/lib/Transforms/Utils/LoopUtils.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUtils.cpp
@@ -317,6 +317,35 @@ std::optional llvm::makeFollowupLoopID(
HasAnyFollowup = true;
for (const MDOperand &Option : drop_begin(FollowupNode->operands())) {
+ // The followup metadata typically forms as follows:
+ //
+ // !0 = distinct !{!0, !1, !2}
+ // !1 = !{!"llvm.loop.distribute.enable", i1 true}
+ // !2 = !{!"llvm.loop.distribute.followup_all", !3}
+ // !3 = distinct !{!3, !4}
+ // !4 = !{!"llvm.loop.vectorize.enable", i1 true}
+ //
+ // If we push Option (!3 in this case) in MDs, the new metadata looks
+ // something like:
+ //
+ // !5 = distinct !{!5, !3}
+ //
+ // This doesn't contain !4, so the vectorization pass doesn't recognize
+ // this loop as vectorization enabled. To make the new metadata contain
!4
+ // instead of !3, traverse all of Option's operands and push them into
+ // MDs if Option seems to be a LoopID.
+ if (auto *MDN = dyn_cast(Option)) {
+// TODO: Is there a proper way to detect LoopID?
+if (MDN->getNumOperands() > 1 && MDN->getOperand(0) == MDN) {
+ for (const MDOperand &NestedOption : drop_begin(MDN->operands())) {
+MDs.push_back(NestedOption.get());
+Changed = true;
+ }
+ continue;
+}
+ }
+
+ // If Option does't seem to be a LoopID, push it as it is.
MDs.push_back(Option.get());
Changed = true;
}
diff --git a/llvm/test/Transforms/LoopVectorize/make-followup-loop-id.ll
b/llvm/test/Transforms/LoopVectorize/make-followup-loop-id.ll
index fa5c206547a07..41f508e0a7641 100644
--- a/llvm/test/Transforms/LoopVectorize/make-followup-loop-id.ll
+++ b/llvm/test/Transforms/LoopVectorize/make-followup-loop-id.ll
@@ -11,10 +11,6 @@
; a[i] *= x;
; }
; }
-;
-; FIXME: Currently unrolling is not applied. This is because the new Loop ID
-; created after vectorization does not directly contain unroll metadata.
-; Unexpected nests have been created.
define void @f(ptr noundef captures(none) %a, float noundef %x) {
; CHECK-LABEL: define void @f(
; CHECK-SAME: ptr noundef captures(none) [[A:%.*]], float noundef [[X:%.*]]) {
@@ -25,14 +21,47 @@ define void @f(ptr noundef captures(none) %a, float noundef
%x) {
; CHECK-NEXT:[[BROADCAST_SPLAT:%.*]] = shufflevector <4 x float>
[[BROADCAST_SPLATINSERT]], <4 x float> poison, <4 x i32> zeroinitializer
; CHECK-NEXT:br label %[[VECTOR_BODY:.*]]
; CHECK: [[VECTOR_BODY]]:
-; CHECK-NEXT:[[INDEX:%.*]] = phi i64 [ 0, %[[VECTOR_PH]] ], [
[[INDEX_NEXT:%.*]], %[[VECTOR_BODY]] ]
-; CHECK-NEXT:[[INDEX_NEXT_6:%.*]] = add i64 [[INDEX]], 0
+; CHECK-NEXT:[[INDEX_NEXT_6:%.*]] = phi i64 [ 0, %[[VECTOR_PH]] ], [
[[INDEX_NEXT:%.*]], %[[VECTOR_BODY]] ]
; CHECK-NEXT:[[TMP14:%.*]] = getelementptr inbounds nuw float, ptr [[A]],
i64 [[INDEX_NEXT_6]]
-; CHECK-NEXT:[[TMP2:%.*]] = getelementptr inbounds nuw float, ptr
[[TMP14]], i32 0
-; CHECK-NEXT:[[WIDE_LOAD_7:%.*]] = load <4 x float>, ptr [[TMP2]], align 4
+; CHECK-NEXT:[[WIDE_LOAD_7:%.*]] = load <4 x float>, ptr [[TMP14]], align 4
; CHECK-NEXT:[[TMP15:%.*]] = fmul <4 x float> [[BROADCAST_SPLAT]],
[[WIDE_LOAD_7]]
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
https://github.com/kasuga-fj edited https://github.com/llvm/llvm-project/pull/131985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
https://github.com/kasuga-fj updated
https://github.com/llvm/llvm-project/pull/131985
>From 63dec28732e6a90f5f3449441fa8472b5b60a9f4 Mon Sep 17 00:00:00 2001
From: Ryotaro Kasuga
Date: Wed, 26 Mar 2025 07:29:18 +
Subject: [PATCH] [clang][CodeGen] Generate follow-up metadata for loops in
correct format
When pragma of loop transformations is specified, follow-up metadata for
loops is generated after each transformation. On the LLVM side,
follow-up metadata is expected to be a list of properties, such as the
following:
```
!followup = !{!"llvm.loop.vectorize.followup_all", !mp, !isvectorized}
!mp = !{!"llvm.loop.mustprogress"}
!isvectorized = !{"llvm.loop.isvectorized"}
```
However, on the clang side, the generated metadata contains an MDNode
that has those properties, as shown below:
```
!followup = !{!"llvm.loop.vectorize.followup_all", !loop_id}
!loop_id = distinct !{!loop_id, !mp, !isvectorized}
!mp = !{!"llvm.loop.mustprogress"}
!isvectorized = !{"llvm.loop.isvectorized"}
```
According to the LangRef, the LLVM side is correct. (ref:
https://llvm.org/docs/TransformMetadata.html#transformation-metadata-structure).
Due to this inconsistency, follow-up metadata was not interpreted
correctly, e.g., only one transformation is applied when multiple
pragmas are used.
This patch fixes clang side to emit followup metadata in correct format.
---
clang/lib/CodeGen/CGLoopInfo.cpp | 133 --
clang/lib/CodeGen/CGLoopInfo.h| 43 +++---
.../test/CodeGenCXX/pragma-followup_inner.cpp | 9 +-
.../test/CodeGenCXX/pragma-followup_outer.cpp | 12 +-
clang/test/CodeGenCXX/pragma-loop.cpp | 25 +---
.../LoopVectorize/make-followup-loop-id.ll| 109 +++---
6 files changed, 181 insertions(+), 150 deletions(-)
diff --git a/clang/lib/CodeGen/CGLoopInfo.cpp b/clang/lib/CodeGen/CGLoopInfo.cpp
index 448571221ef81..2b7d7881ab990 100644
--- a/clang/lib/CodeGen/CGLoopInfo.cpp
+++ b/clang/lib/CodeGen/CGLoopInfo.cpp
@@ -22,20 +22,20 @@ using namespace clang::CodeGen;
using namespace llvm;
MDNode *
-LoopInfo::createLoopPropertiesMetadata(ArrayRef LoopProperties) {
+LoopInfo::createFollowupMetadata(const char *FollowupName,
+ ArrayRef LoopProperties) {
LLVMContext &Ctx = Header->getContext();
- SmallVector NewLoopProperties;
- NewLoopProperties.push_back(nullptr);
- NewLoopProperties.append(LoopProperties.begin(), LoopProperties.end());
- MDNode *LoopID = MDNode::getDistinct(Ctx, NewLoopProperties);
- LoopID->replaceOperandWith(0, LoopID);
- return LoopID;
+ SmallVector Args;
+ Args.push_back(MDString::get(Ctx, FollowupName));
+ Args.append(LoopProperties.begin(), LoopProperties.end());
+ return MDNode::get(Ctx, Args);
}
-MDNode *LoopInfo::createPipeliningMetadata(const LoopAttributes &Attrs,
- ArrayRef LoopProperties,
- bool &HasUserTransforms) {
+SmallVector
+LoopInfo::createPipeliningMetadata(const LoopAttributes &Attrs,
+ ArrayRef LoopProperties,
+ bool &HasUserTransforms) {
LLVMContext &Ctx = Header->getContext();
std::optional Enabled;
@@ -44,23 +44,19 @@ MDNode *LoopInfo::createPipeliningMetadata(const
LoopAttributes &Attrs,
else if (Attrs.PipelineInitiationInterval != 0)
Enabled = true;
+ SmallVector Args;
+ Args.append(LoopProperties.begin(), LoopProperties.end());
+
if (Enabled != true) {
-SmallVector NewLoopProperties;
if (Enabled == false) {
- NewLoopProperties.append(LoopProperties.begin(), LoopProperties.end());
- NewLoopProperties.push_back(
+ Args.push_back(
MDNode::get(Ctx, {MDString::get(Ctx, "llvm.loop.pipeline.disable"),
ConstantAsMetadata::get(ConstantInt::get(
llvm::Type::getInt1Ty(Ctx), 1))}));
- LoopProperties = NewLoopProperties;
}
-return createLoopPropertiesMetadata(LoopProperties);
+return Args;
}
- SmallVector Args;
- Args.push_back(nullptr);
- Args.append(LoopProperties.begin(), LoopProperties.end());
-
if (Attrs.PipelineInitiationInterval > 0) {
Metadata *Vals[] = {
MDString::get(Ctx, "llvm.loop.pipeline.initiationinterval"),
@@ -71,13 +67,11 @@ MDNode *LoopInfo::createPipeliningMetadata(const
LoopAttributes &Attrs,
// No follow-up: This is the last transformation.
- MDNode *LoopID = MDNode::getDistinct(Ctx, Args);
- LoopID->replaceOperandWith(0, LoopID);
HasUserTransforms = true;
- return LoopID;
+ return Args;
}
-MDNode *
+SmallVector
LoopInfo::createPartialUnrollMetadata(const LoopAttributes &Attrs,
ArrayRef LoopProperties,
bool &HasUserTransforms) {
@@ -108,11 +102,10 @@ LoopInfo::createPartialUnrollMetadata(const
LoopAttributes &Attrs,
MDNode::get(Ctx, MDStr
[clang] [clang][CodeGen] Make pragma-loop test more rboust (NFC) (PR #133707)
https://github.com/kasuga-fj closed https://github.com/llvm/llvm-project/pull/133707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [clang][CodeGen] Generate follow-up metadata for loops in correct format (PR #131985)
https://github.com/kasuga-fj edited https://github.com/llvm/llvm-project/pull/131985 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
