[llvm] r268767 - [SimplifyCFG] propagate branch metadata when creating select (retry r268550 / r268751 with possible fix)
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Fri May 6 15:56:29 PDT 2016
For the record, it looks like this iteration of the patch has made it past
the troubled bot:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/11639
...and after staring at the code, I still don't see how this version could
be logically different than the previous rev.
But I don't think I've hit my personal record for reverts of a single
patch, so if we want to revert this while investigating the assert, I don't
object. :)
On Fri, May 6, 2016 at 4:44 PM, Sanjay Patel <spatel at rotateright.com> wrote:
> Hi Pete,
>
> It certainly seems related to branch weights.
>
> I actually haven't looked at what happens to the metadata downstream from
> here. Do you know if the assertion is triggered in an IR optimizer pass or
> the backend?
>
> On Fri, May 6, 2016 at 4:25 PM, Xinliang David Li <xinliangli at gmail.com>
> wrote:
>
>> 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/9456d887/attachment.html>
More information about the llvm-commits
mailing list