Preserve loop metadata when folding branches to a common destination

[danielcdh]

When/how was MD_loop set/used? Is it related to profile?

[mkuper]

In D27830#624499, @danielcdh wrote:

When/how was MD_loop set/used? Is it related to profile?

No, it's not related to profile information.
It's used to do things like explicitly enable or disable vectorization, unrolling, etc. I added you to the review because this is similar in spirit to what you did for the profile information.

[fhahn]

Moving the loop metadata to the new latch looks good to me and I think it's a great idea to preserve it as best as we can. I'm reluctant to accept the revision only because I'm fairly new to LLVM and my experience with SimplifyCFG.cpp is rather limited.

@danielcdh the language ref provides a good overview over the information contained in MD_Loop (http://llvm.org/docs/LangRef.html#llvm-loop)

[danielcdh]

Thanks for the clarification and Thanks Florian for the pointer to the doc.

The patch LGTM, but I think Hal would be more qualified than I do to approve this.

Optional: while you are at it, it'll be very useful to handle the MD_prof here for branch probability. (e.g. when PBI has no MD_prof while BI has, or like the case described in TODO comment).

Thanks,
Dehao

lib/Transforms/Utils/SimplifyCFG.cpp

2795 ↗

(On Diff #81677)

Maybe make it explicit in the comment that "BI was a loop latch" means "BI has MD_loop" Metadata.

[mkuper]

In D27830#624538, @danielcdh wrote:

Optional: while you are at it, it'll be very useful to handle the MD_prof here for branch probability. (e.g. when PBI has no MD_prof while BI has, or like the case described in TODO comment).

I'm a bit confused, I thought merging MD_prof is what the code directly above this does.

lib/Transforms/Utils/SimplifyCFG.cpp

2795 ↗

(On Diff #81677)

Right, what I was trying to say was that MD_loop is expected to live on loop latches, so that's when BI could have MD_loop.
I'll rephrase, thanks.

[danielcdh]

In D27830#624550, @mkuper wrote:

In D27830#624538, @danielcdh wrote:

Optional: while you are at it, it'll be very useful to handle the MD_prof here for branch probability. (e.g. when PBI has no MD_prof while BI has, or like the case described in TODO comment).

I'm a bit confused, I thought merging MD_prof is what the code directly above this does.

You are right, it's already done, I did not look above.

[hfinkel]

In D27830#624538, @danielcdh wrote:

Thanks for the clarification and Thanks Florian for the pointer to the doc.

The patch LGTM, but I think Hal would be more qualified than I do to approve this.

LGTM too.

llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp

if (BI->isConditional()) {
                                   MergedCond);
       }
       // Change PBI from Conditional to Unconditional.
       BranchInst *New_PBI = BranchInst::Create(TrueDest, PBI);
       EraseTerminatorInstAndDCECond(PBI);
       PBI = New_PBI;
     }
 
+    // If BI was a loop latch, it may have had associated loop metadata.
+    // We need to copy it to the new latch, that is, PBI.
+    if (MDNode *LoopMD = BI->getMetadata(LLVMContext::MD_loop))
+      PBI->setMetadata(LLVMContext::MD_loop, LoopMD);
+
     // TODO: If BB is reachable from all paths through PredBlock, then we
     // could replace PBI's branch probabilities with BI's.
 
     // Copy any debug value intrinsics into the end of PredBlock.
     for (Instruction &I : *BB)
       if (isa<DbgInfoIntrinsic>(I))
         I.clone()->insertBefore(PBI);
 

llvm/trunk/test/Transforms/LoopSimplify/preserve-llvm-loop-metadata.ll

 ; RUN: opt -loop-simplify -S < %s | FileCheck %s
 
+; CHECK-LABEL: @test1
 define void @test1(i32 %n) {
 entry:
   br label %while.cond
 
 while.cond:                                       ; preds = %if.then, %if.else, %entry
   %count.0 = phi i32 [ 0, %entry ], [ %add, %if.then ], [ %add2, %if.else ]
   %cmp = icmp ugt i32 %count.0, %n
   br i1 %cmp, label %while.end, label %while.body

 ; CHECK-NOT: br {{.*}}!llvm.loop{{.*}}
 
 ; CHECK: while.cond.backedge:
 ; CHECK: br label %while.cond, !llvm.loop !0
 
 ; CHECK: if.else
 ; CHECK-NOT: br {{.*}}!llvm.loop{{.*}}
 
+; CHECK-LABEL: @test2
+; CHECK: for.body:
+; CHECK: br i1 %{{.*}}, label %for.body, label %cleanup.loopexit, !llvm.loop !0
+define void @test2(i32 %k)  {
+entry: 
+  %cmp9 = icmp sgt i32 %k, 0
+  br i1 %cmp9, label %for.body.preheader, label %cleanup
+
+for.body.preheader:                               ; preds = %entry
+  br label %for.body
+
+for.cond:                                         ; preds = %for.body
+  %cmp = icmp slt i32 %inc, %k
+  br i1 %cmp, label %for.body, label %cleanup.loopexit, !llvm.loop !0
+
+for.body:                                         ; preds = %for.body.preheader, %for.cond
+  %i.010 = phi i32 [ %inc, %for.cond ], [ 0, %for.body.preheader ]
+  %cmp3 = icmp sgt i32 %i.010, 3
+  %inc = add nsw i32 %i.010, 1
+  br i1 %cmp3, label %cleanup.loopexit, label %for.cond
+
+cleanup.loopexit:                                 ; preds = %for.body, %for.cond
+  br label %cleanup
+
+cleanup:                                          ; preds = %cleanup.loopexit, %entry
+  ret void
+}
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.distribute.enable", i1 true}
 ; CHECK: !0 = distinct !{!0, !1}
 ; CHECK: !1 = !{!"llvm.loop.distribute.enable", i1 true}