[UnJ] Ensure unroll_and_jam metadata is removed once consumed.

[dmgreen]

Summary

If we have unroll and jammed a loop based on metadata, make sure we
remove the now consumed unroll_and_jam metadata.

[hiraditya]

include/llvm/Analysis/LoopInfo.h

568

I think we can add the Prefix at the caller all the time, a default value of "llvm.loop.unroll." may not be general. It seems renaming the function to reflect a more general semantics like: setLoopAlreadyTransformed and then renaming appropriate local variables would be a good improvement.

[Meinersbur]

include/llvm/Analysis/LoopInfo.h

568

There are a lot of inconsistencies in handling metadata. LoopVectorizer adds llvm.loop.isvectorized instead of removing its metadata. LoopDistribution doesn't care about remaining metadata at all.

I am trying to add a bit of uniformness in D49281. If a followup attribute is set, it drops all loop preexisting metadata since the transformed loop is a different loop than the loop the metadata is for.

LoopUnroll and LoopUnrollAndJam are strangely intertwined. Some setting to LoopUnroll also apply on LoopUnrollAndJam. Not an ideal situation.

If we go for a different name, I'd suggest forceDisableTransformation or similar. setLoopAlreadyUnrolled sounds like it does just sets a flag, but does more.

lib/Analysis/LoopInfo.cpp

295

[serious] This is intended to disable unroll-and-jam as well? why not using Prefix + "disable"?

[dmgreen]

include/llvm/Analysis/LoopInfo.h

568

As this is only used for unroll and unroll and jam, I think setLoopAlreadyUnrolled still fits, at least for the moment.

I think that once it's in it would be good to move towards the function in D49281, which should be able to handle this in a more generic way. This patch is an attempt to make that work better and not produce warnings for unrolled and jammed loops.

lib/Analysis/LoopInfo.cpp

295

Yep, it was intended. But with Prefix may be better.

llvm.loop.unroll.disable also disables unroll-and-jam if there is no unroll_and_jam metadata. As we remove all the unroll_and_jam metadata above, this will disable any further unrolling or unroll-and-jamming.

Whether it is entirely necessary to disable unroll here, I'm not 100% sure of. If a user does:

#pragma unroll_and_jam(2)
for ..
  #pragma unroll
  for ..

I'd expect it to be unroll-and-jammed by 2, but not then unrolled more. Using "llvm.loop.unroll.disable" will disable any follow-on unroll or unroll-and-jam.

[Meinersbur]

lib/Analysis/LoopInfo.cpp

295

I'd prefer to minimize the interaction between different passes. I am understanding unroll-and-jam as a pass orthogonal to unroll. I have no model in what why their metadata interact, and I'd prefer if it was predictable.

At the very least, please add a comment about the intention; it look too much like something has been forgotten.

I'd expect it to be unroll-and-jammed by 2, but not then unrolled more. Using "llvm.loop.unroll.disable" will disable any follow-on unroll or unroll-and-jam.

I don't understand. This this patch is already using llvm.loop.unroll.disable and disables further unrolling of the outer loop. There is no effect on the inner loop. That is, your example works as expected (I tried).

include/llvm/Analysis/LoopInfo.h

public:
   /// the loop that branches to the loop header.
   ///
   /// The LoopID metadata node should have one or more operands and the first
   /// operand should be the node itself.
   void setLoopID(MDNode *LoopID) const;
 
   /// Add llvm.loop.unroll.disable to this loop's loop id metadata.
   ///
-  /// Remove existing unroll metadata and add unroll disable metadata to
+  /// Remove existing Prefix metadata and add unroll disable metadata to
   /// indicate the loop has already been unrolled.  This prevents a loop
   /// from being unrolled more than is directed by a pragma if the loop
   /// unrolling pass is run more than once (which it generally is).
-  void setLoopAlreadyUnrolled();
+  void setLoopAlreadyUnrolled(StringRef Prefix = "llvm.loop.unroll.");
 
   void dump() const;
   void dumpVerbose() const;
 
   /// Return the debug location of the start of this loop.
   /// This looks for a BB terminating instruction with a known debug
   /// location by looking at the preheader and header blocks. If it
   /// cannot find a terminating instruction with location information,

lib/Analysis/LoopInfo.cpp

for (BasicBlock *BB : this->blocks()) {
     TerminatorInst *TI = BB->getTerminator();
     for (BasicBlock *Successor : TI->successors()) {
       if (Successor == H)
         TI->setMetadata(LLVMContext::MD_loop, LoopID);
     }
   }
 }
 
-void Loop::setLoopAlreadyUnrolled() {
+void Loop::setLoopAlreadyUnrolled(StringRef Prefix) {
   MDNode *LoopID = getLoopID();
   // First remove any existing loop unrolling metadata.
   SmallVector<Metadata *, 4> MDs;
   // Reserve first location for self reference to the LoopID metadata node.
   MDs.push_back(nullptr);
 
   if (LoopID) {
     for (unsigned i = 1, ie = LoopID->getNumOperands(); i < ie; ++i) {
       bool IsUnrollMetadata = false;
       MDNode *MD = dyn_cast<MDNode>(LoopID->getOperand(i));
       if (MD) {
         const MDString *S = dyn_cast<MDString>(MD->getOperand(0));
-        IsUnrollMetadata = S && S->getString().startswith("llvm.loop.unroll.");
+        IsUnrollMetadata = S && S->getString().startswith(Prefix);
       }
       if (!IsUnrollMetadata)
         MDs.push_back(LoopID->getOperand(i));
     }
   }
 
   // Add unroll(disable) metadata to disable future unrolling.
   LLVMContext &Context = getHeader()->getContext();
   SmallVector<Metadata *, 1> DisableOperands;
   DisableOperands.push_back(MDString::get(Context, "llvm.loop.unroll.disable"));
   MDNode *DisableNode = MDNode::get(Context, DisableOperands);
   MDs.push_back(DisableNode);
 
   MDNode *NewLoopID = MDNode::get(Context, MDs);
   // Set operand 0 to refer to the loop id itself.
   NewLoopID->replaceOperandWith(0, NewLoopID);
   setLoopID(NewLoopID);
 }

lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp

tryToUnrollAndJamLoop(Loop *L, DominatorTree &DT, LoopInfo *LI,
 
   LoopUnrollResult UnrollResult =
       UnrollAndJamLoop(L, UP.Count, OuterTripCount, OuterTripMultiple,
                        UP.UnrollRemainder, LI, &SE, &DT, &AC, &ORE);
 
   // If loop has an unroll count pragma or unrolled by explicitly set count
   // mark loop as unrolled to prevent unrolling beyond that requested.
   if (UnrollResult != LoopUnrollResult::FullyUnrolled && IsCountSetExplicitly)
-    L->setLoopAlreadyUnrolled();
+    L->setLoopAlreadyUnrolled("llvm.loop.unroll_and_jam.");
 
   return UnrollResult;
 }
 
 namespace {
 
 class LoopUnrollAndJam : public LoopPass {
 public:

test/Transforms/LoopUnrollAndJam/pragma.ll

for.end:
   ret void
 }
 
 
 ; CHECK-LABEL: nounroll_and_jam
 ; #pragma nounroll_and_jam
 define void @nounroll_and_jam(i32 %I, i32 %J, i32* noalias nocapture %A, i32* noalias nocapture readonly %B) {
 ; CHECK: %i.us = phi i32 [ %add8.us, %for.latch ], [ 0, %for.outer.preheader ]
+; CHECK: br i1 %exitcond25, label %for.end.loopexit, label %for.outer, !llvm.loop ![[NOUNROLLANDJAM:.*]]
 entry:
   %cmp = icmp ne i32 %J, 0
   %cmp122 = icmp ne i32 %I, 0
   %or.cond = and i1 %cmp, %cmp122
   br i1 %or.cond, label %for.outer.preheader, label %for.end
 
 for.outer.preheader:
   br label %for.outer
for.end:
   ret void
 }
 
 
 ; CHECK-LABEL: unroll_and_jam_count
 ; #pragma unroll_and_jam(8)
 define void @unroll_and_jam_count(i32 %I, i32 %J, i32* noalias nocapture %A, i32* noalias nocapture readonly %B) {
 ; CHECK: %i.us = phi i32 [ %add8.us.7, %for.latch ], [ 0, %for.outer.preheader.new ]
+; CHECK: br i1 %niter.ncmp.7, label %for.end.loopexit.unr-lcssa.loopexit, label %for.outer, !llvm.loop ![[NOUNROLL1:.*]]
 entry:
   %cmp = icmp ne i32 %J, 0
   %cmp122 = icmp ne i32 %I, 0
   %or.cond = and i1 %cmp, %cmp122
   br i1 %or.cond, label %for.outer.preheader, label %for.end
 
 for.outer.preheader:
   br label %for.outer

 }
 
 
 ; CHECK-LABEL: unroll_and_jam
 ; #pragma unroll_and_jam
 define void @unroll_and_jam(i32 %I, i32 %J, i32* noalias nocapture %A, i32* noalias nocapture readonly %B) {
 ; CHECK: %i.us = phi i32 [ %add8.us.{{[1-9]*}}, %for.latch ], [ 0, %for.outer.preheader.new ]
 ; CHECK-LOWTHRES: %i.us = phi i32 [ %add8.us.{{[1-9]*}}, %for.latch ], [ 0, %for.outer.preheader.new ]
+; CHECK: br i1 %niter.ncmp.7, label %for.end.loopexit.unr-lcssa.loopexit, label %for.outer, !llvm.loop ![[NOUNROLL2:.*]]
 entry:
   %cmp = icmp ne i32 %J, 0
   %cmp122 = icmp ne i32 %I, 0
   %or.cond = and i1 %cmp, %cmp122
   br i1 %or.cond, label %for.outer.preheader, label %for.end
 
 for.outer.preheader:
   br label %for.outer
for.end:
   ret void
 }
 
 
 ; CHECK-LABEL: nounroll
 ; #pragma nounroll (which we take to mean disable unroll and jam too)
 define void @nounroll(i32 %I, i32 %J, i32* noalias nocapture %A, i32* noalias nocapture readonly %B) {
 ; CHECK: %i.us = phi i32 [ %add8.us, %for.latch ], [ 0, %for.outer.preheader ]
+; CHECK: br i1 %exitcond25, label %for.end.loopexit, label %for.outer, !llvm.loop ![[NOUNROLL3:.*]]
 entry:
   %cmp = icmp ne i32 %J, 0
   %cmp122 = icmp ne i32 %I, 0
   %or.cond = and i1 %cmp, %cmp122
   br i1 %or.cond, label %for.outer.preheader, label %for.end
 
 for.outer.preheader:
   br label %for.outer
for.end:
   ret void
 }
 
 
 ; CHECK-LABEL: unroll
 ; #pragma unroll (which we take to mean disable unroll and jam)
 define void @unroll(i32 %I, i32 %J, i32* noalias nocapture %A, i32* noalias nocapture readonly %B) {
 ; CHECK: %i.us = phi i32 [ %add8.us, %for.latch ], [ 0, %for.outer.preheader ]
+; CHECK: br i1 %exitcond25, label %for.end.loopexit, label %for.outer, !llvm.loop ![[UNROLL:.*]]
 entry:
   %cmp = icmp ne i32 %J, 0
   %cmp122 = icmp ne i32 %I, 0
   %or.cond = and i1 %cmp, %cmp122
   br i1 %or.cond, label %for.outer.preheader, label %for.end
 
 for.outer.preheader:
   br label %for.outer
for.end:
   ret void
 }
 
 
 ; CHECK-LABEL: nounroll_plus_unroll_and_jam
 ; #pragma clang loop nounroll, unroll_and_jam (which we take to mean do unroll_and_jam)
 define void @nounroll_plus_unroll_and_jam(i32 %I, i32 %J, i32* noalias nocapture %A, i32* noalias nocapture readonly %B) {
 ; CHECK: %i.us = phi i32 [ %add8.us.{{[1-9]*}}, %for.latch ], [ 0, %for.outer.preheader.new ]
+; CHECK: br i1 %niter.ncmp.7, label %for.end.loopexit.unr-lcssa.loopexit, label %for.outer, !llvm.loop ![[NOUNROLLNOUNROLL:.*]]
 entry:
   %cmp = icmp ne i32 %J, 0
   %cmp122 = icmp ne i32 %I, 0
   %or.cond = and i1 %cmp, %cmp122
   br i1 %or.cond, label %for.outer.preheader, label %for.end
 
 for.outer.preheader:
   br label %for.outer

 
 for.end.loopexit:
   br label %for.end
 
 for.end:
   ret void
 }
 
-
-!1 = distinct !{!1, !2}
-!2 = distinct !{!"llvm.loop.unroll_and_jam.disable"}
-!3 = distinct !{!3, !4}
-!4 = distinct !{!"llvm.loop.unroll_and_jam.count", i32 8}
-!5 = distinct !{!5, !6}
-!6 = distinct !{!"llvm.loop.unroll_and_jam.enable"}
-!7 = distinct !{!7, !8}
-!8 = distinct !{!"llvm.loop.unroll.disable"}
-!9 = distinct !{!9, !10}
-!10 = distinct !{!"llvm.loop.unroll.enable"}
-!11 = distinct !{!11, !8, !6}
+!1 = !{!1, !2}
+!2 = !{!"llvm.loop.unroll_and_jam.disable"}
+!3 = !{!3, !4}
+!4 = !{!"llvm.loop.unroll_and_jam.count", i32 8}
+!5 = !{!5, !6}
+!6 = !{!"llvm.loop.unroll_and_jam.enable"}
+!7 = !{!7, !8}
+!8 = !{!"llvm.loop.unroll.disable"}
+!9 = !{!9, !10}
+!10 = !{!"llvm.loop.unroll.enable"}
+!11 = !{!11, !8, !6}
+
+;CHECK: ![[NOUNROLL_DATA:.*]] = !{!"llvm.loop.unroll.disable"}
+;CHECK: ![[NOUNROLLANDJAM]] = distinct !{![[NOUNROLLANDJAM]], ![[NOUNROLLANDJAM_DATA:.*]]}
+;CHECK: ![[NOUNROLLANDJAM_DATA]] = !{!"llvm.loop.unroll_and_jam.disable"}
+;CHECK: ![[NOUNROLL1]] = distinct !{![[NOUNROLL1]], ![[NOUNROLL_DATA]]}
+;CHECK: ![[NOUNROLL2]] = distinct !{![[NOUNROLL2]], ![[NOUNROLL_DATA]]}
+;CHECK: ![[NOUNROLL3]] = distinct !{![[NOUNROLL3]], ![[NOUNROLL_DATA]]}
+;CHECK: ![[UNROLL]] = distinct !{![[UNROLL]], ![[UNROLL_DATA:.*]]}
+;CHECK: ![[UNROLL_DATA]] = !{!"llvm.loop.unroll.enable"}
+;CHECK: ![[NOUNROLLNOUNROLL]] = distinct !{![[NOUNROLLNOUNROLL]], ![[NOUNROLL_DATA]], ![[NOUNROLL_DATA]]}