<div dir="ltr">I ran the tests on my local machine, it worked fine.<div>I will revert this commit.</div><div><br></div><div>Manman</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jan 29, 2014 at 4:56 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Looks like this broke test/CodeGen/ARM/lsr-unfolded-offset.ll.<br>
<div class="HOEnZb"><div class="h5"><br>
On 29 January 2014 19:24, Manman Ren <<a href="mailto:manman.ren@gmail.com">manman.ren@gmail.com</a>> wrote:<br>
> Author: mren<br>
> Date: Wed Jan 29 18:24:37 2014<br>
> New Revision: 200431<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=200431&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=200431&view=rev</a><br>
> Log:<br>
> PGO branch weight: update edge weights in SelectionDAGBuilder.<br>
><br>
> When converting from "or + br" to two branches, or converting from<br>
> "and + br" to two branches, we correctly update the edge weights of<br>
> the two branches.<br>
><br>
> Added:<br>
> llvm/trunk/test/CodeGen/X86/MachineBranchProb.ll<br>
> Modified:<br>
> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp<br>
> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=200431&r1=200430&r2=200431&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=200431&r1=200430&r2=200431&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Wed Jan 29 18:24:37 2014<br>
> @@ -1393,7 +1393,9 @@ SelectionDAGBuilder::EmitBranchForMerged<br>
> MachineBasicBlock *TBB,<br>
> MachineBasicBlock *FBB,<br>
> MachineBasicBlock *CurBB,<br>
> - MachineBasicBlock *SwitchBB) {<br>
> + MachineBasicBlock *SwitchBB,<br>
> + uint32_t TWeight,<br>
> + uint32_t FWeight) {<br>
> const BasicBlock *BB = CurBB->getBasicBlock();<br>
><br>
> // If the leaf of the tree is a comparison, merge the condition into<br>
> @@ -1418,7 +1420,7 @@ SelectionDAGBuilder::EmitBranchForMerged<br>
> }<br>
><br>
> CaseBlock CB(Condition, BOp->getOperand(0),<br>
> - BOp->getOperand(1), NULL, TBB, FBB, CurBB);<br>
> + BOp->getOperand(1), NULL, TBB, FBB, CurBB, TWeight, FWeight);<br>
> SwitchCases.push_back(CB);<br>
> return;<br>
> }<br>
> @@ -1426,17 +1428,26 @@ SelectionDAGBuilder::EmitBranchForMerged<br>
><br>
> // Create a CaseBlock record representing this branch.<br>
> CaseBlock CB(ISD::SETEQ, Cond, ConstantInt::getTrue(*DAG.getContext()),<br>
> - NULL, TBB, FBB, CurBB);<br>
> + NULL, TBB, FBB, CurBB, TWeight, FWeight);<br>
> SwitchCases.push_back(CB);<br>
> }<br>
><br>
> +/// Scale down both weights to fit into uint32_t.<br>
> +static void ScaleWeights(uint64_t &NewTrue, uint64_t &NewFalse) {<br>
> + uint64_t NewMax = (NewTrue > NewFalse) ? NewTrue : NewFalse;<br>
> + uint32_t Scale = (NewMax / UINT32_MAX) + 1;<br>
> + NewTrue = NewTrue / Scale;<br>
> + NewFalse = NewFalse / Scale;<br>
> +}<br>
> +<br>
> /// FindMergedConditions - If Cond is an expression like<br>
> void SelectionDAGBuilder::FindMergedConditions(const Value *Cond,<br>
> MachineBasicBlock *TBB,<br>
> MachineBasicBlock *FBB,<br>
> MachineBasicBlock *CurBB,<br>
> MachineBasicBlock *SwitchBB,<br>
> - unsigned Opc) {<br>
> + unsigned Opc, uint32_t TWeight,<br>
> + uint32_t FWeight) {<br>
> // If this node is not part of the or/and tree, emit it as a branch.<br>
> const Instruction *BOp = dyn_cast<Instruction>(Cond);<br>
> if (!BOp || !(isa<BinaryOperator>(BOp) || isa<CmpInst>(BOp)) ||<br>
> @@ -1444,7 +1455,8 @@ void SelectionDAGBuilder::FindMergedCond<br>
> BOp->getParent() != CurBB->getBasicBlock() ||<br>
> !InBlock(BOp->getOperand(0), CurBB->getBasicBlock()) ||<br>
> !InBlock(BOp->getOperand(1), CurBB->getBasicBlock())) {<br>
> - EmitBranchForMergedCondition(Cond, TBB, FBB, CurBB, SwitchBB);<br>
> + EmitBranchForMergedCondition(Cond, TBB, FBB, CurBB, SwitchBB,<br>
> + TWeight, FWeight);<br>
> return;<br>
> }<br>
><br>
> @@ -1456,6 +1468,7 @@ void SelectionDAGBuilder::FindMergedCond<br>
><br>
> if (Opc == Instruction::Or) {<br>
> // Codegen X | Y as:<br>
> + // BB1:<br>
> // jmp_if_X TBB<br>
> // jmp TmpBB<br>
> // TmpBB:<br>
> @@ -1463,14 +1476,34 @@ void SelectionDAGBuilder::FindMergedCond<br>
> // jmp FBB<br>
> //<br>
><br>
> + // We have flexibility in setting Prob for BB1 and Prob for TmpBB.<br>
> + // The requirement is that<br>
> + // TrueProb for BB1 + (FalseProb for BB1 * TrueProb for TmpBB)<br>
> + // = TrueProb for orignal BB.<br>
> + // Assuming the orignal weights are A and B, one choice is to set BB1's<br>
> + // weights to A and A+2B, and set TmpBB's weights to A and 2B. This choice<br>
> + // assumes that<br>
> + // TrueProb for BB1 == FalseProb for BB1 * TrueProb for TmpBB.<br>
> + // Another choice is to assume TrueProb for BB1 equals to TrueProb for<br>
> + // TmpBB, but the math is more complicated.<br>
> +<br>
> + uint64_t NewTrueWeight = TWeight;<br>
> + uint64_t NewFalseWeight = (uint64_t)TWeight + 2 * (uint64_t)FWeight;<br>
> + ScaleWeights(NewTrueWeight, NewFalseWeight);<br>
> // Emit the LHS condition.<br>
> - FindMergedConditions(BOp->getOperand(0), TBB, TmpBB, CurBB, SwitchBB, Opc);<br>
> + FindMergedConditions(BOp->getOperand(0), TBB, TmpBB, CurBB, SwitchBB, Opc,<br>
> + NewTrueWeight, NewFalseWeight);<br>
><br>
> + NewTrueWeight = TWeight;<br>
> + NewFalseWeight = 2 * (uint64_t)FWeight;<br>
> + ScaleWeights(NewTrueWeight, NewFalseWeight);<br>
> // Emit the RHS condition into TmpBB.<br>
> - FindMergedConditions(BOp->getOperand(1), TBB, FBB, TmpBB, SwitchBB, Opc);<br>
> + FindMergedConditions(BOp->getOperand(1), TBB, FBB, TmpBB, SwitchBB, Opc,<br>
> + NewTrueWeight, NewFalseWeight);<br>
> } else {<br>
> assert(Opc == Instruction::And && "Unknown merge op!");<br>
> // Codegen X & Y as:<br>
> + // BB1:<br>
> // jmp_if_X TmpBB<br>
> // jmp FBB<br>
> // TmpBB:<br>
> @@ -1479,11 +1512,28 @@ void SelectionDAGBuilder::FindMergedCond<br>
> //<br>
> // This requires creation of TmpBB after CurBB.<br>
><br>
> + // We have flexibility in setting Prob for BB1 and Prob for TmpBB.<br>
> + // The requirement is that<br>
> + // FalseProb for BB1 + (TrueProb for BB1 * FalseProb for TmpBB)<br>
> + // = FalseProb for orignal BB.<br>
> + // Assuming the orignal weights are A and B, one choice is to set BB1's<br>
> + // weights to 2A+B and B, and set TmpBB's weights to 2A and B. This choice<br>
> + // assumes that<br>
> + // FalseProb for BB1 == TrueProb for BB1 * FalseProb for TmpBB.<br>
> +<br>
> + uint64_t NewTrueWeight = 2 * (uint64_t)TWeight + (uint64_t)FWeight;<br>
> + uint64_t NewFalseWeight = FWeight;<br>
> + ScaleWeights(NewTrueWeight, NewFalseWeight);<br>
> // Emit the LHS condition.<br>
> - FindMergedConditions(BOp->getOperand(0), TmpBB, FBB, CurBB, SwitchBB, Opc);<br>
> + FindMergedConditions(BOp->getOperand(0), TmpBB, FBB, CurBB, SwitchBB, Opc,<br>
> + NewTrueWeight, NewFalseWeight);<br>
><br>
> + NewTrueWeight = 2 * (uint64_t)TWeight;<br>
> + NewFalseWeight = FWeight;<br>
> + ScaleWeights(NewTrueWeight, NewFalseWeight);<br>
> // Emit the RHS condition into TmpBB.<br>
> - FindMergedConditions(BOp->getOperand(1), TBB, FBB, TmpBB, SwitchBB, Opc);<br>
> + FindMergedConditions(BOp->getOperand(1), TBB, FBB, TmpBB, SwitchBB, Opc,<br>
> + NewTrueWeight, NewFalseWeight);<br>
> }<br>
> }<br>
><br>
> @@ -1570,7 +1620,8 @@ void SelectionDAGBuilder::visitBr(const<br>
> (BOp->getOpcode() == Instruction::And ||<br>
> BOp->getOpcode() == Instruction::Or)) {<br>
> FindMergedConditions(BOp, Succ0MBB, Succ1MBB, BrMBB, BrMBB,<br>
> - BOp->getOpcode());<br>
> + BOp->getOpcode(), getEdgeWeight(BrMBB, Succ0MBB),<br>
> + getEdgeWeight(BrMBB, Succ1MBB));<br>
> // If the compares in later blocks need to use values not currently<br>
> // exported from this block, export them now. This block should always<br>
> // be the first entry.<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h?rev=200431&r1=200430&r2=200431&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h?rev=200431&r1=200430&r2=200431&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h (original)<br>
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h Wed Jan 29 18:24:37 2014<br>
> @@ -612,11 +612,13 @@ public:<br>
><br>
> void FindMergedConditions(const Value *Cond, MachineBasicBlock *TBB,<br>
> MachineBasicBlock *FBB, MachineBasicBlock *CurBB,<br>
> - MachineBasicBlock *SwitchBB, unsigned Opc);<br>
> + MachineBasicBlock *SwitchBB, unsigned Opc,<br>
> + uint32_t TW, uint32_t FW);<br>
> void EmitBranchForMergedCondition(const Value *Cond, MachineBasicBlock *TBB,<br>
> MachineBasicBlock *FBB,<br>
> MachineBasicBlock *CurBB,<br>
> - MachineBasicBlock *SwitchBB);<br>
> + MachineBasicBlock *SwitchBB,<br>
> + uint32_t TW, uint32_t FW);<br>
> bool ShouldEmitAsBranches(const std::vector<CaseBlock> &Cases);<br>
> bool isExportableFromCurrentBlock(const Value *V, const BasicBlock *FromBB);<br>
> void CopyToExportRegsIfNeeded(const Value *V);<br>
><br>
> Added: llvm/trunk/test/CodeGen/X86/MachineBranchProb.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/MachineBranchProb.ll?rev=200431&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/MachineBranchProb.ll?rev=200431&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/CodeGen/X86/MachineBranchProb.ll (added)<br>
> +++ llvm/trunk/test/CodeGen/X86/MachineBranchProb.ll Wed Jan 29 18:24:37 2014<br>
> @@ -0,0 +1,34 @@<br>
> +; RUN: llc < %s -mtriple=x86_64-apple-darwin -print-machineinstrs=expand-isel-pseudos -o /dev/null 2>&1 | FileCheck %s<br>
> +<br>
> +;; Make sure a transformation in SelectionDAGBuilder that converts "or + br" to<br>
> +;; two branches correctly updates the branch probability.<br>
> +<br>
> +@max_regno = common global i32 0, align 4<br>
> +<br>
> +define void @test(i32* %old, i32 %final) {<br>
> +for.cond:<br>
> + br label %for.cond2<br>
> +<br>
> +for.cond2: ; preds = %for.inc, %for.cond<br>
> + %i.1 = phi i32 [ %inc19, %for.inc ], [ 0, %for.cond ]<br>
> + %bit.0 = phi i32 [ %shl, %for.inc ], [ 1, %for.cond ]<br>
> + %tobool = icmp eq i32 %bit.0, 0<br>
> + %v3 = load i32* @max_regno, align 4<br>
> + %cmp4 = icmp eq i32 %i.1, %v3<br>
> + %or.cond = or i1 %tobool, %cmp4<br>
> + br i1 %or.cond, label %for.inc20, label %for.inc, !prof !0<br>
> +; CHECK: BB#1: derived from LLVM BB %for.cond2<br>
> +; CHECK: Successors according to CFG: BB#3(28004359) BB#4(1101746182)<br>
> +; CHECK: BB#4: derived from LLVM BB %for.cond2<br>
> +; CHECK: Successors according to CFG: BB#3(56008718) BB#2(2147483647)<br>
> +<br>
> +for.inc: ; preds = %for.cond2<br>
> + %shl = shl i32 %bit.0, 1<br>
> + %inc19 = add nsw i32 %i.1, 1<br>
> + br label %for.cond2<br>
> +<br>
> +for.inc20: ; preds = %for.cond2<br>
> + ret void<br>
> +}<br>
> +<br>
> +!0 = metadata !{metadata !"branch_weights", i32 112017436, i32 -735157296}<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>