<div dir="ltr">Hi Hans<div><br></div><div>Could you please take a look at this patch and see if it is ok to be checked in? Thank you!</div><div><br></div><div><br></div><div>Cong</div></div><div class="gmail_extra"><br clear="all"><div><div class="gmail_signature">thanks,<br>Cong</div></div>
<br><div class="gmail_quote">On Fri, Aug 21, 2015 at 4:47 PM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hans added a comment.<br>
<br>
This looks fine to me, but it would be really nice to get tests here.<br>
<br>
<br>
================<br>
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:1900<br>
@@ -1899,3 +1899,3 @@<br>
<br>
-  addSuccessorWithWeight(SwitchBB, B.Default);<br>
-  addSuccessorWithWeight(SwitchBB, MBB);<br>
+  uint32_t DefaultWeight = getEdgeWeight(SwitchBB, B.Default);<br>
+  addSuccessorWithWeight(SwitchBB, B.Default, DefaultWeight);<br>
----------------<br>
Does it get the DefaultWeight right here? (Not a criticism, just wondering if this ever worked.)<br>
<br>
================<br>
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8019<br>
@@ +8018,3 @@<br>
+        for (auto Succ : JumpMBB->successors())<br>
+          JumpWeight += getEdgeWeight(JumpMBB, Succ);<br>
+        uint64_t FallthruWeight = getEdgeWeight(CurMBB, Fallthrough);<br>
----------------<br>
I think the sum of all the branch weights on a branch (in this case a SwitchInst) will not overflow an uint32_t. Since we're adding up weights from the same switch it should be safe to use uint32_t for JumpWeight and skip the ScaleWeights below.<br>
<br>
<br>
<a href="http://reviews.llvm.org/D12166" rel="noreferrer" target="_blank">http://reviews.llvm.org/D12166</a><br>
<br>
<br>
<br>
</blockquote></div><br></div>