spatel updated this revision to Diff 54423. spatel added a comment. Patch updated:
1. Fixed/removed comments 2. Changed likely/unlikely profile weights to be min/max values. I'm not sure why 4 and 64 were used in LowerExpectIntrinsics, but that may not trigger the programmer's intent with builtin_expect(). http://reviews.llvm.org/D19299 Files: lib/CodeGen/CGBuiltin.cpp lib/CodeGen/CGStmt.cpp lib/CodeGen/CodeGenFunction.cpp test/CodeGen/builtin-expect.c
Index: test/CodeGen/builtin-expect.c =================================================================== --- test/CodeGen/builtin-expect.c +++ test/CodeGen/builtin-expect.c @@ -1,13 +1,15 @@ // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O1 -disable-llvm-optzns | FileCheck %s --check-prefix=ALL --check-prefix=O1 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | FileCheck %s --check-prefix=ALL --check-prefix=O0 -// In all tests, make sure that no expect is generated if optimizations are off. -// If optimizations are on, generate the correct expect and preserve other necessary operations. +// In all tests, make sure that the builtin is gone. +// If optimizations are on, generate the correct expect metadata and preserve other necessary operations. +// If optimizations are off, no expect metadata is generated but other operations should be preserved. int expect_taken(int x) { // ALL-LABEL: define i32 @expect_taken -// O1: call i64 @llvm.expect.i64(i64 {{%.*}}, i64 1) -// O0-NOT: @llvm.expect +// ALL-NOT: builtin_expect +// O1: !prof [[BR_TRUE_METADATA:.+]] +// O0-NOT: !prof if (__builtin_expect (x, 1)) return 0; @@ -17,8 +19,9 @@ int expect_not_taken(int x) { // ALL-LABEL: define i32 @expect_not_taken -// O1: call i64 @llvm.expect.i64(i64 {{%.*}}, i64 0) -// O0-NOT: @llvm.expect +// ALL-NOT: builtin_expect +// O1: !prof [[BR_FALSE_METADATA:.+]] +// O0-NOT: !prof if (__builtin_expect (x, 0)) return 0; @@ -33,9 +36,10 @@ void expect_value_side_effects() { // ALL-LABEL: define void @expect_value_side_effects() // ALL: [[CALL:%.*]] = call i32 @y +// ALL-NOT: builtin_expect // O1: [[SEXT:%.*]] = sext i32 [[CALL]] to i64 -// O1: call i64 @llvm.expect.i64(i64 {{%.*}}, i64 [[SEXT]]) -// O0-NOT: @llvm.expect +// O1: !prof [[BR_TRUE_METADATA:.+]] +// O0-NOT: !prof if (__builtin_expect (x, y())) foo (); @@ -52,17 +56,18 @@ // ALL-LABEL: define i32 @main() // ALL: call void @isigprocmask() // ALL: [[CALL:%.*]] = call i64 (...) @bar() -// O1: call i64 @llvm.expect.i64(i64 0, i64 [[CALL]]) -// O0-NOT: @llvm.expect +// ALL-NOT: builtin_expect +// ALL-NOT: !prof (void) __builtin_expect((isigprocmask(), 0), bar()); } int switch_cond(int x) { // ALL-LABEL: define i32 @switch_cond -// O1: call i64 @llvm.expect.i64(i64 {{%.*}}, i64 5) -// O0-NOT: @llvm.expect +// ALL-NOT: builtin_expect +// O1: !prof [[SWITCH_METADATA:.+]] +// O0-NOT: !prof switch(__builtin_expect(x, 5)) { default: @@ -78,3 +83,7 @@ return 0; } +// O1: [[BR_TRUE_METADATA]] = !{!"branch_weights", i32 -1, i32 0} +// O1: [[BR_FALSE_METADATA]] = !{!"branch_weights", i32 0, i32 -1} +// O1: [[SWITCH_METADATA]] = !{!"branch_weights", i32 0, i32 0, i32 0, i32 0, i32 -1} + Index: lib/CodeGen/CodeGenFunction.cpp =================================================================== --- lib/CodeGen/CodeGenFunction.cpp +++ lib/CodeGen/CodeGenFunction.cpp @@ -1309,24 +1309,45 @@ return; } - // If the branch has a condition wrapped by __builtin_unpredictable, - // create metadata that specifies that the branch is unpredictable. - // Don't bother if not optimizing because that metadata would not be used. llvm::MDNode *Unpredictable = nullptr; + llvm::MDNode *Weights = nullptr; auto *Call = dyn_cast<CallExpr>(Cond); + + // If the branch has a condition wrapped by __builtin_unpredictable, + // create metadata that specifies that the branch is unpredictable. + // + // If the branch has a condition wrapped by __builtin_expect, create + // metadata that sets the profile weights to the extreme values. + // + // But don't add performance metadata if not optimizing because that metadata + // would not be used anyway. if (Call && CGM.getCodeGenOpts().OptimizationLevel != 0) { - auto *FD = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl()); - if (FD && FD->getBuiltinID() == Builtin::BI__builtin_unpredictable) { + if (auto *FD = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl())) { llvm::MDBuilder MDHelper(getLLVMContext()); - Unpredictable = MDHelper.createUnpredictable(); + if (FD->getBuiltinID() == Builtin::BI__builtin_unpredictable) { + Unpredictable = MDHelper.createUnpredictable(); + } else if (FD->getBuiltinID() == Builtin::BI__builtin_expect) { + const int LikelyWeight = ~0; + const int UnlikelyWeight = 0; + + llvm::Value *ExpectedVal = EmitScalarExpr(Call->getArg(1)); + auto *ExpectedConst = dyn_cast<llvm::ConstantInt>(ExpectedVal); + + // If expecting the false case, set that side to the likely weight. + if (ExpectedConst && ExpectedConst->isNullValue()) + Weights = MDHelper.createBranchWeights(UnlikelyWeight, LikelyWeight); + else + Weights = MDHelper.createBranchWeights(LikelyWeight, UnlikelyWeight); + } } } // Create branch weights based on the number of times we get here and the // number of times the condition should be true. - uint64_t CurrentCount = std::max(getCurrentProfileCount(), TrueCount); - llvm::MDNode *Weights = - createProfileWeights(TrueCount, CurrentCount - TrueCount); + if (!Weights) { + uint64_t CurrentCount = std::max(getCurrentProfileCount(), TrueCount); + Weights = createProfileWeights(TrueCount, CurrentCount - TrueCount); + } // Emit the code with the fully general case. llvm::Value *CondV; Index: lib/CodeGen/CGStmt.cpp =================================================================== --- lib/CodeGen/CGStmt.cpp +++ lib/CodeGen/CGStmt.cpp @@ -1549,18 +1549,42 @@ // If the switch has a condition wrapped by __builtin_unpredictable, // create metadata that specifies that the switch is unpredictable. - // Don't bother if not optimizing because that metadata would not be used. + // + // If the switch has a condition wrapped by __builtin_expect, create + // metadata that sets the profile weights to the extreme values. + // + // But don't add performance metadata if not optimizing because that metadata + // would not be used anyway. auto *Call = dyn_cast<CallExpr>(S.getCond()); if (Call && CGM.getCodeGenOpts().OptimizationLevel != 0) { - auto *FD = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl()); - if (FD && FD->getBuiltinID() == Builtin::BI__builtin_unpredictable) { + if (auto *FD = dyn_cast_or_null<FunctionDecl>(Call->getCalleeDecl())) { llvm::MDBuilder MDHelper(getLLVMContext()); - SwitchInsn->setMetadata(llvm::LLVMContext::MD_unpredictable, - MDHelper.createUnpredictable()); + if (FD->getBuiltinID() == Builtin::BI__builtin_unpredictable) { + SwitchInsn->setMetadata(llvm::LLVMContext::MD_unpredictable, + MDHelper.createUnpredictable()); + } else if (FD->getBuiltinID() == Builtin::BI__builtin_expect) { + const int LikelyWeight = ~0; + const int UnlikelyWeight = 0; + + llvm::Value *ExpectedVal = EmitScalarExpr(Call->getArg(1)); + if (auto *ExpectConst = dyn_cast<llvm::ConstantInt>(ExpectedVal)) { + // The +1 is for the default case. + SmallVector<uint32_t, 16> Weights(SwitchInsn->getNumCases() + 1, + UnlikelyWeight); + auto ExpectedCase = SwitchInsn->findCaseValue(ExpectConst); + if (ExpectedCase == SwitchInsn->case_default()) + Weights[0] = LikelyWeight; + else + Weights[ExpectedCase.getCaseIndex() + 1] = LikelyWeight; + + SwitchInsn->setMetadata(llvm::LLVMContext::MD_prof, + MDHelper.createBranchWeights(Weights)); + } + } } } - if (SwitchWeights) { + if (!SwitchInsn->getMetadata(llvm::LLVMContext::MD_prof) && SwitchWeights) { assert(SwitchWeights->size() == 1 + SwitchInsn->getNumCases() && "switch weights do not match switch cases"); // If there's only one jump destination there's no sense weighting it. Index: lib/CodeGen/CGBuiltin.cpp =================================================================== --- lib/CodeGen/CGBuiltin.cpp +++ lib/CodeGen/CGBuiltin.cpp @@ -631,28 +631,22 @@ "cast"); return RValue::get(Result); } - case Builtin::BI__builtin_unpredictable: { - // Always return the argument of __builtin_unpredictable. LLVM does not - // handle this builtin. Metadata for this builtin should be added directly - // to instructions such as branches or switches that use it. - return RValue::get(EmitScalarExpr(E->getArg(0))); - } + + case Builtin::BI__builtin_unpredictable: case Builtin::BI__builtin_expect: { - Value *ArgValue = EmitScalarExpr(E->getArg(0)); - llvm::Type *ArgType = ArgValue->getType(); + // Always return the first argument. LLVM does not handle these builtins. + // Metadata for these builtins should be added directly to instructions such + // as branches or switches that use the builtin. + Value *Arg0 = EmitScalarExpr(E->getArg(0)); - Value *ExpectedValue = EmitScalarExpr(E->getArg(1)); - // Don't generate llvm.expect on -O0 as the backend won't use it for - // anything. - // Note, we still IRGen ExpectedValue because it could have side-effects. - if (CGM.getCodeGenOpts().OptimizationLevel == 0) - return RValue::get(ArgValue); + // We must IRGen the expected value of builtin_expect because it could have + // side-effects. + if (BuiltinID == Builtin::BI__builtin_expect) + EmitScalarExpr(E->getArg(1)); - Value *FnExpect = CGM.getIntrinsic(Intrinsic::expect, ArgType); - Value *Result = - Builder.CreateCall(FnExpect, {ArgValue, ExpectedValue}, "expval"); - return RValue::get(Result); + return RValue::get(Arg0); } + case Builtin::BI__builtin_assume_aligned: { Value *PtrValue = EmitScalarExpr(E->getArg(0)); Value *OffsetValue =
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits