[clang][CodeGen] Generate follow-up metadata for loops in correct format (llvm#131985)

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](https://llvm.org/docs/TransformMetadata.html#transformation-metadata-structure),
the LLVM side is correct. 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.

Author: kasuga-fj

clang/lib/CodeGen/CGLoopInfo.cpp

@@ -22,20 +22,20 @@ using namespace clang::CodeGen;
 using namespace llvm;
 
 MDNode *
-LoopInfo::createLoopPropertiesMetadata(ArrayRef<Metadata *> LoopProperties) {
+LoopInfo::createFollowupMetadata(const char *FollowupName,
+                                 ArrayRef<llvm::Metadata *> LoopProperties) {
   LLVMContext &Ctx = Header->getContext();
-  SmallVector<Metadata *, 4> NewLoopProperties;
-  NewLoopProperties.push_back(nullptr);
-  NewLoopProperties.append(LoopProperties.begin(), LoopProperties.end());
 
-  MDNode *LoopID = MDNode::getDistinct(Ctx, NewLoopProperties);
-  LoopID->replaceOperandWith(0, LoopID);
-  return LoopID;
+  SmallVector<Metadata *, 4> 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<Metadata *> LoopProperties,
-                                           bool &HasUserTransforms) {
+SmallVector<Metadata *, 4>
+LoopInfo::createPipeliningMetadata(const LoopAttributes &Attrs,
+                                   ArrayRef<Metadata *> LoopProperties,
+                                   bool &HasUserTransforms) {
   LLVMContext &Ctx = Header->getContext();
 
   std::optional<bool> Enabled;
@@ -44,23 +44,19 @@ MDNode *LoopInfo::createPipeliningMetadata(const LoopAttributes &Attrs,
   else if (Attrs.PipelineInitiationInterval != 0)
     Enabled = true;
 
+  SmallVector<Metadata *, 4> Args;
+  Args.append(LoopProperties.begin(), LoopProperties.end());
+
   if (Enabled != true) {
-    SmallVector<Metadata *, 4> 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<Metadata *, 4> 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<Metadata *, 4>
 LoopInfo::createPartialUnrollMetadata(const LoopAttributes &Attrs,
                                       ArrayRef<Metadata *> LoopProperties,
                                       bool &HasUserTransforms) {
@@ -108,11 +102,10 @@ LoopInfo::createPartialUnrollMetadata(const LoopAttributes &Attrs,
       MDNode::get(Ctx, MDString::get(Ctx, "llvm.loop.unroll.disable")));
 
   bool FollowupHasTransforms = false;
-  MDNode *Followup = createPipeliningMetadata(Attrs, FollowupLoopProperties,
-                                              FollowupHasTransforms);
+  SmallVector<Metadata *, 4> Followup = createPipeliningMetadata(
+      Attrs, FollowupLoopProperties, FollowupHasTransforms);
 
   SmallVector<Metadata *, 4> Args;
-  Args.push_back(nullptr);
   Args.append(LoopProperties.begin(), LoopProperties.end());
 
   // Setting unroll.count
@@ -130,16 +123,14 @@ LoopInfo::createPartialUnrollMetadata(const LoopAttributes &Attrs,
   }
 
   if (FollowupHasTransforms)
-    Args.push_back(MDNode::get(
-        Ctx, {MDString::get(Ctx, "llvm.loop.unroll.followup_all"), Followup}));
+    Args.push_back(
+        createFollowupMetadata("llvm.loop.unroll.followup_all", Followup));
 
-  MDNode *LoopID = MDNode::getDistinct(Ctx, Args);
-  LoopID->replaceOperandWith(0, LoopID);
   HasUserTransforms = true;
-  return LoopID;
+  return Args;
 }
 
-MDNode *
+SmallVector<Metadata *, 4>
 LoopInfo::createUnrollAndJamMetadata(const LoopAttributes &Attrs,
                                      ArrayRef<Metadata *> LoopProperties,
                                      bool &HasUserTransforms) {
@@ -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<Metadata *, 4> Followup = createPartialUnrollMetadata(
+      Attrs, FollowupLoopProperties, FollowupHasTransforms);
 
   SmallVector<Metadata *, 4> Args;
-  Args.push_back(nullptr);
   Args.append(LoopProperties.begin(), LoopProperties.end());
 
   // Setting unroll_and_jam.count
@@ -192,22 +182,18 @@ LoopInfo::createUnrollAndJamMetadata(const LoopAttributes &Attrs,
   }
 
   if (FollowupHasTransforms)
-    Args.push_back(MDNode::get(
-        Ctx, {MDString::get(Ctx, "llvm.loop.unroll_and_jam.followup_outer"),
-              Followup}));
+    Args.push_back(createFollowupMetadata(
+        "llvm.loop.unroll_and_jam.followup_outer", Followup));
 
-  if (UnrollAndJamInnerFollowup)
-    Args.push_back(MDNode::get(
-        Ctx, {MDString::get(Ctx, "llvm.loop.unroll_and_jam.followup_inner"),
-              UnrollAndJamInnerFollowup}));
+  if (UnrollAndJamInnerFollowup.has_value())
+    Args.push_back(createFollowupMetadata(
+        "llvm.loop.unroll_and_jam.followup_inner", *UnrollAndJamInnerFollowup));
 
-  MDNode *LoopID = MDNode::getDistinct(Ctx, Args);
-  LoopID->replaceOperandWith(0, LoopID);
   HasUserTransforms = true;
-  return LoopID;
+  return Args;
 }
 
-MDNode *
+SmallVector<Metadata *, 4>
 LoopInfo::createLoopVectorizeMetadata(const LoopAttributes &Attrs,
                                       ArrayRef<Metadata *> LoopProperties,
                                       bool &HasUserTransforms) {
@@ -244,11 +230,10 @@ LoopInfo::createLoopVectorizeMetadata(const LoopAttributes &Attrs,
       MDNode::get(Ctx, MDString::get(Ctx, "llvm.loop.isvectorized")));
 
   bool FollowupHasTransforms = false;
-  MDNode *Followup = createUnrollAndJamMetadata(Attrs, FollowupLoopProperties,
-                                                FollowupHasTransforms);
+  SmallVector<Metadata *, 4> Followup = createUnrollAndJamMetadata(
+      Attrs, FollowupLoopProperties, FollowupHasTransforms);
 
   SmallVector<Metadata *, 4> Args;
-  Args.push_back(nullptr);
   Args.append(LoopProperties.begin(), LoopProperties.end());
 
   // Setting vectorize.predicate when it has been specified and vectorization
@@ -315,17 +300,14 @@ LoopInfo::createLoopVectorizeMetadata(const LoopAttributes &Attrs,
   }
 
   if (FollowupHasTransforms)
-    Args.push_back(MDNode::get(
-        Ctx,
-        {MDString::get(Ctx, "llvm.loop.vectorize.followup_all"), Followup}));
+    Args.push_back(
+        createFollowupMetadata("llvm.loop.vectorize.followup_all", Followup));
 
-  MDNode *LoopID = MDNode::getDistinct(Ctx, Args);
-  LoopID->replaceOperandWith(0, LoopID);
   HasUserTransforms = true;
-  return LoopID;
+  return Args;
 }
 
-MDNode *
+SmallVector<Metadata *, 4>
 LoopInfo::createLoopDistributeMetadata(const LoopAttributes &Attrs,
                                        ArrayRef<Metadata *> LoopProperties,
                                        bool &HasUserTransforms) {
@@ -352,11 +334,10 @@ LoopInfo::createLoopDistributeMetadata(const LoopAttributes &Attrs,
   }
 
   bool FollowupHasTransforms = false;
-  MDNode *Followup =
+  SmallVector<Metadata *, 4> Followup =
       createLoopVectorizeMetadata(Attrs, LoopProperties, FollowupHasTransforms);
 
   SmallVector<Metadata *, 4> Args;
-  Args.push_back(nullptr);
   Args.append(LoopProperties.begin(), LoopProperties.end());
 
   Metadata *Vals[] = {MDString::get(Ctx, "llvm.loop.distribute.enable"),
@@ -366,19 +347,17 @@ LoopInfo::createLoopDistributeMetadata(const LoopAttributes &Attrs,
   Args.push_back(MDNode::get(Ctx, Vals));
 
   if (FollowupHasTransforms)
-    Args.push_back(MDNode::get(
-        Ctx,
-        {MDString::get(Ctx, "llvm.loop.distribute.followup_all"), Followup}));
+    Args.push_back(
+        createFollowupMetadata("llvm.loop.distribute.followup_all", Followup));
 
-  MDNode *LoopID = MDNode::getDistinct(Ctx, Args);
-  LoopID->replaceOperandWith(0, LoopID);
   HasUserTransforms = true;
-  return LoopID;
+  return Args;
 }
 
-MDNode *LoopInfo::createFullUnrollMetadata(const LoopAttributes &Attrs,
-                                           ArrayRef<Metadata *> LoopProperties,
-                                           bool &HasUserTransforms) {
+SmallVector<Metadata *, 4>
+LoopInfo::createFullUnrollMetadata(const LoopAttributes &Attrs,
+                                   ArrayRef<Metadata *> LoopProperties,
+                                   bool &HasUserTransforms) {
   LLVMContext &Ctx = Header->getContext();
 
   std::optional<bool> Enabled;
@@ -400,20 +379,17 @@ MDNode *LoopInfo::createFullUnrollMetadata(const LoopAttributes &Attrs,
   }
 
   SmallVector<Metadata *, 4> Args;
-  Args.push_back(nullptr);
   Args.append(LoopProperties.begin(), LoopProperties.end());
   Args.push_back(MDNode::get(Ctx, MDString::get(Ctx, "llvm.loop.unroll.full")));
 
   // No follow-up: there is no loop after full unrolling.
   // TODO: Warn if there are transformations after full unrolling.
 
-  MDNode *LoopID = MDNode::getDistinct(Ctx, Args);
-  LoopID->replaceOperandWith(0, LoopID);
   HasUserTransforms = true;
-  return LoopID;
+  return Args;
 }
 
-MDNode *LoopInfo::createMetadata(
+SmallVector<Metadata *, 4> LoopInfo::createMetadata(
     const LoopAttributes &Attrs,
     llvm::ArrayRef<llvm::Metadata *> AdditionalLoopProperties,
     bool &HasUserTransforms) {
@@ -579,8 +555,8 @@ void LoopInfo::finish() {
             MDNode::get(Ctx, MDString::get(Ctx, "llvm.loop.isvectorized")));
 
       bool InnerFollowupHasTransform = false;
-      MDNode *InnerFollowup = createMetadata(AfterJam, BeforeLoopProperties,
-                                             InnerFollowupHasTransform);
+      SmallVector<Metadata *, 4> InnerFollowup = createMetadata(
+          AfterJam, BeforeLoopProperties, InnerFollowupHasTransform);
       if (InnerFollowupHasTransform)
         Parent->UnrollAndJamInnerFollowup = InnerFollowup;
     }
@@ -589,7 +565,14 @@ void LoopInfo::finish() {
   }
 
   bool HasUserTransforms = false;
-  LoopID = createMetadata(CurLoopAttr, {}, HasUserTransforms);
+  SmallVector<Metadata *, 4> Properties =
+      createMetadata(CurLoopAttr, {}, HasUserTransforms);
+  SmallVector<Metadata *, 4> Args;
+  Args.push_back(nullptr);
+  Args.append(Properties.begin(), Properties.end());
+  LoopID = MDNode::getDistinct(Ctx, Args);
+  LoopID->replaceOperandWith(0, LoopID);
+
   TempLoopID->replaceAllUsesWith(LoopID);
 }
 

clang/lib/CodeGen/CGLoopInfo.h

@@ -132,17 +132,19 @@ class LoopInfo {
   /// If this loop has unroll-and-jam metadata, this can be set by the inner
   /// loop's LoopInfo to set the llvm.loop.unroll_and_jam.followup_inner
   /// metadata.
-  llvm::MDNode *UnrollAndJamInnerFollowup = nullptr;
+  std::optional<llvm::SmallVector<llvm::Metadata *, 4>>
+      UnrollAndJamInnerFollowup;
 
-  /// Create a LoopID without any transformations.
+  /// Create a followup MDNode that has @p LoopProperties as its attributes.
   llvm::MDNode *
-  createLoopPropertiesMetadata(llvm::ArrayRef<llvm::Metadata *> LoopProperties);
+  createFollowupMetadata(const char *FollowupName,
+                         llvm::ArrayRef<llvm::Metadata *> LoopProperties);
 
-  /// Create a LoopID for transformations.
+  /// Create a metadata list for transformations.
   ///
   /// The methods call each other in case multiple transformations are applied
-  /// to a loop. The transformation first to be applied will use LoopID of the
-  /// next transformation in its followup attribute.
+  /// to a loop. The transformation first to be applied will use metadata list
+  /// of the next transformation in its followup attribute.
   ///
   /// @param Attrs             The loop's transformations.
   /// @param LoopProperties    Non-transformation properties such as debug
@@ -152,36 +154,37 @@ class LoopInfo {
   /// @param HasUserTransforms [out] Set to true if the returned MDNode encodes
   ///                          at least one transformation.
   ///
-  /// @return A LoopID (metadata node) that can be used for the llvm.loop
-  ///         annotation or followup-attribute.
+  /// @return A metadata list that can be used for the llvm.loop annotation or
+  ///         followup-attribute.
   /// @{
-  llvm::MDNode *
+  llvm::SmallVector<llvm::Metadata *, 4>
   createPipeliningMetadata(const LoopAttributes &Attrs,
                            llvm::ArrayRef<llvm::Metadata *> LoopProperties,
                            bool &HasUserTransforms);
-  llvm::MDNode *
+  llvm::SmallVector<llvm::Metadata *, 4>
   createPartialUnrollMetadata(const LoopAttributes &Attrs,
                               llvm::ArrayRef<llvm::Metadata *> LoopProperties,
                               bool &HasUserTransforms);
-  llvm::MDNode *
+  llvm::SmallVector<llvm::Metadata *, 4>
   createUnrollAndJamMetadata(const LoopAttributes &Attrs,
                              llvm::ArrayRef<llvm::Metadata *> LoopProperties,
                              bool &HasUserTransforms);
-  llvm::MDNode *
+  llvm::SmallVector<llvm::Metadata *, 4>
   createLoopVectorizeMetadata(const LoopAttributes &Attrs,
                               llvm::ArrayRef<llvm::Metadata *> LoopProperties,
                               bool &HasUserTransforms);
-  llvm::MDNode *
+  llvm::SmallVector<llvm::Metadata *, 4>
   createLoopDistributeMetadata(const LoopAttributes &Attrs,
                                llvm::ArrayRef<llvm::Metadata *> LoopProperties,
                                bool &HasUserTransforms);
-  llvm::MDNode *
+  llvm::SmallVector<llvm::Metadata *, 4>
   createFullUnrollMetadata(const LoopAttributes &Attrs,
                            llvm::ArrayRef<llvm::Metadata *> LoopProperties,
                            bool &HasUserTransforms);
+
   /// @}
 
-  /// Create a LoopID for this loop, including transformation-unspecific
+  /// Create a metadata list for this loop, including transformation-unspecific
   /// metadata such as debug location.
   ///
   /// @param Attrs             This loop's attributes and transformations.
@@ -191,11 +194,11 @@ class LoopInfo {
   /// @param HasUserTransforms [out] Set to true if the returned MDNode encodes
   ///                          at least one transformation.
   ///
-  /// @return A LoopID (metadata node) that can be used for the llvm.loop
-  ///         annotation.
-  llvm::MDNode *createMetadata(const LoopAttributes &Attrs,
-                               llvm::ArrayRef<llvm::Metadata *> LoopProperties,
-                               bool &HasUserTransforms);
+  /// @return A metadata list that can be used for the llvm.loop annotation.
+  llvm::SmallVector<llvm::Metadata *, 4>
+  createMetadata(const LoopAttributes &Attrs,
+                 llvm::ArrayRef<llvm::Metadata *> LoopProperties,
+                 bool &HasUserTransforms);
 };
 
 /// A stack of loop information corresponding to loop nesting levels.

clang/test/CodeGenCXX/pragma-followup_inner.cpp

@@ -23,20 +23,17 @@ extern "C" void followup_inner(int n, int *x) {
 // CHECK-DAG: ![[INNERLOOP_3]] = distinct !{![[INNERLOOP_3]], ![[PARALLEL_ACCESSES_4:[0-9]+]], ![[DISTRIBUTE_5:[0-9]+]], ![[DISTRIBUTE_FOLLOWUP_6:[0-9]+]]}
 // CHECK-DAG: ![[PARALLEL_ACCESSES_4]] = !{!"llvm.loop.parallel_accesses", ![[ACCESSGROUP_2]]}
 // CHECK-DAG: ![[DISTRIBUTE_5]] = !{!"llvm.loop.distribute.enable", i1 true}
-// CHECK-DAG: ![[DISTRIBUTE_FOLLOWUP_6]] = !{!"llvm.loop.distribute.followup_all", ![[LOOP_7:[0-9]+]]}
 
-// CHECK-DAG: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[PARALLEL_ACCESSES_4]], ![[VECTORIZE_8:[0-9]+]]}
+// CHECK-DAG: ![[DISTRIBUTE_FOLLOWUP_6]] = !{!"llvm.loop.distribute.followup_all", ![[PARALLEL_ACCESSES_4]], ![[VECTORIZE_8:[0-9]+]]}
 // CHECK-DAG: ![[VECTORIZE_8]] = !{!"llvm.loop.vectorize.enable", i1 true}
 
 // CHECK-DAG: ![[OUTERLOOP_9]] = distinct !{![[OUTERLOOP_9]], [[MP:![0-9]+]], ![[UNROLLANDJAM_COUNT_10:[0-9]+]], ![[UNROLLANDJAM_FOLLOWUPINNER_11:[0-9]+]]}
 // CHECK-DAG: ![[UNROLLANDJAM_COUNT_10]] = !{!"llvm.loop.unroll_and_jam.count", i32 4}
-// CHECK-DAG: ![[UNROLLANDJAM_FOLLOWUPINNER_11]] = !{!"llvm.loop.unroll_and_jam.followup_inner", ![[LOOP_12:[0-9]+]]}
 
-// CHECK-DAG: ![[LOOP_12]] = distinct !{![[LOOP_12:[0-9]+]], ![[PARALLEL_ACCESSES_4]], ![[ISVECTORIZED_13:[0-9]+]], ![[UNROLL_COUNT_13:[0-9]+]], ![[UNROLL_FOLLOWUP_14:[0-9]+]]}
+// CHECK-DAG: ![[UNROLLANDJAM_FOLLOWUPINNER_11]] = !{!"llvm.loop.unroll_and_jam.followup_inner", ![[PARALLEL_ACCESSES_4]], ![[ISVECTORIZED_13:[0-9]+]], ![[UNROLL_COUNT_13:[0-9]+]], ![[UNROLL_FOLLOWUP_14:[0-9]+]]}
 // CHECK-DAG: ![[ISVECTORIZED_13]] = !{!"llvm.loop.isvectorized"}
 // CHECK-DAG: ![[UNROLL_COUNT_13]] = !{!"llvm.loop.unroll.count", i32 4}
-// CHECK-DAG: ![[UNROLL_FOLLOWUP_14]] = !{!"llvm.loop.unroll.followup_all", ![[LOOP_15:[0-9]+]]}
 
-// CHECK-DAG: ![[LOOP_15]] = distinct !{![[LOOP_15]], ![[PARALLEL_ACCESSES_4]], ![[ISVECTORIZED_13]], ![[UNROLL_DISABLE_16:[0-9]+]], ![[PIPELINE_17:[0-9]+]]}
+// CHECK-DAG: ![[UNROLL_FOLLOWUP_14]] = !{!"llvm.loop.unroll.followup_all", ![[PARALLEL_ACCESSES_4]], ![[ISVECTORIZED_13]], ![[UNROLL_DISABLE_16:[0-9]+]], ![[PIPELINE_17:[0-9]+]]}
 // CHECK-DAG: ![[UNROLL_DISABLE_16]] = !{!"llvm.loop.unroll.disable"}
 // CHECK-DAG: ![[PIPELINE_17]] = !{!"llvm.loop.pipeline.initiationinterval", i32 10}