From: Chandler Carruth Date: Mon, 17 Oct 2011 01:11:57 +0000 (+0000) Subject: Add a routine to swap branch instruction operands, and update any X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=602650c98822371d4a34b00353ec71051621b7fb 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 23e3e7e0f7b..3faab35bf68 100644 --- 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 c66f423a292..92874b95234 100644 --- a/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1190,8 +1190,7 @@ Instruction *InstCombiner::visitBranchInst(BranchInst &BI) { !isa(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(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 82f883ad98a..b3a720527a8 100644 --- 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 24090abcb5a..869546d57dc 100644 --- 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}