diff --git a/clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp b/clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp --- a/clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp +++ b/clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp @@ -18,7 +18,7 @@ bool g() { // CHECK-LABEL: define{{.*}} zeroext i1 @_Z1gv - // CHECK: br {{.*}} !prof !8 + // CHECK: br {{.*}} !prof !9 if (b) [[unlikely]] { return A(); @@ -29,7 +29,7 @@ bool h() { // CHECK-LABEL: define{{.*}} zeroext i1 @_Z1hv - // CHECK: br {{.*}} !prof !8 + // CHECK: br {{.*}} !prof !9 if (b) [[unlikely]] return A(); @@ -38,7 +38,7 @@ void NullStmt() { // CHECK-LABEL: define{{.*}}NullStmt - // CHECK: br {{.*}} !prof !8 + // CHECK: br {{.*}} !prof !9 if (b) [[unlikely]]; else { @@ -49,7 +49,7 @@ void IfStmt() { // CHECK-LABEL: define{{.*}}IfStmt - // CHECK: br {{.*}} !prof !8 + // CHECK: br {{.*}} !prof !9 if (b) [[unlikely]] if (B()) {} @@ -63,7 +63,7 @@ void WhileStmt() { // CHECK-LABEL: define{{.*}}WhileStmt - // CHECK: br {{.*}} !prof !8 + // CHECK: br {{.*}} !prof !9 if (b) [[unlikely]] while (B()) {} @@ -76,7 +76,7 @@ void DoStmt() { // CHECK-LABEL: define{{.*}}DoStmt - // CHECK: br {{.*}} !prof !8 + // CHECK: br {{.*}} !prof !9 if (b) [[unlikely]] do {} while (B()) @@ -91,7 +91,7 @@ void ForStmt() { // CHECK-LABEL: define{{.*}}ForStmt - // CHECK: br {{.*}} !prof !8 + // CHECK: br {{.*}} !prof !9 if (b) [[unlikely]] for (; B();) {} @@ -104,7 +104,7 @@ void GotoStmt() { // CHECK-LABEL: define{{.*}}GotoStmt - // CHECK: br {{.*}} !prof !8 + // CHECK: br {{.*}} !prof !9 if (b) [[unlikely]] goto end; else { @@ -116,7 +116,7 @@ void ReturnStmt() { // CHECK-LABEL: define{{.*}}ReturnStmt - // CHECK: br {{.*}} !prof !8 + // CHECK: br {{.*}} !prof !9 if (b) [[unlikely]] return; else { @@ -127,7 +127,7 @@ void SwitchStmt() { // CHECK-LABEL: define{{.*}}SwitchStmt - // CHECK: br {{.*}} !prof !8 + // CHECK: br {{.*}} !prof !9 if (b) [[unlikely]] switch (i) {} else { @@ -145,4 +145,4 @@ } // CHECK: !7 = !{!"branch_weights", i32 [[UNLIKELY]], i32 [[LIKELY]]} -// CHECK: !8 = !{!"branch_weights", i32 [[LIKELY]], i32 [[UNLIKELY]]} +// CHECK: !9 = !{!"branch_weights", i32 [[LIKELY]], i32 [[UNLIKELY]]} diff --git a/clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp b/clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp --- a/clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp +++ b/clang/test/CodeGenCXX/attr-likelihood-if-vs-builtin-expect.cpp @@ -9,7 +9,7 @@ void ab1(int &i) { // CHECK-LABEL: define{{.*}}ab1 - // CHECK: br {{.*}} !prof [[BW_LIKELY:!.+]] + // CHECK: br {{.*}} !prof [[BW_LIKELY:!.+]], // CHECK: br {{.*}} !prof [[BW_LIKELY]] // CHECK: br {{.*}} !prof [[BW_LIKELY]] if (__builtin_expect(a() && b() && a(), 1)) { @@ -36,7 +36,7 @@ // CHECK: br {{.*}}end{{$}} // CHECK: br {{.*}}end{{$}} // CHECK: br {{.*}}end{{$}} - // CHECK: br {{.*}} !prof [[BW_UNLIKELY:!.+]] + // CHECK: br {{.*}} !prof [[BW_UNLIKELY:!.+]], if (__builtin_expect(a() && b() && c(), 0)) { ++i; } else { diff --git a/llvm/include/llvm/IR/FixedMetadataKinds.def b/llvm/include/llvm/IR/FixedMetadataKinds.def --- a/llvm/include/llvm/IR/FixedMetadataKinds.def +++ b/llvm/include/llvm/IR/FixedMetadataKinds.def @@ -47,3 +47,4 @@ LLVM_FIXED_MD_KIND(MD_exclude, "exclude", 33) LLVM_FIXED_MD_KIND(MD_memprof, "memprof", 34) LLVM_FIXED_MD_KIND(MD_callsite, "callsite", 35) +LLVM_FIXED_MD_KIND(MD_expected, "expected", 36) diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp --- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp +++ b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp @@ -280,7 +280,8 @@ NewCS->setAttributes(AttributeList::get(F->getContext(), CallPAL.getFnAttrs(), CallPAL.getRetAttrs(), ArgAttrVec)); - NewCS->copyMetadata(CB, {LLVMContext::MD_prof, LLVMContext::MD_dbg}); + NewCS->copyMetadata(CB, {LLVMContext::MD_prof, LLVMContext::MD_expected, + LLVMContext::MD_dbg}); Args.clear(); ArgAttrVec.clear(); diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -2675,7 +2675,9 @@ } // Copy over various properties and the new attributes. - NewCB->copyMetadata(*OldCB, {LLVMContext::MD_prof, LLVMContext::MD_dbg}); + NewCB->copyMetadata(*OldCB, + {LLVMContext::MD_prof, LLVMContext::MD_expected, + LLVMContext::MD_dbg}); NewCB->setCallingConv(OldCB->getCallingConv()); NewCB->takeName(OldCB); NewCB->setAttributes(AttributeList::get( diff --git a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp --- a/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp +++ b/llvm/lib/Transforms/IPO/DeadArgumentElimination.cpp @@ -205,7 +205,8 @@ } NewCB->setCallingConv(CB->getCallingConv()); NewCB->setAttributes(PAL); - NewCB->copyMetadata(*CB, {LLVMContext::MD_prof, LLVMContext::MD_dbg}); + NewCB->copyMetadata(*CB, {LLVMContext::MD_prof, LLVMContext::MD_expected, + LLVMContext::MD_dbg}); Args.clear(); @@ -936,7 +937,8 @@ } NewCB->setCallingConv(CB.getCallingConv()); NewCB->setAttributes(NewCallPAL); - NewCB->copyMetadata(CB, {LLVMContext::MD_prof, LLVMContext::MD_dbg}); + NewCB->copyMetadata(CB, {LLVMContext::MD_prof, LLVMContext::MD_expected, + LLVMContext::MD_dbg}); Args.clear(); ArgAttrVec.clear(); diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp --- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp +++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp @@ -770,6 +770,8 @@ BranchInst *NewBr = BranchInst::Create(Then, Else, OffsetInRange); NewBr->setMetadata(LLVMContext::MD_prof, Br->getMetadata(LLVMContext::MD_prof)); + NewBr->setMetadata(LLVMContext::MD_expected, + Br->getMetadata(LLVMContext::MD_expected)); ReplaceInstWithInst(InitialBB->getTerminator(), NewBr); // Update phis in Else resulting from InitialBB being split diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -3437,7 +3437,8 @@ NewCall->setAttributes(NewCallerPAL); // Preserve prof metadata if any. - NewCall->copyMetadata(*Caller, {LLVMContext::MD_prof}); + NewCall->copyMetadata(*Caller, + {LLVMContext::MD_prof, LLVMContext::MD_expected}); // Insert a cast of the return type as necessary. Instruction *NC = NewCall; diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp --- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp @@ -509,6 +509,7 @@ case LLVMContext::MD_dbg: case LLVMContext::MD_tbaa: case LLVMContext::MD_prof: + case LLVMContext::MD_expected: case LLVMContext::MD_fpmath: case LLVMContext::MD_tbaa_struct: case LLVMContext::MD_alias_scope: diff --git a/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp b/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp --- a/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp +++ b/llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp @@ -107,6 +107,8 @@ SI.setMetadata(LLVMContext::MD_prof, MDBuilder(CI->getContext()).createBranchWeights(Weights)); + SI.setMetadata(LLVMContext::MD_expected, MDNode::get(CI->getContext(), None)); + return true; } @@ -255,6 +257,8 @@ BI->setMetadata(LLVMContext::MD_prof, MDB.createBranchWeights(UnlikelyBranchWeightVal, LikelyBranchWeightVal)); + BI->setMetadata(LLVMContext::MD_expected, + MDNode::get(BI->getContext(), None)); } } @@ -336,6 +340,8 @@ misexpect::checkFrontendInstrumentation(BSI, ExpectedWeights); BSI.setMetadata(LLVMContext::MD_prof, Node); + BSI.setMetadata(LLVMContext::MD_expected, + MDNode::get(BSI.getContext(), None)); return true; } diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp --- a/llvm/lib/Transforms/Utils/Local.cpp +++ b/llvm/lib/Transforms/Utils/Local.cpp @@ -317,6 +317,8 @@ MDBuilder(BB->getContext()). createBranchWeights(SICase->getValue().getZExtValue(), SIDef->getValue().getZExtValue())); + NewBr->setMetadata(LLVMContext::MD_expected, + SI->getMetadata(LLVMContext::MD_expected)); } // Update make.implicit metadata to the newly-created conditional branch. @@ -2634,6 +2636,7 @@ case LLVMContext::MD_dbg: case LLVMContext::MD_tbaa: case LLVMContext::MD_prof: + case LLVMContext::MD_expected: case LLVMContext::MD_fpmath: case LLVMContext::MD_tbaa_struct: case LLVMContext::MD_invariant_load: diff --git a/llvm/lib/Transforms/Utils/MisExpect.cpp b/llvm/lib/Transforms/Utils/MisExpect.cpp --- a/llvm/lib/Transforms/Utils/MisExpect.cpp +++ b/llvm/lib/Transforms/Utils/MisExpect.cpp @@ -58,9 +58,10 @@ cl::desc("Use this option to turn on/off " "warnings about incorrect usage of llvm.expect intrinsics.")); +// Command line option for setting the diagnostic tolerance threshold static cl::opt MisExpectTolerance( "misexpect-tolerance", cl::init(0), - cl::desc("Prevents emiting diagnostics when profile counts are " + cl::desc("Prevents emitting diagnostics when profile counts are " "within N% of the threshold..")); } // namespace llvm @@ -90,7 +91,6 @@ // improve diagnostic output, such as the caret. If the same problem exists // for branch instructions, then we should remove this function and directly // use the instruction - // else if (auto *S = dyn_cast(I)) { Ret = dyn_cast(S->getCondition()); } @@ -158,15 +158,14 @@ uint64_t TotalBranchWeight = LikelyBranchWeight + (UnlikelyBranchWeight * NumUnlikelyTargets); - // FIXME: When we've addressed sample profiling, restore the assertion - // - // We cannot calculate branch probability if either of these invariants aren't - // met. However, MisExpect diagnostics should not prevent code from compiling, - // so we simply forgo emitting diagnostics here, and return early. - // assert((TotalBranchWeight >= LikelyBranchWeight) && (TotalBranchWeight > 0) - // && "TotalBranchWeight is less than the Likely branch weight"); - if ((TotalBranchWeight == 0) || (TotalBranchWeight <= LikelyBranchWeight)) - return; + // Failing this assert means that we've either got corrupted metadata, that + // we're checking stale or manually inserted branch weights, or that branch + // weights are being added multiple times, as is the case for SampleProfiling + // under ThinLTO. We gate all known entry paths to verifyMisExpect() by first + // checking for the presence of the MD_expected metadata, which is *only* + // added in the LowerExpectIntrinsic Pass, avoiding the assert. + assert((TotalBranchWeight >= LikelyBranchWeight) && (TotalBranchWeight > 0) && + "TotalBranchWeight is less than the Likely branch weight"); // To determine our threshold value we need to obtain the branch probability // for the weights added by llvm.expect and use that proportion to calculate @@ -193,6 +192,14 @@ void checkBackendInstrumentation(Instruction &I, const ArrayRef RealWeights) { + if (!I.getMetadata(LLVMContext::MD_expected)) + return; + // TODO consider writing a small pass that will remove all MD_expected + // metadata that we can run at after the PGO and SampleProfiling passes + // + // For now, since we're doing the misexpect check here, drop the MD_expected + // metadata + I.setMetadata(LLVMContext::MD_expected, nullptr); SmallVector ExpectedWeights; if (!extractBranchWeights(I, ExpectedWeights)) return; diff --git a/llvm/test/Transforms/LowerExpectIntrinsic/basic.ll b/llvm/test/Transforms/LowerExpectIntrinsic/basic.ll --- a/llvm/test/Transforms/LowerExpectIntrinsic/basic.ll +++ b/llvm/test/Transforms/LowerExpectIntrinsic/basic.ll @@ -138,7 +138,7 @@ %conv1 = sext i32 %conv to i64 %expval = call i64 @llvm.expect.i64(i64 %conv1, i64 0) %tobool = icmp ne i64 %expval, 0 -; CHECK: !prof !1 +; CHECK: !prof !2 ; CHECK-NOT: @llvm.expect br i1 %tobool, label %if.then, label %if.end @@ -165,7 +165,7 @@ %tmp = load i32, i32* %x.addr, align 4 %conv = sext i32 %tmp to i64 %expval = call i64 @llvm.expect.i64(i64 %conv, i64 2) -; CHECK: !prof !2 +; CHECK: !prof !3 ; CHECK-NOT: @llvm.expect switch i64 %expval, label %sw.epilog [ i64 1, label %sw.bb @@ -194,7 +194,7 @@ %tmp = load i32, i32* %x.addr, align 4 %conv = sext i32 %tmp to i64 %expval = call i64 @llvm.expect.i64(i64 %conv, i64 1) -; CHECK: !prof !3 +; CHECK: !prof !4 ; CHECK-NOT: @llvm.expect switch i64 %expval, label %sw.epilog [ i64 2, label %sw.bb @@ -278,7 +278,7 @@ %t7 = call i64 @llvm.expect.i64(i64 %t6, i64 0) %t8 = icmp ne i64 %t7, 0 %t9 = select i1 %t8, i32 1, i32 2 -; CHECK: select{{.*}}, !prof !1 +; CHECK: select{{.*}}, !prof !2 ret i32 %t9 } @@ -286,6 +286,6 @@ declare i1 @llvm.expect.i1(i1, i1) nounwind readnone ; CHECK: !0 = !{!"branch_weights", i32 2000, i32 1} -; CHECK: !1 = !{!"branch_weights", i32 1, i32 2000} -; CHECK: !2 = !{!"branch_weights", i32 1, i32 1, i32 2000} -; CHECK: !3 = !{!"branch_weights", i32 2000, i32 1, i32 1} +; CHECK: !2 = !{!"branch_weights", i32 1, i32 2000} +; CHECK: !3 = !{!"branch_weights", i32 1, i32 1, i32 2000} +; CHECK: !4 = !{!"branch_weights", i32 2000, i32 1, i32 1} diff --git a/llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll b/llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll --- a/llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll +++ b/llvm/test/Transforms/LowerExpectIntrinsic/expect-with-probability.ll @@ -138,7 +138,7 @@ %conv1 = sext i32 %conv to i64 %expval = call i64 @llvm.expect.with.probability.i64(i64 %conv1, i64 0, double 8.000000e-01) %tobool = icmp ne i64 %expval, 0 -; CHECK: !prof !1 +; CHECK: !prof !2 ; CHECK-NOT: @llvm.expect.with.probability br i1 %tobool, label %if.then, label %if.end @@ -165,7 +165,7 @@ %tmp = load i32, i32* %x.addr, align 4 %conv = sext i32 %tmp to i64 %expval = call i64 @llvm.expect.with.probability.i64(i64 %conv, i64 2, double 8.000000e-01) -; CHECK: !prof !2 +; CHECK: !prof !3 ; CHECK-NOT: @llvm.expect.with.probability switch i64 %expval, label %sw.epilog [ i64 1, label %sw.bb @@ -194,7 +194,7 @@ %tmp = load i32, i32* %x.addr, align 4 %conv = sext i32 %tmp to i64 %expval = call i64 @llvm.expect.with.probability.i64(i64 %conv, i64 1, double 8.000000e-01) -; CHECK: !prof !3 +; CHECK: !prof !4 ; CHECK-NOT: @llvm.expect.with.probability switch i64 %expval, label %sw.epilog [ i64 2, label %sw.bb @@ -278,7 +278,7 @@ %t7 = call i64 @llvm.expect.with.probability.i64(i64 %t6, i64 0, double 8.000000e-01) %t8 = icmp ne i64 %t7, 0 %t9 = select i1 %t8, i32 1, i32 2 -; CHECK: select{{.*}}, !prof !1 +; CHECK: select{{.*}}, !prof !2 ret i32 %t9 } @@ -286,6 +286,6 @@ declare i1 @llvm.expect.with.probability.i1(i1, i1, double) nounwind readnone ; CHECK: !0 = !{!"branch_weights", i32 1717986918, i32 429496731} -; CHECK: !1 = !{!"branch_weights", i32 429496731, i32 1717986918} -; CHECK: !2 = !{!"branch_weights", i32 214748366, i32 214748366, i32 1717986918} -; CHECK: !3 = !{!"branch_weights", i32 1717986918, i32 214748366, i32 214748366} +; CHECK: !2 = !{!"branch_weights", i32 429496731, i32 1717986918} +; CHECK: !3 = !{!"branch_weights", i32 214748366, i32 214748366, i32 1717986918} +; CHECK: !4 = !{!"branch_weights", i32 1717986918, i32 214748366, i32 214748366}