<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 14, 2016 at 10:10 PM, Dehao Chen <span dir="ltr"><<a href="mailto:dehao@google.com" target="_blank">dehao@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Thanks for fixing this. But I think the current equation is still incorrect because if ProfileLikelyProb>75, the probability will be > 100%<div><br></div><div>I thinks the right equation is:</div><div><br></div><div>Let's use S to denote Prob(BB->succ), P to denote Prob(BB->pred), L to denote ProfileLikelyProb</div>
<div><br></div><div>S / (S + P * 2) = L</div><div>the new threshold for triangle case = S / (S + P) = L / (2 - L)</div><div><br></div><div>With this, when L is 50%, return threshold = 100 / 150</div><div>when L is 100% return threshold = 100 / 100</div></div></blockquote><div><br></div><div>I am not sure. Branch cost based threshold computation should be independent of value 'L'. On the other hand, how do we honor user specified branch bias is implementation defined, so I would like to have it more intuitive (for >100% case we can simply cap it). Your newly proposed formula maps well to the range of 67% to 100% continuously, but the problem I don't see good intuition -- or you can explain it to me ;)</div><div><br></div><div>thanks,</div><div><br></div><div>David</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span class="HOEnZb"><font color="#888888"><div><br></div><div>Dehao</div></font></span></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 14, 2016 at 8:32 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Thanks!</div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 14, 2016 at 8:12 PM, Xinliang David Li <span dir="ltr"><<a href="mailto:xinliangli@gmail.com" target="_blank">xinliangli@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>Added documentation.</div><div><br></div><div>I actually overlooked an error in the code review. The patch produces the right threshold by default, but when parameter is set to be non-default, the result is not correct. I have corrected it in this patch.</div><span><font color="#888888"><div><br></div><div>David</div><div><br></div><div><br></div></font></span></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 14, 2016 at 7:11 PM, Sean Silva via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Tue, Jun 14, 2016 at 3:27 PM, Dehao Chen via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dehao<br>
Date: Tue Jun 14 17:27:17 2016<br>
New Revision: 272729<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=272729&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=272729&view=rev</a><br>
Log:<br>
Set machine block placement hot prob threshold for both static and runtime profile.<br>
<br>
Summary: With runtime profile, we have more confidence in branch probability, thus during basic block layout, we set a lower hot prob threshold so that blocks can be layouted optimally.<br>
<br>
Reviewers: djasper, davidxl<br>
<br>
Subscribers: llvm-commits<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D20991" rel="noreferrer" target="_blank">http://reviews.llvm.org/D20991</a><br>
<br>
Modified:<br>
llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp<br>
llvm/trunk/lib/CodeGen/MachineBranchProbabilityInfo.cpp<br>
llvm/trunk/test/CodeGen/X86/block-placement.ll<br>
<br>
Modified: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp?rev=272729&r1=272728&r2=272729&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp?rev=272729&r1=272728&r2=272729&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp Tue Jun 14 17:27:17 2016<br>
@@ -125,6 +125,7 @@ BranchFoldPlacement("branch-fold-placeme<br>
cl::init(true), cl::Hidden);<br>
<br>
extern cl::opt<unsigned> StaticLikelyProb;<br>
+extern cl::opt<unsigned> ProfileLikelyProb;<br>
<br>
namespace {<br>
class BlockChain;<br>
@@ -520,13 +521,20 @@ bool MachineBlockPlacement::shouldPredBl<br>
return false;<br>
}<br>
<br>
-// FIXME (PGO handling)<br>
-// For now this method just returns a fixed threshold. It needs to be enhanced<br>
-// such that BB and Succ is passed in so that CFG shapes are examined such that<br>
-// the threshold is computed with more precise cost model when PGO is on.<br>
-static BranchProbability getLayoutSuccessorProbThreshold() {<br>
- BranchProbability HotProb(StaticLikelyProb, 100);<br>
- return HotProb;<br>
+// When profile is not present, return the StaticLikelyProb.<br>
+// When profile is available, we need to handle the triangle-shape CFG.<br>
+static BranchProbability getLayoutSuccessorProbThreshold(<br>
+ MachineBasicBlock *BB) {<br>
+ if (!BB->getParent()->getFunction()->getEntryCount())<br>
+ return BranchProbability(StaticLikelyProb, 100);<br>
+ if (BB->succ_size() == 2) {<br>
+ const MachineBasicBlock *Succ1 = *BB->succ_begin();<br>
+ const MachineBasicBlock *Succ2 = *(BB->succ_begin() + 1);<br>
+ if (Succ1->isSuccessor(Succ2) || Succ2->isSuccessor(Succ1))<br>
+ return BranchProbability(<br>
+ 200 - 2 * ProfileLikelyProb, 200 - ProfileLikelyProb);<br></blockquote><div><br></div></div></div><div>Please comment on how these numbers were derived.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ }<br>
+ return BranchProbability(ProfileLikelyProb, 100);<br>
}<br>
<br>
/// Checks to see if the layout candidate block \p Succ has a better layout<br>
@@ -593,7 +601,7 @@ bool MachineBlockPlacement::hasBetterLay<br>
// edge: Prob(Succ->BB) needs to >= HotProb in order to be selected (without<br>
// profile data).<br>
<br>
- BranchProbability HotProb = getLayoutSuccessorProbThreshold();<br>
+ BranchProbability HotProb = getLayoutSuccessorProbThreshold(BB);<br>
<br>
// Forward checking. For case 2, SuccProb will be 1.<br>
if (SuccProb < HotProb) {<br>
<br>
Modified: llvm/trunk/lib/CodeGen/MachineBranchProbabilityInfo.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBranchProbabilityInfo.cpp?rev=272729&r1=272728&r2=272729&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBranchProbabilityInfo.cpp?rev=272729&r1=272728&r2=272729&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/MachineBranchProbabilityInfo.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/MachineBranchProbabilityInfo.cpp Tue Jun 14 17:27:17 2016<br>
@@ -29,6 +29,12 @@ cl::opt<unsigned> StaticLikelyProb(<br>
cl::desc("branch probability threshold to be considered very likely"),<br>
cl::init(80), cl::Hidden);<br>
<br>
+cl::opt<unsigned> ProfileLikelyProb(<br>
+ "profile-likely-prob",<br>
+ cl::desc("branch probability threshold to be considered very likely "<br>
+ "when profile is available"),<br>
+ cl::init(51), cl::Hidden);<br></blockquote><div><br></div></span><div>Please mention in the description that this is a percentage.</div><div><br></div><div>-- Sean Silva</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
char MachineBranchProbabilityInfo::ID = 0;<br>
<br>
void MachineBranchProbabilityInfo::anchor() {}<br>
<br>
Modified: llvm/trunk/test/CodeGen/X86/block-placement.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/block-placement.ll?rev=272729&r1=272728&r2=272729&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/block-placement.ll?rev=272729&r1=272728&r2=272729&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/CodeGen/X86/block-placement.ll (original)<br>
+++ llvm/trunk/test/CodeGen/X86/block-placement.ll Tue Jun 14 17:27:17 2016<br>
@@ -1207,4 +1207,82 @@ exit:<br>
ret void<br>
}<br>
<br>
+define void @test_hot_branch_profile(i32* %a) !prof !6 {<br>
+; Test that a hot branch that has a probability a little larger than 60% will<br>
+; break CFG constrains when doing block placement when profile is available.<br>
+; CHECK-LABEL: test_hot_branch_profile:<br>
+; CHECK: %entry<br>
+; CHECK: %then<br>
+; CHECK: %exit<br>
+; CHECK: %else<br>
+<br>
+entry:<br>
+ %gep1 = getelementptr i32, i32* %a, i32 1<br>
+ %val1 = load i32, i32* %gep1<br>
+ %cond1 = icmp ugt i32 %val1, 1<br>
+ br i1 %cond1, label %then, label %else, !prof !5<br>
+<br>
+then:<br>
+ call void @hot_function()<br>
+ br label %exit<br>
+<br>
+else:<br>
+ call void @cold_function()<br>
+ br label %exit<br>
+<br>
+exit:<br>
+ call void @hot_function()<br>
+ ret void<br>
+}<br>
+<br>
+define void @test_hot_branch_triangle_profile(i32* %a) !prof !6 {<br>
+; Test that a hot branch that has a probability a little larger than 80% will<br>
+; break triangle shaped CFG constrains when doing block placement if profile<br>
+; is present.<br>
+; CHECK-LABEL: test_hot_branch_triangle_profile:<br>
+; CHECK: %entry<br>
+; CHECK: %exit<br>
+; CHECK: %then<br>
+<br>
+entry:<br>
+ %gep1 = getelementptr i32, i32* %a, i32 1<br>
+ %val1 = load i32, i32* %gep1<br>
+ %cond1 = icmp ugt i32 %val1, 1<br>
+ br i1 %cond1, label %exit, label %then, !prof !5<br>
+<br>
+then:<br>
+ call void @hot_function()<br>
+ br label %exit<br>
+<br>
+exit:<br>
+ call void @hot_function()<br>
+ ret void<br>
+}<br>
+<br>
+define void @test_hot_branch_triangle_profile_topology(i32* %a) !prof !6 {<br>
+; Test that a hot branch that has a probability between 50% and 66% will not<br>
+; break triangle shaped CFG constrains when doing block placement if profile<br>
+; is present.<br>
+; CHECK-LABEL: test_hot_branch_triangle_profile_topology:<br>
+; CHECK: %entry<br>
+; CHECK: %then<br>
+; CHECK: %exit<br>
+<br>
+entry:<br>
+ %gep1 = getelementptr i32, i32* %a, i32 1<br>
+ %val1 = load i32, i32* %gep1<br>
+ %cond1 = icmp ugt i32 %val1, 1<br>
+ br i1 %cond1, label %exit, label %then, !prof !7<br>
+<br>
+then:<br>
+ call void @hot_function()<br>
+ br label %exit<br>
+<br>
+exit:<br>
+ call void @hot_function()<br>
+ ret void<br>
+}<br>
+<br>
!5 = !{!"branch_weights", i32 84, i32 16}<br>
+!6 = !{!"function_entry_count", i32 10}<br>
+!7 = !{!"branch_weights", i32 60, i32 40}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div><br></div></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>