<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Oct 17, 2011, at 1:29 AM, Chandler Carruth wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div class="gmail_quote">On Mon, Oct 17, 2011 at 12:32 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
This patch teaches the BranchProbabilityInfo pass to use metadata encoded probabilities when present on branch instructions. Switch instructions still need to be handled.</blockquote><div><br></div><div>And here is a patch which generalizes this logic to support switch instructions. It should be applied on top of the previous patch.</div>
<div><br></div><div>BTW, Chris mentioned an interest in getting this fixed and potentially merged onto the 3.0 release branch, so prompt review would be appreciated. =] </div></div>
<span><use-branch-prob-metadata-switches.patch></span>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote></div><div><br></div>Chandler,<div><br></div><div>Your patches look great to me. I don't suggest any changes. I just have </div><div>some general comments for the record:</div><div><br><div><div>In calcMetaDataWeight, if we really wanted to recover from overflow, we</div><div>would scale back all edge weights (all by the same divisor) until the</div><div>sum is less than UINT32_MAX. But I don't think this is an expected</div><div>case, so your patch should be fine--it's simpler this way, and seems</div><div>like it will handle the output of LowerExpect.</div><div><br></div><div>As you're aware, the biggest problem with the branch weight metadata</div><div>is CFG transforms invalidating it. You've already fixed the obvious</div><div>swapSuccessors issue. That should at least make it usable, but we</div><div>really have no way to verify this stuff currently.</div><div><br></div><div>It's also quite silly for opt to run a LowerExpectIntrinsics</div><div>pass. That's a language-specific front-end implementation hack that</div><div>should be part of the clang driver. Given that we're standardizing on</div><div>metadata for branch weights, we should never see those intrinsics in</div><div>bitcode.</div></div><div><br></div></div><div><br></div><div>-Andy</div></body></html>