[llvm] r268767 - [SimplifyCFG] propagate branch metadata when creating select (retry r268550 / r268751 with possible fix)

Xinliang David Li via llvm-commits llvm-commits at lists.llvm.org
Fri May 6 15:25:56 PDT 2016


It is likely caused some other existing problems that get triggered by the
change.

David

On Fri, May 6, 2016 at 2:59 PM, Pete Cooper via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Hi Sanjay
>
> I’m seeing a failure on our internal bots which may be caused by this.
> I’m not actually sure how to proceed.  Do you have any idea if this could
> have been caused by your change?  It was the only one in the list that
> looks like it may have done so.
>
> The error is:
>
> /Users/buildslave/jenkins/sharedspace/apple-clang-stage2-RA at 2/clang.roots/BuildRecords/clang-9999.99_install/Objects/obj-llvm/./bin/clang++
>  -DGTEST_HAS_RTTI=0 -D_DEBUG -D__STDC_CONSTANT_MACROS
> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/Target/X86
> -I/Users/buildslave/jenkins/sharedspace/apple-clang-stage2-RA at 2/clang/src/lib/Target/X86
> -Iinclude -I/Users/buildslave/jenkins/sharedspace/apple-clang-stage2-RA at 2/clang/src/include
> -fno-stack-protector -fno-common -Wno-profile-instr-unprofiled -fPIC
> -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings
> -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long
> -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor
> -Werror=date-time -std=c++11 -fcolor-diagnostics -O2 -gline-tables-only
> -arch x86_64 -isysroot
> /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk
> -mmacosx-version-min=10.10   -UNDEBUG
> -fprofile-instr-use=/Users/buildslave/jenkins/sharedspace/apple-clang-stage2-RA at 2/clang.roots/BuildRecords/clang-9999.99_install/Objects/clang.profdata
> -fno-exceptions -fno-rtti -MMD -MT
> lib/Target/X86/CMakeFiles/LLVMX86CodeGen.dir/X86TargetTransformInfo.cpp.o
> -MF
> lib/Target/X86/CMakeFiles/LLVMX86CodeGen.dir/X86TargetTransformInfo.cpp.o.d
> -o
> lib/Target/X86/CMakeFiles/LLVMX86CodeGen.dir/X86TargetTransformInfo.cpp.o
> -c '/Users/buildslave/jenkins/sharedspace/apple-clang-stage2-RA at 2
> /clang/src/lib/Target/X86/X86TargetTransformInfo.cpp'
>
> warning: profile data may be out of date: of 251 functions, 2 have no data
> and 43 have mismatched data that will be ignored
> [-Wprofile-instr-out-of-date]
>
> Assertion failed: (Denominator > 0 && "Denominator cannot be 0!"),
> function BranchProbability, file
> /Users/buildslave/jenkins/sharedspace/apple-clang-stage2-RA at 2/clang/src/lib/Support/BranchProbability.cpp,
> line 38.
>
> Cheers,
> Pete
> > On May 6, 2016, at 11:07 AM, Sanjay Patel via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
> >
> > Author: spatel
> > Date: Fri May  6 13:07:46 2016
> > New Revision: 268767
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=268767&view=rev
> > Log:
> > [SimplifyCFG] propagate branch metadata when creating select (retry
> r268550 / r268751 with possible fix)
> >
> > Retrying r268550/r268751 which were reverted at r268577/r268765 due a
> memory sanitizer failure.
> > I have not been able to reproduce that failure, but I've taken another
> guess at fixing
> > the problem in this version of the patch and will watch for another
> failure.
> >
> > Original commit message:
> > Unlike earlier similar fixes, we need to recalculate the branch weights
> > in this case.
> >
> > Differential Revision: http://reviews.llvm.org/D19674
> >
> >
> > Modified:
> >    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
> >    llvm/trunk/test/Transforms/SimplifyCFG/preserve-branchweights.ll
> >
> > Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=268767&r1=268766&r2=268767&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
> > +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Fri May  6 13:07:46
> 2016
> > @@ -2860,9 +2860,29 @@ static bool SimplifyCondBranchToCondBran
> >     Value *PBIV = PN->getIncomingValue(PBBIdx);
> >     if (BIV != PBIV) {
> >       // Insert a select in PBI to pick the right value.
> > -      Value *NV = cast<SelectInst>
> > +      SelectInst *NV = cast<SelectInst>
> >         (Builder.CreateSelect(PBICond, PBIV, BIV, PBIV->getName() +
> ".mux"));
> >       PN->setIncomingValue(PBBIdx, NV);
> > +      // Although the select has the same condition as PBI, the
> original branch
> > +      // weights for PBI do not apply to the new select because the
> select's
> > +      // 'logical' edges are incoming edges of the phi that is
> eliminated, not
> > +      // the outgoing edges of PBI.
> > +      if (PredHasWeights && SuccHasWeights) {
> > +        uint64_t PredCommon = PBIOp ? PredFalseWeight : PredTrueWeight;
> > +        uint64_t PredOther = PBIOp ? PredTrueWeight : PredFalseWeight;
> > +        uint64_t SuccCommon = BIOp ? SuccFalseWeight : SuccTrueWeight;
> > +        uint64_t SuccOther = BIOp ? SuccTrueWeight : SuccFalseWeight;
> > +        // The weight to PredCommonDest should be PredCommon *
> SuccTotal.
> > +        // The weight to PredOtherDest should be PredOther * SuccCommon.
> > +        uint64_t NewWeights[2] = {PredCommon * (SuccCommon + SuccOther),
> > +                                  PredOther * SuccCommon};
> > +
> > +        FitWeights(NewWeights);
> > +
> > +        NV->setMetadata(LLVMContext::MD_prof,
> > +                        MDBuilder(BI->getContext())
> > +                            .createBranchWeights(NewWeights[0],
> NewWeights[1]));
> > +      }
> >     }
> >   }
> >
> >
> > Modified:
> llvm/trunk/test/Transforms/SimplifyCFG/preserve-branchweights.ll
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/preserve-branchweights.ll?rev=268767&r1=268766&r2=268767&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/test/Transforms/SimplifyCFG/preserve-branchweights.ll
> (original)
> > +++ llvm/trunk/test/Transforms/SimplifyCFG/preserve-branchweights.ll Fri
> May  6 13:07:46 2016
> > @@ -412,22 +412,48 @@ return:
> >   ret i32 %retval.0
> > }
> >
> > -; The 1st select should have branch weights equal to the 1st branch.
> > -; The 2nd select should have freshly calculated branch weights.
> > +; The selects should have freshly calculated branch weights.
> >
> > define i32 @SimplifyCondBranchToCondBranch(i1 %cmpa, i1 %cmpb) {
> > ; CHECK-LABEL: @SimplifyCondBranchToCondBranch(
> > ; CHECK-NEXT:  block1:
> > -; CHECK-NEXT:    [[BRMERGE:%.*]] = or i1 %cmpb, %cmpa
> > -; CHECK-NEXT:    [[DOTMUX:%.*]] = select i1 %cmpb, i32 0, i32 2
> > -; CHECK-NEXT:    [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32
> [[DOTMUX]], i32 1, !prof !12
> > +; CHECK-NEXT:    [[BRMERGE:%.*]] = or i1 %cmpa, %cmpb
> > +; CHECK-NEXT:    [[DOTMUX:%.*]] = select i1 %cmpa, i32 0, i32 2, !prof
> !12
> > +; CHECK-NEXT:    [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32
> [[DOTMUX]], i32 1, !prof !13
> > ; CHECK-NEXT:    ret i32 [[OUTVAL]]
> > ;
> > block1:
> > -  br i1 %cmpb, label %block3, label %block2, !prof !0
> > +  br i1 %cmpa, label %block3, label %block2, !prof !13
> >
> > block2:
> > -  br i1 %cmpa, label %block3, label %exit, !prof !2
> > +  br i1 %cmpb, label %block3, label %exit, !prof !14
> > +
> > +block3:
> > +  %cowval = phi i32 [ 2, %block2 ], [ 0, %block1 ]
> > +  br label %exit
> > +
> > +exit:
> > +  %outval = phi i32 [ %cowval, %block3 ], [ 1, %block2 ]
> > +  ret i32 %outval
> > +}
> > +
> > +; Swap the operands of the compares to verify that the weights update
> correctly.
> > +
> > +define i32 @SimplifyCondBranchToCondBranchSwap(i1 %cmpa, i1 %cmpb) {
> > +; CHECK-LABEL: @SimplifyCondBranchToCondBranchSwap(
> > +; CHECK-NEXT:  block1:
> > +; CHECK-NEXT:    [[CMPA_NOT:%.*]] = xor i1 %cmpa, true
> > +; CHECK-NEXT:    [[CMPB_NOT:%.*]] = xor i1 %cmpb, true
> > +; CHECK-NEXT:    [[BRMERGE:%.*]] = or i1 [[CMPA_NOT]], [[CMPB_NOT]]
> > +; CHECK-NEXT:    [[DOTMUX:%.*]] = select i1 [[CMPA_NOT]], i32 0, i32 2,
> !prof !14
> > +; CHECK-NEXT:    [[OUTVAL:%.*]] = select i1 [[BRMERGE]], i32
> [[DOTMUX]], i32 1, !prof !15
> > +; CHECK-NEXT:    ret i32 [[OUTVAL]]
> > +;
> > +block1:
> > +  br i1 %cmpa, label %block2, label %block3, !prof !13
> > +
> > +block2:
> > +  br i1 %cmpb, label %exit, label %block3, !prof !14
> >
> > block3:
> >   %cowval = phi i32 [ 2, %block2 ], [ 0, %block1 ]
> > @@ -452,6 +478,8 @@ exit:
> > !10 = !{!"branch_weights", i32 672646, i32 21604207}
> > !11 = !{!"branch_weights", i32 6960, i32 21597248}
> > !12 = !{!"these_are_not_the_branch_weights_you_are_looking_for", i32 3,
> i32 5}
> > +!13 = !{!"branch_weights", i32 2, i32 3}
> > +!14 = !{!"branch_weights", i32 4, i32 7}
> >
> > ; CHECK: !0 = !{!"branch_weights", i32 5, i32 11}
> > ; CHECK: !1 = !{!"branch_weights", i32 1, i32 5}
> > @@ -467,5 +495,8 @@ exit:
> > ;; treat the weight as an unsigned integer.
> > ; CHECK: !10 = !{!"branch_weights", i32 112017436, i32 -735157296}
> > ; CHECK: !11 = !{!"branch_weights", i32 3, i32 5}
> > -; CHECK: !12 = !{!"branch_weights", i32 14, i32 10}
> > +; CHECK: !12 = !{!"branch_weights", i32 22, i32 12}
> > +; CHECK: !13 = !{!"branch_weights", i32 34, i32 21}
> > +; CHECK: !14 = !{!"branch_weights", i32 33, i32 14}
> > +; CHECK: !15 = !{!"branch_weights", i32 47, i32 8}
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> 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/20160506/a6cebd81/attachment.html>


More information about the llvm-commits mailing list