<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi David,<div class=""><br class=""></div><div class="">I think this commit is causing a failure in the compiler-rt tests:</div><div class=""><a href="http://green.lab.llvm.org/green//job/clang-stage1-cmake-RA-expensive/6811" class="">http://green.lab.llvm.org/green//job/clang-stage1-cmake-RA-expensive/6811</a></div><div class=""><br class=""></div><div class="">Could you take a look?</div><div class=""><br class=""></div><div class="">thanks,</div><div class="">vedant</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jun 1, 2017, at 12:05 PM, Xinliang David Li via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Author: davidxl<br class="">Date: Thu Jun 1 14:05:55 2017<br class="">New Revision: 304453<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=304453&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=304453&view=rev</a><br class="">Log:<br class="">[Profile] Fix builtin_expect lowering bug<br class=""><br class="">The lowerer wrongly assumes the ICMP instruction <br class=""> 1) always has a constant operand;<br class=""> 2) the operand has value 0.<br class=""><br class="">It also assumes the expected value can only be one, thus<br class="">other values other than one will be considered 'zero'.<br class=""><br class="">This leads to wrong profile annotation when other integer values<br class="">are used other than 0, 1 in the comparison or in the expect intrinsic.<br class=""><br class="">Also missing is handling of equal predicate.<br class=""><br class="">This patch fixes all the above problems.<br class=""><br class="">Differential Revision: <a href="http://reviews.llvm.org/D33757" class="">http://reviews.llvm.org/D33757</a><br class=""><br class="">Added:<br class=""> llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll<br class="">Modified:<br class=""> llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp<br class=""><br class="">Modified: llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp?rev=304453&r1=304452&r2=304453&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp?rev=304453&r1=304452&r2=304453&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp (original)<br class="">+++ llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp Thu Jun 1 14:05:55 2017<br class="">@@ -98,11 +98,20 @@ template <class BrSelInst> static bool h<br class=""> CallInst *CI;<br class=""><br class=""> ICmpInst *CmpI = dyn_cast<ICmpInst>(BSI.getCondition());<br class="">+ CmpInst::Predicate Predicate;<br class="">+ uint64_t ValueComparedTo = 0;<br class=""> if (!CmpI) {<br class=""> CI = dyn_cast<CallInst>(BSI.getCondition());<br class="">+ Predicate = CmpInst::ICMP_NE;<br class="">+ ValueComparedTo = 0;<br class=""> } else {<br class="">- if (CmpI->getPredicate() != CmpInst::ICMP_NE)<br class="">+ Predicate = CmpI->getPredicate();<br class="">+ if (Predicate != CmpInst::ICMP_NE && Predicate != CmpInst::ICMP_EQ)<br class=""> return false;<br class="">+ ConstantInt *CmpConstOperand = dyn_cast<ConstantInt>(CmpI->getOperand(1));<br class="">+ if (!CmpConstOperand)<br class="">+ return false;<br class="">+ ValueComparedTo = CmpConstOperand->getZExtValue();<br class=""> CI = dyn_cast<CallInst>(CmpI->getOperand(0));<br class=""> }<br class=""><br class="">@@ -121,9 +130,8 @@ template <class BrSelInst> static bool h<br class=""> MDBuilder MDB(CI->getContext());<br class=""> MDNode *Node;<br class=""><br class="">- // If expect value is equal to 1 it means that we are more likely to take<br class="">- // branch 0, in other case more likely is branch 1.<br class="">- if (ExpectedValue->isOne())<br class="">+ if ((ExpectedValue->getZExtValue() == ValueComparedTo) ==<br class="">+ (Predicate == CmpInst::ICMP_EQ))<br class=""> Node = MDB.createBranchWeights(LikelyBranchWeight, UnlikelyBranchWeight);<br class=""> else<br class=""> Node = MDB.createBranchWeights(UnlikelyBranchWeight, LikelyBranchWeight);<br class=""><br class="">Added: llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll<br class="">URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll?rev=304453&view=auto" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll?rev=304453&view=auto</a><br class="">==============================================================================<br class="">--- llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll (added)<br class="">+++ llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll Thu Jun 1 14:05:55 2017<br class="">@@ -0,0 +1,104 @@<br class="">+; RUN: opt -lower-expect -S -o - < %s | FileCheck %s<br class="">+; RUN: opt -S -passes='function(lower-expect)' < %s | FileCheck %s<br class="">+<br class="">+define i32 @foo(i32 %arg) #0 {<br class="">+; CHECK-LABEL: @foo(i32{{.*}})<br class="">+bb:<br class="">+ %tmp = sext i32 %arg to i64<br class="">+ %tmp1 = call i64 @llvm.expect.i64(i64 %tmp, i64 4)<br class="">+ %tmp2 = icmp ne i64 %tmp1, 0<br class="">+ br i1 %tmp2, label %bb3, label %bb5<br class="">+; CHECK: br i1 %tmp2{{.*}}!prof [[LIKELY:![0-9]+]]<br class="">+<br class="">+bb3: ; preds = %bb<br class="">+ %tmp4 = call i32 (...) @bar()<br class="">+ br label %bb5<br class="">+<br class="">+bb5: ; preds = %bb3, %bb<br class="">+ ret i32 1<br class="">+}<br class="">+<br class="">+define i32 @foo2(i32 %arg) #0 {<br class="">+; CHECK-LABEL: @foo2<br class="">+bb:<br class="">+ %tmp = sext i32 %arg to i64<br class="">+ %tmp1 = call i64 @llvm.expect.i64(i64 %tmp, i64 4)<br class="">+ %tmp2 = icmp eq i64 %tmp1, 2<br class="">+ br i1 %tmp2, label %bb3, label %bb5<br class="">+; CHECK: br i1 %tmp2{{.*}}!prof [[UNLIKELY:![0-9]+]]<br class="">+<br class="">+bb3: ; preds = %bb<br class="">+ %tmp4 = call i32 (...) @bar()<br class="">+ br label %bb5<br class="">+<br class="">+bb5: ; preds = %bb3, %bb<br class="">+ ret i32 1<br class="">+}<br class="">+<br class="">+define i32 @foo3(i32 %arg) #0 {<br class="">+; CHECK-LABEL: @foo3<br class="">+bb:<br class="">+ %tmp = sext i32 %arg to i64<br class="">+ %tmp1 = call i64 @llvm.expect.i64(i64 %tmp, i64 4)<br class="">+ %tmp2 = icmp eq i64 %tmp1, 4<br class="">+ br i1 %tmp2, label %bb3, label %bb5<br class="">+; CHECK: br i1 %tmp2{{.*}}!prof [[LIKELY]]<br class="">+<br class="">+bb3: ; preds = %bb<br class="">+ %tmp4 = call i32 (...) @bar()<br class="">+ br label %bb5<br class="">+<br class="">+bb5: ; preds = %bb3, %bb<br class="">+ ret i32 1<br class="">+}<br class="">+<br class="">+define i32 @foo4(i32 %arg) #0 {<br class="">+; CHECK-LABEL: @foo4<br class="">+bb:<br class="">+ %tmp = sext i32 %arg to i64<br class="">+ %tmp1 = call i64 @llvm.expect.i64(i64 %tmp, i64 4)<br class="">+ %tmp2 = icmp ne i64 %tmp1, 2<br class="">+ br i1 %tmp2, label %bb3, label %bb5<br class="">+; CHECK: br i1 %tmp2{{.*}}!prof [[LIKELY]]<br class="">+<br class="">+bb3: ; preds = %bb<br class="">+ %tmp4 = call i32 (...) @bar()<br class="">+ br label %bb5<br class="">+<br class="">+bb5: ; preds = %bb3, %bb<br class="">+ ret i32 1<br class="">+}<br class="">+<br class="">+define i32 @foo5(i32 %arg, i32 %arg1) #0 {<br class="">+; CHECK-LABEL: @foo5<br class="">+bb:<br class="">+ %tmp = sext i32 %arg1 to i64<br class="">+ %tmp2 = call i64 @llvm.expect.i64(i64 %tmp, i64 4)<br class="">+ %tmp3 = sext i32 %arg to i64<br class="">+ %tmp4 = icmp ne i64 %tmp2, %tmp3<br class="">+ br i1 %tmp4, label %bb5, label %bb7<br class="">+; CHECK-NOT: !prof<br class="">+<br class="">+bb5: ; preds = %bb<br class="">+ %tmp6 = call i32 (...) @bar()<br class="">+ br label %bb7<br class="">+<br class="">+bb7: ; preds = %bb5, %bb<br class="">+ ret i32 1<br class="">+}<br class="">+<br class="">+declare i64 @llvm.expect.i64(i64, i64) #1<br class="">+<br class="">+declare i32 @bar(...) local_unnamed_addr #0<br class="">+<br class="">+attributes #0 = { nounwind uwtable }<br class="">+attributes #1 = { nounwind readnone }<br class="">+<br class="">+!llvm.module.flags = !{!0}<br class="">+!llvm.ident = !{!1}<br class="">+<br class="">+!0 = !{i32 1, !"wchar_size", i32 4}<br class="">+!1 = !{!"clang version 5.0.0 (trunk 304373)"}<br class="">+; CHECK: [[LIKELY]] = !{!"branch_weights", i32 2000, i32 1}<br class="">+; CHECK: [[UNLIKELY]] = !{!"branch_weights", i32 1, i32 2000}<br class="">+<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class=""></div></div></blockquote></div><br class=""></div></body></html>