<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 11, 2016 at 2:28 PM, Jun Bum Lim <span dir="ltr"><<a href="mailto:junbuml@codeaurora.org" target="_blank">junbuml@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">junbuml updated this revision to Diff 74284.<br>
junbuml added a comment.<br>
<br>
Integrated David's comments. Thanks David for your review with the detail comments.<br>
<br>
Basically, I took the heuristic you suggested Freq(Pred) / Freq(BB) > 2. Please find the detail from the comment in the code.<br>
<br>
Using the latest truck, ran performance test for spec2000/2006 and I didn't see regression even in spec2006.povray. The function applied by this patch is not even hot in my profile so the previous regression I observed must be caused by difference code alignment.<br>
<br>
Regarding inspecting PHIs, for me, it seems not that easy to predict if a PHI ends up with a COPY in CodeGenPrepare, as COPYs might be added when performing deSSA and removed in later passes (e.g., Register Coalescing or Machine Copy Propagation). In this cost heuristic in CGP, I believe it's not unreasonable to see a PHI as one potential COPY.  In the worst case where none of PHIs results in a COPY, the empty BB which is skipped here might end up with only one branch instruction, so it will be removed in BranchFold pass.<br>
<br>
Regarding the multiple empty block case, when there are multiple empty blocks which are used as incoming blocks in the same PHI, we may be able to merge only one of them in most case. That is because of the conflict in incoming values in the PHI if one of them is already merged. I modified the testcase to introduce two empty blocks. In the first test, f_switch(), both two empty blocks are skipped as both of them are unlikely executed. In the second test, f_switch2(),  once the first empty block (<a href="http://sw.bb" rel="noreferrer" target="_blank">sw.bb</a>) is merged, the second block(sw.bb2) cannot be merged because of the conflict in  incoming value from <a href="http://sw.bb" rel="noreferrer" target="_blank">sw.bb</a> which is already merged. If all the incoming values are the same from the multiple empty blocks in the same PHI, then it should be either all merged or skipped, but I doubt if we can see such case in CGP in general. Please let me know if you were mentioning different cases in your previous comment.<br></blockquote><div><br></div><div>What I meant is the the case where if there are more than one empty blocks sharing the same incoming value for the phi, they should be considered together.</div><div><br></div><div>Say we are examining empty block 'BB' with 'Pred' and 'DestBB'.   While examining phis in DestBB, first find the incoming value from 'BB', then it needs to iterate all phi args and collect all empty BBs that share the same incoming value.   Skip merging 'BB' only when none of the BBs in the set has higher frequency than threshold (another way is to check the sum of the frequency of these BBs ).</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">
<br>
Thanks,<br>
Jun<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
<a href="https://reviews.llvm.org/D22696" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D22696</a><br>
<br>
Files:<br>
  lib/CodeGen/CodeGenPrepare.cpp<br>
  test/Transforms/<wbr>CodeGenPrepare/skip-merging-<wbr>case-block.ll<br>
<br>
</div></div></blockquote></div><br></div></div>