Add a routine to swap branch instruction operands, and update any

profile metadata at the same time. Use it to preserve metadata attached
to a branch when re-writing it in InstCombine.

Add metadata to the canonicalize_branch InstCombine test, and check that
it is tranformed correctly.

Reviewed by Nick Lewycky!

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@142168 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Instructions.h b/include/llvm/Instructions.h
index 23e3e7e0f7b6884c8189c024bebc2e1dd718615d..3faab35bf6873da99cb13946975b1edac3918208 100644 (file)
--- a/include/llvm/Instructions.h
+++ b/include/llvm/Instructions.h
@@ -2367,6 +2367,13 @@ public:
     *(&Op<-1>() - idx) = (Value*)NewSucc;
   }
 
+  /// \brief Swap the successors of this branch instruction.
+  ///
+  /// Swaps the successors of the branch instruction. This also swaps any
+  /// branch weight metadata associated with the instruction so that it
+  /// continues to map correctly to each operand.
+  void swapSuccessors();
+
   // Methods for support type inquiry through isa, cast, and dyn_cast:
   static inline bool classof(const BranchInst *) { return true; }
   static inline bool classof(const Instruction *I) {

diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp
index c66f423a292e9ab9c0d6329fdbf4630d9cf24cdf..92874b95234dfb350f52348d3932f235662ee5f2 100644 (file)
--- a/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1190,8 +1190,7 @@ Instruction *InstCombiner::visitBranchInst(BranchInst &BI) {
       !isa<Constant>(X)) {
     // Swap Destinations and condition...
     BI.setCondition(X);
-    BI.setSuccessor(0, FalseDest);
-    BI.setSuccessor(1, TrueDest);
+    BI.swapSuccessors();
     return &BI;
   }
 
@@ -1206,8 +1205,7 @@ Instruction *InstCombiner::visitBranchInst(BranchInst &BI) {
       Cond->setPredicate(FCmpInst::getInversePredicate(FPred));
       
       // Swap Destinations and condition.
-      BI.setSuccessor(0, FalseDest);
-      BI.setSuccessor(1, TrueDest);
+      BI.swapSuccessors();
       Worklist.Add(Cond);
       return &BI;
     }
@@ -1223,8 +1221,7 @@ Instruction *InstCombiner::visitBranchInst(BranchInst &BI) {
       ICmpInst *Cond = cast<ICmpInst>(BI.getCondition());
       Cond->setPredicate(ICmpInst::getInversePredicate(IPred));
       // Swap Destinations and condition.
-      BI.setSuccessor(0, FalseDest);
-      BI.setSuccessor(1, TrueDest);
+      BI.swapSuccessors();
       Worklist.Add(Cond);
       return &BI;
     }

diff --git a/lib/VMCore/Instructions.cpp b/lib/VMCore/Instructions.cpp
index 82f883ad98a34eca7040d3f5d843bb0841699212..b3a720527a8ec563fcf1d9f4584e8633c4d779be 100644 (file)
--- a/lib/VMCore/Instructions.cpp
+++ b/lib/VMCore/Instructions.cpp
@@ -785,6 +785,27 @@ BranchInst::BranchInst(const BranchInst &BI) :
   SubclassOptionalData = BI.SubclassOptionalData;
 }
 
+void BranchInst::swapSuccessors() {
+  assert(isConditional() &&
+         "Cannot swap successors of an unconditional branch");
+  Op<-1>().swap(Op<-2>());
+
+  // Update profile metadata if present and it matches our structural
+  // expectations.
+  MDNode *ProfileData = getMetadata(LLVMContext::MD_prof);
+  if (!ProfileData || ProfileData->getNumOperands() != 3)
+    return;
+
+  // The first operand is the name. Fetch them backwards and build a new one.
+  Value *Ops[] = {
+    ProfileData->getOperand(0),
+    ProfileData->getOperand(2),
+    ProfileData->getOperand(1)
+  };
+  setMetadata(LLVMContext::MD_prof,
+              MDNode::get(ProfileData->getContext(), Ops));
+}
+
 BasicBlock *BranchInst::getSuccessorV(unsigned idx) const {
   return getSuccessor(idx);
 }

diff --git a/test/Transforms/InstCombine/canonicalize_branch.ll b/test/Transforms/InstCombine/canonicalize_branch.ll
index 24090abcb5acd3f96c51c4f613a4b9361666e649..869546d57dcd95a9cfac33b81516f7ff2066cdd8 100644 (file)
--- a/test/Transforms/InstCombine/canonicalize_branch.ll
+++ b/test/Transforms/InstCombine/canonicalize_branch.ll
@@ -1,8 +1,23 @@
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
+; Test an already canonical branch to make sure we don't flip those.
+define i32 @test0(i32 %X, i32 %Y) {
+        %C = icmp eq i32 %X, %Y
+        br i1 %C, label %T, label %F, !prof !0
+
+; CHECK: @test0
+; CHECK: %C = icmp eq i32 %X, %Y
+; CHECK: br i1 %C, label %T, label %F
+
+T:
+        ret i32 12
+F:
+        ret i32 123
+}
+
 define i32 @test1(i32 %X, i32 %Y) {
         %C = icmp ne i32 %X, %Y
-        br i1 %C, label %T, label %F
+        br i1 %C, label %T, label %F, !prof !1
 
 ; CHECK: @test1
 ; CHECK: %C = icmp eq i32 %X, %Y
@@ -16,7 +31,7 @@ F:
 
 define i32 @test2(i32 %X, i32 %Y) {
         %C = icmp ule i32 %X, %Y
-        br i1 %C, label %T, label %F
+        br i1 %C, label %T, label %F, !prof !2
 
 ; CHECK: @test2
 ; CHECK: %C = icmp ugt i32 %X, %Y
@@ -30,7 +45,7 @@ F:
 
 define i32 @test3(i32 %X, i32 %Y) {
         %C = icmp uge i32 %X, %Y
-        br i1 %C, label %T, label %F
+        br i1 %C, label %T, label %F, !prof !3
 
 ; CHECK: @test3
 ; CHECK: %C = icmp ult i32 %X, %Y
@@ -42,3 +57,13 @@ F:
         ret i32 123
 }
 
+!0 = metadata !{metadata !"branch_weights", i32 1, i32 2}
+!1 = metadata !{metadata !"branch_weights", i32 3, i32 4}
+!2 = metadata !{metadata !"branch_weights", i32 5, i32 6}
+!3 = metadata !{metadata !"branch_weights", i32 7, i32 8}
+; Base case shouldn't change.
+; CHECK: !0 = {{.*}} i32 1, i32 2}
+; Ensure that the branch metadata is reversed to match the reversals above.
+; CHECK: !1 = {{.*}} i32 4, i32 3}
+; CHECK: !2 = {{.*}} i32 6, i32 5}
+; CHECK: !3 = {{.*}} i32 8, i32 7}