[llvm] r304453 - [Profile] Fix builtin_expect lowering bug

Dimitry Andric via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 2 14:24:12 PDT 2017


Thanks David, that was r304489, right?  Indeed, it fixes the assertion.

-Dimitry

> On 2 Jun 2017, at 18:37, Xinliang David Li <davidxl at google.com> wrote:
> 
> I already committed a fix. Did you still see the failure?
> 
> David
> 
> On Fri, Jun 2, 2017 at 5:33 AM, Dimitry Andric <dimitry at andric.com <mailto:dimitry at andric.com>> wrote:
> Minimized test case:
> 
> /* clang -cc1 -triple x86_64 -S -O1 test-int128.c */
> __int128 a;
> void c();
> void b() {
>   if (a == -1)
>     c();
> }
> 
> Results in:
> 
> Assertion failed: (getActiveBits() <= 64 && "Too many bits for uint64_t"), function getZExtValue, file include/llvm/ADT/APInt.h, line 1526.
> 
> Program received signal SIGABRT, Aborted.
> 0x00000008046815da in thr_kill () from /lib/libc.so.7
> (gdb) bt
> #0  0x00000008046815da in thr_kill () from /lib/libc.so.7
> #1  0x00000008046815a4 in raise () from /lib/libc.so.7
> #2  0x0000000804681519 in abort () from /lib/libc.so.7
> #3  0x00000008046fd371 in __assert () from /lib/libc.so.7
> #4  0x000000000112cce0 in lowerExpectIntrinsic(llvm::Function&) ()
> #5  0x0000000000e41b8a in llvm::FPPassManager::runOnFunction(llvm::Function&) ()
> #6  0x0000000000e41319 in llvm::legacy::FunctionPassManagerImpl::run(llvm::Function&) ()
> #7  0x0000000000e4119c in llvm::legacy::FunctionPassManager::run(llvm::Function&) ()
> #8  0x00000000013d0dac in clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions const&, clang::CodeGenOptions const&, clang::TargetOptions const&, clang::LangOptions const&, llvm::DataLayout const&, llvm::Module*, clang::BackendAction, std::__1::unique_ptr<llvm::raw_pwrite_stream, std::__1::default_delete<llvm::raw_pwrite_stream> >) ()
> #9  0x0000000001a7c2ca in clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) ()
> #10 0x0000000001d2cbe2 in clang::ParseAST(clang::Sema&, bool, bool) ()
> #11 0x000000000175d99c in clang::FrontendAction::Execute() ()
> #12 0x0000000001725c31 in clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) ()
> #13 0x00000000017dadb3 in clang::ExecuteCompilerInvocation(clang::CompilerInstance*) ()
> #14 0x0000000000721972 in cc1_main(llvm::ArrayRef<char const*>, char const*, void*) ()
> #15 0x000000000071fbc9 in main ()
> 
> -Dimitry
> 
> > On 2 Jun 2017, at 00:59, Xinliang David Li via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> >
> > working on it.
> >
> > On Thu, Jun 1, 2017 at 3:37 PM, Vedant Kumar <vsk at apple.com <mailto:vsk at apple.com>> wrote:
> > Hi David,
> >
> > I think this commit is causing a failure in the compiler-rt tests:
> > http://green.lab.llvm.org/green//job/clang-stage1-cmake-RA-expensive/6811 <http://green.lab.llvm.org/green//job/clang-stage1-cmake-RA-expensive/6811>
> >
> > Could you take a look?
> >
> > thanks,
> > vedant
> >
> >> On Jun 1, 2017, at 12:05 PM, Xinliang David Li via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> >>
> >> Author: davidxl
> >> Date: Thu Jun  1 14:05:55 2017
> >> New Revision: 304453
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=304453&view=rev <http://llvm.org/viewvc/llvm-project?rev=304453&view=rev>
> >> Log:
> >> [Profile] Fix builtin_expect lowering bug
> >>
> >> The lowerer wrongly assumes the ICMP instruction
> >> 1) always has a constant operand;
> >> 2) the operand has value 0.
> >>
> >> It also assumes the expected value can only be one, thus
> >> other values other than one will be considered 'zero'.
> >>
> >> This leads to wrong profile annotation when other integer values
> >> are used other than 0, 1 in the comparison or in the expect intrinsic.
> >>
> >> Also missing is handling of equal predicate.
> >>
> >> This patch fixes all the above problems.
> >>
> >> Differential Revision: http://reviews.llvm.org/D33757 <http://reviews.llvm.org/D33757>
> >>
> >> Added:
> >>    llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
> >> Modified:
> >>    llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
> >>
> >> Modified: llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp?rev=304453&r1=304452&r2=304453&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp?rev=304453&r1=304452&r2=304453&view=diff>
> >> ==============================================================================
> >> --- llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp (original)
> >> +++ llvm/trunk/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp Thu Jun  1 14:05:55 2017
> >> @@ -98,11 +98,20 @@ template <class BrSelInst> static bool h
> >>   CallInst *CI;
> >>
> >>   ICmpInst *CmpI = dyn_cast<ICmpInst>(BSI.getCondition());
> >> +  CmpInst::Predicate Predicate;
> >> +  uint64_t ValueComparedTo = 0;
> >>   if (!CmpI) {
> >>     CI = dyn_cast<CallInst>(BSI.getCondition());
> >> +    Predicate = CmpInst::ICMP_NE;
> >> +    ValueComparedTo = 0;
> >>   } else {
> >> -    if (CmpI->getPredicate() != CmpInst::ICMP_NE)
> >> +    Predicate = CmpI->getPredicate();
> >> +    if (Predicate != CmpInst::ICMP_NE && Predicate != CmpInst::ICMP_EQ)
> >>       return false;
> >> +    ConstantInt *CmpConstOperand = dyn_cast<ConstantInt>(CmpI->getOperand(1));
> >> +    if (!CmpConstOperand)
> >> +      return false;
> >> +    ValueComparedTo = CmpConstOperand->getZExtValue();
> >>     CI = dyn_cast<CallInst>(CmpI->getOperand(0));
> >>   }
> >>
> >> @@ -121,9 +130,8 @@ template <class BrSelInst> static bool h
> >>   MDBuilder MDB(CI->getContext());
> >>   MDNode *Node;
> >>
> >> -  // If expect value is equal to 1 it means that we are more likely to take
> >> -  // branch 0, in other case more likely is branch 1.
> >> -  if (ExpectedValue->isOne())
> >> +  if ((ExpectedValue->getZExtValue() == ValueComparedTo) ==
> >> +      (Predicate == CmpInst::ICMP_EQ))
> >>     Node = MDB.createBranchWeights(LikelyBranchWeight, UnlikelyBranchWeight);
> >>   else
> >>     Node = MDB.createBranchWeights(UnlikelyBranchWeight, LikelyBranchWeight);
> >>
> >> Added: llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll
> >> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll?rev=304453&view=auto <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll?rev=304453&view=auto>
> >> ==============================================================================
> >> --- llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll (added)
> >> +++ llvm/trunk/test/Transforms/LowerExpectIntrinsic/expect_nonboolean.ll Thu Jun  1 14:05:55 2017
> >> @@ -0,0 +1,104 @@
> >> +; RUN: opt -lower-expect  -S -o - < %s | FileCheck %s
> >> +; RUN: opt -S -passes='function(lower-expect)' < %s | FileCheck %s
> >> +
> >> +define i32 @foo(i32 %arg) #0 {
> >> +; CHECK-LABEL: @foo(i32{{.*}})
> >> +bb:
> >> +  %tmp = sext i32 %arg to i64
> >> +  %tmp1 = call i64 @llvm.expect.i64(i64 %tmp, i64 4)
> >> +  %tmp2 = icmp ne i64 %tmp1, 0
> >> +  br i1 %tmp2, label %bb3, label %bb5
> >> +; CHECK: br i1 %tmp2{{.*}}!prof [[LIKELY:![0-9]+]]
> >> +
> >> +bb3:                                              ; preds = %bb
> >> +  %tmp4 = call i32 (...) @bar()
> >> +  br label %bb5
> >> +
> >> +bb5:                                              ; preds = %bb3, %bb
> >> +  ret i32 1
> >> +}
> >> +
> >> +define i32 @foo2(i32 %arg) #0 {
> >> +; CHECK-LABEL: @foo2
> >> +bb:
> >> +  %tmp = sext i32 %arg to i64
> >> +  %tmp1 = call i64 @llvm.expect.i64(i64 %tmp, i64 4)
> >> +  %tmp2 = icmp eq i64 %tmp1, 2
> >> +  br i1 %tmp2, label %bb3, label %bb5
> >> +; CHECK: br i1 %tmp2{{.*}}!prof [[UNLIKELY:![0-9]+]]
> >> +
> >> +bb3:                                              ; preds = %bb
> >> +  %tmp4 = call i32 (...) @bar()
> >> +  br label %bb5
> >> +
> >> +bb5:                                              ; preds = %bb3, %bb
> >> +  ret i32 1
> >> +}
> >> +
> >> +define i32 @foo3(i32 %arg) #0 {
> >> +; CHECK-LABEL: @foo3
> >> +bb:
> >> +  %tmp = sext i32 %arg to i64
> >> +  %tmp1 = call i64 @llvm.expect.i64(i64 %tmp, i64 4)
> >> +  %tmp2 = icmp eq i64 %tmp1, 4
> >> +  br i1 %tmp2, label %bb3, label %bb5
> >> +; CHECK: br i1 %tmp2{{.*}}!prof [[LIKELY]]
> >> +
> >> +bb3:                                              ; preds = %bb
> >> +  %tmp4 = call i32 (...) @bar()
> >> +  br label %bb5
> >> +
> >> +bb5:                                              ; preds = %bb3, %bb
> >> +  ret i32 1
> >> +}
> >> +
> >> +define i32 @foo4(i32 %arg) #0 {
> >> +; CHECK-LABEL: @foo4
> >> +bb:
> >> +  %tmp = sext i32 %arg to i64
> >> +  %tmp1 = call i64 @llvm.expect.i64(i64 %tmp, i64 4)
> >> +  %tmp2 = icmp ne i64 %tmp1, 2
> >> +  br i1 %tmp2, label %bb3, label %bb5
> >> +; CHECK: br i1 %tmp2{{.*}}!prof [[LIKELY]]
> >> +
> >> +bb3:                                              ; preds = %bb
> >> +  %tmp4 = call i32 (...) @bar()
> >> +  br label %bb5
> >> +
> >> +bb5:                                              ; preds = %bb3, %bb
> >> +  ret i32 1
> >> +}
> >> +
> >> +define i32 @foo5(i32 %arg, i32 %arg1) #0 {
> >> +; CHECK-LABEL: @foo5
> >> +bb:
> >> +  %tmp = sext i32 %arg1 to i64
> >> +  %tmp2 = call i64 @llvm.expect.i64(i64 %tmp, i64 4)
> >> +  %tmp3 = sext i32 %arg to i64
> >> +  %tmp4 = icmp ne i64 %tmp2, %tmp3
> >> +  br i1 %tmp4, label %bb5, label %bb7
> >> +; CHECK-NOT: !prof
> >> +
> >> +bb5:                                              ; preds = %bb
> >> +  %tmp6 = call i32 (...) @bar()
> >> +  br label %bb7
> >> +
> >> +bb7:                                              ; preds = %bb5, %bb
> >> +  ret i32 1
> >> +}
> >> +
> >> +declare i64 @llvm.expect.i64(i64, i64) #1
> >> +
> >> +declare i32 @bar(...) local_unnamed_addr #0
> >> +
> >> +attributes #0 = { nounwind uwtable }
> >> +attributes #1 = { nounwind readnone }
> >> +
> >> +!llvm.module.flags = !{!0}
> >> +!llvm.ident = !{!1}
> >> +
> >> +!0 = !{i32 1, !"wchar_size", i32 4}
> >> +!1 = !{!"clang version 5.0.0 (trunk 304373)"}
> >> +; CHECK: [[LIKELY]] = !{!"branch_weights", i32 2000, i32 1}
> >> +; CHECK: [[UNLIKELY]] = !{!"branch_weights", i32 1, i32 2000}
> >> +
> >>
> >>
> >> _______________________________________________
> >> llvm-commits mailing list
> >> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170602/9a31769c/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: Message signed with OpenPGP
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170602/9a31769c/attachment.sig>


More information about the llvm-commits mailing list