PGO: Statically generate data structures
Justin Bogner
mail at justinbogner.com
Fri Mar 14 21:01:05 PDT 2014
"Duncan P. N. Exon Smith" <dexonsmith at apple.com> writes:
> commit c7da43152504ab45597732a5984f3e66605a58fd
> Author: Duncan P. N. Exon Smith <dexonsmith at apple.com>
> Date: Thu Mar 13 17:52:40 2014 -0700
>
> PGO: Statically generate data structures
>
> In instrumentation-based profiling, we need a set of data structures to
> represent the counters. Previously, these were built up during static
> initialization. Now, they're shoved into a specially-named section so
> that they show up as an array.
>
> As a consequence of the reorganizing symbols, instrumentation data
> structures for linkonce functions are now correctly coalesced.
>
> This is the first step in making instrumentation-based profiling stop
> relying on the userspace.
>
> <rdar://problem/15943240>
>
> diff --git a/lib/CodeGen/CGBlocks.cpp b/lib/CodeGen/CGBlocks.cpp
> index a1f1368..0aaf9e6 100644
> --- a/lib/CodeGen/CGBlocks.cpp
> +++ b/lib/CodeGen/CGBlocks.cpp
> @@ -1199,7 +1199,7 @@ CodeGenFunction::GenerateBlockFunction(GlobalDecl GD,
> RegionCounter Cnt = getPGORegionCounter(blockDecl->getBody());
> Cnt.beginRegion(Builder);
> EmitStmt(blockDecl->getBody());
> - PGO.emitWriteoutFunction();
> + PGO.emitInstrGenerateData();
This name is awkward and confusing. Why does this say "generate"? The
data's the same for generation and use. How about either
"emitInstrProfData", to match the backend naming, or just
"emitInstrumentationData", which should be obvious from context?
> PGO.destroyRegionCounters();
> }
>
> diff --git a/lib/CodeGen/CGObjC.cpp b/lib/CodeGen/CGObjC.cpp
> index 01ad1f2..c051ed4 100644
> --- a/lib/CodeGen/CGObjC.cpp
> +++ b/lib/CodeGen/CGObjC.cpp
> @@ -507,7 +507,7 @@ void CodeGenFunction::GenerateObjCMethod(const ObjCMethodDecl *OMD) {
> Cnt.beginRegion(Builder);
> EmitCompoundStmtWithoutScope(*cast<CompoundStmt>(OMD->getBody()));
> FinishFunction(OMD->getBodyRBrace());
> - PGO.emitWriteoutFunction();
> + PGO.emitInstrGenerateData();
> PGO.destroyRegionCounters();
> }
>
> diff --git a/lib/CodeGen/CodeGenFunction.cpp b/lib/CodeGen/CodeGenFunction.cpp
> index 2d8389f..062abb7 100644
> --- a/lib/CodeGen/CodeGenFunction.cpp
> +++ b/lib/CodeGen/CodeGenFunction.cpp
> @@ -821,7 +821,7 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
> if (!CurFn->doesNotThrow())
> TryMarkNoThrow(CurFn);
>
> - PGO.emitWriteoutFunction();
> + PGO.emitInstrGenerateData();
> PGO.destroyRegionCounters();
> }
>
> diff --git a/lib/CodeGen/CodeGenPGO.cpp b/lib/CodeGen/CodeGenPGO.cpp
> index 3206daa..dd5b537 100644
> --- a/lib/CodeGen/CodeGenPGO.cpp
> +++ b/lib/CodeGen/CodeGenPGO.cpp
> @@ -144,16 +144,16 @@ bool PGOProfileData::getFunctionCounts(StringRef FuncName,
> }
>
> void CodeGenPGO::setFuncName(llvm::Function *Fn) {
> - StringRef Func = Fn->getName();
> + NormalFuncName = Fn->getName();
NormalFuncName? Is there a SpecialFuncName? This is just FunctionName,
isn't it?
>
> // Function names may be prefixed with a binary '1' to indicate
> // that the backend should not modify the symbols due to any platform
> // naming convention. Do not include that '1' in the PGO profile name.
> - if (Func[0] == '\1')
> - Func = Func.substr(1);
> + if (NormalFuncName[0] == '\1')
> + NormalFuncName = NormalFuncName.substr(1);
>
> if (!Fn->hasLocalLinkage()) {
> - FuncName = new std::string(Func);
> + FuncName = new std::string(NormalFuncName);
> return;
> }
>
> @@ -165,98 +165,160 @@ void CodeGenPGO::setFuncName(llvm::Function *Fn) {
> if (FuncName->empty())
> FuncName->assign("<unknown>");
> FuncName->append(":");
> - FuncName->append(Func);
> + FuncName->append(NormalFuncName);
> }
>
> -void CodeGenPGO::emitWriteoutFunction() {
> - if (!CGM.getCodeGenOpts().ProfileInstrGenerate)
> - return;
> -
> - llvm::LLVMContext &Ctx = CGM.getLLVMContext();
> +static llvm::Function *getRegisterFunc(CodeGenModule &CGM) {
> + return CGM.getModule().getFunction("__llvm_pgo_register_functions");
> +}
>
> - llvm::Type *Int32Ty = llvm::Type::getInt32Ty(Ctx);
> - llvm::Type *Int8PtrTy = llvm::Type::getInt8PtrTy(Ctx);
> -
> - llvm::Function *WriteoutF =
> - CGM.getModule().getFunction("__llvm_pgo_writeout");
> - if (!WriteoutF) {
> - llvm::FunctionType *WriteoutFTy =
> - llvm::FunctionType::get(llvm::Type::getVoidTy(Ctx), false);
> - WriteoutF = llvm::Function::Create(WriteoutFTy,
> - llvm::GlobalValue::InternalLinkage,
> - "__llvm_pgo_writeout", &CGM.getModule());
> - }
> - WriteoutF->setUnnamedAddr(true);
> - WriteoutF->addFnAttr(llvm::Attribute::NoInline);
> +static llvm::BasicBlock *getOrInsertRegisterBB(CodeGenModule &CGM) {
> + // Only need to insert this once per module.
> + if (llvm::Function *RegisterF = getRegisterFunc(CGM))
> + return &RegisterF->getEntryBlock();
> +
> + // Construct the function.
> + auto *VoidTy = llvm::Type::getVoidTy(CGM.getLLVMContext());
> + auto *RegisterFTy = llvm::FunctionType::get(VoidTy, false);
> + auto *RegisterF
> + = llvm::Function::Create(RegisterFTy,
> + llvm::GlobalValue::InternalLinkage,
> + "__llvm_pgo_register_functions", &CGM.getModule());
The indentation is funny here:
auto *RegisterF = llvm::Function::Create(RegisterFTy,
llvm::GlobalValue::InternalLinkage,
"__llvm_pgo_register_functions",
&CGM.getModule());
> + RegisterF->setUnnamedAddr(true);
> + RegisterF->addFnAttr(llvm::Attribute::NoInline);
> if (CGM.getCodeGenOpts().DisableRedZone)
> - WriteoutF->addFnAttr(llvm::Attribute::NoRedZone);
> + RegisterF->addFnAttr(llvm::Attribute::NoRedZone);
> +
> + // Construct and return the entry block.
> + auto *BB = llvm::BasicBlock::Create(CGM.getLLVMContext(), "", RegisterF);
> + CGBuilderTy Builder(BB);
> + Builder.CreateRetVoid();
> + return BB;
> +}
>
> - llvm::BasicBlock *BB = WriteoutF->empty() ?
> - llvm::BasicBlock::Create(Ctx, "", WriteoutF) : &WriteoutF->getEntryBlock();
> +static llvm::Constant *getOrInsertRuntimeRegister(CodeGenModule &CGM) {
> + auto *VoidTy = llvm::Type::getVoidTy(CGM.getLLVMContext());
> + auto *VoidPtrTy = llvm::Type::getInt8PtrTy(CGM.getLLVMContext());
> + auto *RuntimeRegisterTy = llvm::FunctionType::get(VoidTy, VoidPtrTy, false);
> + return CGM.getModule().getOrInsertFunction("__llvm_pgo_register_function",
> + RuntimeRegisterTy);
> +}
> +
> +static llvm::Constant *getOrInsertRuntimeWriteAtExit(CodeGenModule &CGM) {
> + // TODO: make this depend on a command-line option.
> + auto *VoidTy = llvm::Type::getVoidTy(CGM.getLLVMContext());
> + auto *WriteAtExitTy = llvm::FunctionType::get(VoidTy, false);
> + return CGM.getModule().getOrInsertFunction("__llvm_pgo_register_write_atexit",
> + WriteAtExitTy);
> +}
> +
> +static StringRef getCountersSection(const CodeGenModule &CGM) {
> + if (CGM.getTarget().getTriple().getObjectFormat() == llvm::Triple::MachO)
> + return "__DATA,__llvm_pgo_cnts";
> + else
> + return "__llvm_pgo_cnts";
> +}
>
> - CGBuilderTy PGOBuilder(BB);
> +static StringRef getNameSection(const CodeGenModule &CGM) {
> + if (CGM.getTarget().getTriple().getObjectFormat() == llvm::Triple::MachO)
> + return "__DATA,__llvm_pgo_names";
> + else
> + return "__llvm_pgo_names";
> +}
>
> - llvm::Instruction *I = BB->getTerminator();
> - if (!I)
> - I = PGOBuilder.CreateRetVoid();
> - PGOBuilder.SetInsertPoint(I);
> +static StringRef getDataSection(const CodeGenModule &CGM) {
> + if (CGM.getTarget().getTriple().getObjectFormat() == llvm::Triple::MachO)
> + return "__DATA,__llvm_pgo_data";
> + else
> + return "__llvm_pgo_data";
> +}
This smells wrong to me. (1) We're at a pretty high level for checking the
triple, and (2) why is MachO special?
>
> - llvm::Type *Int64PtrTy = llvm::Type::getInt64PtrTy(Ctx);
> - llvm::Type *Args[] = {
> - Int8PtrTy, // const char *FuncName
> - Int32Ty, // uint32_t NumCounters
> - Int64PtrTy // uint64_t *Counters
> +llvm::GlobalVariable *CodeGenPGO::buildDataVar() {
> + // Create name variable.
> + llvm::LLVMContext &Ctx = CGM.getLLVMContext();
> + auto *VarName =
> + llvm::ConstantDataArray::getString(Ctx, getFuncName(), false);
> + auto *Name =
> + new llvm::GlobalVariable(CGM.getModule(), VarName->getType(), true,
> + FuncLinkage, VarName, getFuncVarName("name"));
I think this reads better without the newlines after =:
auto *VarName = llvm::ConstantDataArray::getString(Ctx, getFuncName(), false);
auto *Name = new llvm::GlobalVariable(CGM.getModule(), VarName->getType(),
true, FuncLinkage, VarName,
getFuncVarName("name"));
> + Name->setSection(getNameSection(CGM));
> + Name->setAlignment(1);
> +
> + // Create data variable.
> + auto *Int32Ty = llvm::Type::getInt32Ty(Ctx);
> + auto *Int8PtrTy = llvm::Type::getInt8PtrTy(Ctx);
> + auto *Int64PtrTy = llvm::Type::getInt64PtrTy(Ctx);
> + llvm::Type *DataTypes[] = {
> + Int32Ty, Int32Ty, Int8PtrTy, Int64PtrTy
> };
> - llvm::FunctionType *FTy =
> - llvm::FunctionType::get(PGOBuilder.getVoidTy(), Args, false);
> - llvm::Constant *EmitFunc =
> - CGM.getModule().getOrInsertFunction("llvm_pgo_emit", FTy);
> -
> - llvm::Constant *NameString =
> - CGM.GetAddrOfConstantCString(getFuncName(), "__llvm_pgo_name");
> - NameString = llvm::ConstantExpr::getBitCast(NameString, Int8PtrTy);
> - PGOBuilder.CreateCall3(EmitFunc, NameString,
> - PGOBuilder.getInt32(NumRegionCounters),
> - PGOBuilder.CreateBitCast(RegionCounters, Int64PtrTy));
> + auto *DataTy = llvm::StructType::get(Ctx, makeArrayRef(DataTypes));
> + llvm::Constant *DataVals[] = {
> + llvm::ConstantInt::get(Int32Ty, getFuncName().size()),
> + llvm::ConstantInt::get(Int32Ty, NumRegionCounters),
> + llvm::ConstantExpr::getBitCast(Name, Int8PtrTy),
> + llvm::ConstantExpr::getBitCast(RegionCounters, Int64PtrTy)
> + };
> + auto *Data =
> + new llvm::GlobalVariable(CGM.getModule(), DataTy, true, FuncLinkage,
> + llvm::ConstantStruct::get(DataTy, DataVals),
> + getFuncVarName("data"));
> +
> + // All the data should be packed into an array in its own section.
> + Data->setSection(getDataSection(CGM));
> + Data->setAlignment(8);
> +
> + // Make sure the data doesn't get deleted.
> + CGM.addUsedGlobal(Data);
> + return Data;
> +}
> +
> +void CodeGenPGO::emitInstrGenerateData() {
> + if (!CGM.getCodeGenOpts().ProfileInstrGenerate)
> + return;
> +
> + // Build the data.
> + auto *Data = buildDataVar();
> +
> + // Register the data.
> + //
> + // TODO: only register when static initialization is required.
> + CGBuilderTy Builder(getOrInsertRegisterBB(CGM)->getTerminator());
> + auto *VoidPtrTy = llvm::Type::getInt8PtrTy(CGM.getLLVMContext());
> + Builder.CreateCall(getOrInsertRuntimeRegister(CGM),
> + Builder.CreateBitCast(Data, VoidPtrTy));
> }
>
> llvm::Function *CodeGenPGO::emitInitialization(CodeGenModule &CGM) {
> - llvm::Function *WriteoutF =
> - CGM.getModule().getFunction("__llvm_pgo_writeout");
> - if (!WriteoutF)
> - return NULL;
> + if (!CGM.getCodeGenOpts().ProfileInstrGenerate)
> + return 0;
Why wasn't this check needed here before?
> - // Create a small bit of code that registers the "__llvm_pgo_writeout" to
> - // be executed at exit.
> - llvm::Function *F = CGM.getModule().getFunction("__llvm_pgo_init");
> - if (F)
> - return NULL;
> + // Only need to create this once per module.
> + if (CGM.getModule().getFunction("__llvm_pgo_init"))
> + return 0;
>
> - llvm::LLVMContext &Ctx = CGM.getLLVMContext();
> - llvm::FunctionType *FTy = llvm::FunctionType::get(llvm::Type::getVoidTy(Ctx),
> - false);
> - F = llvm::Function::Create(FTy, llvm::GlobalValue::InternalLinkage,
> - "__llvm_pgo_init", &CGM.getModule());
> + // Get the functions to call at initialization.
> + llvm::Constant *RegisterF = getRegisterFunc(CGM);
> + llvm::Constant *WriteAtExitF = getOrInsertRuntimeWriteAtExit(CGM);
> + if (!RegisterF && !WriteAtExitF)
> + return 0;
> +
> + // Create the initialization function.
> + auto *VoidTy = llvm::Type::getVoidTy(CGM.getLLVMContext());
> + auto *F = llvm::Function::Create(llvm::FunctionType::get(VoidTy, false),
> + llvm::GlobalValue::InternalLinkage,
> + "__llvm_pgo_init", &CGM.getModule());
> F->setUnnamedAddr(true);
> - F->setLinkage(llvm::GlobalValue::InternalLinkage);
> F->addFnAttr(llvm::Attribute::NoInline);
> if (CGM.getCodeGenOpts().DisableRedZone)
> F->addFnAttr(llvm::Attribute::NoRedZone);
>
> - llvm::BasicBlock *BB = llvm::BasicBlock::Create(CGM.getLLVMContext(), "", F);
> - CGBuilderTy PGOBuilder(BB);
> -
> - FTy = llvm::FunctionType::get(PGOBuilder.getVoidTy(), false);
> - llvm::Type *Params[] = {
> - llvm::PointerType::get(FTy, 0)
> - };
> - FTy = llvm::FunctionType::get(PGOBuilder.getVoidTy(), Params, false);
> -
> - // Inialize the environment and register the local writeout function.
> - llvm::Constant *PGOInit =
> - CGM.getModule().getOrInsertFunction("llvm_pgo_init", FTy);
> - PGOBuilder.CreateCall(PGOInit, WriteoutF);
> - PGOBuilder.CreateRetVoid();
> + // Add the basic block and the necessary calls.
> + CGBuilderTy Builder(llvm::BasicBlock::Create(CGM.getLLVMContext(), "", F));
> + if (RegisterF)
> + Builder.CreateCall(RegisterF);
> + if (WriteAtExitF)
> + Builder.CreateCall(WriteAtExitF);
> + Builder.CreateRetVoid();
>
> return F;
> }
> @@ -764,6 +826,7 @@ void CodeGenPGO::assignRegionCounters(const Decl *D, llvm::Function *Fn) {
> if (!D)
> return;
> setFuncName(Fn);
> + FuncLinkage = Fn->getLinkage();
> mapRegionCounters(D);
> if (InstrumentRegions)
> emitCounterVariables();
> @@ -819,10 +882,11 @@ void CodeGenPGO::emitCounterVariables() {
> llvm::ArrayType *CounterTy = llvm::ArrayType::get(llvm::Type::getInt64Ty(Ctx),
> NumRegionCounters);
> RegionCounters =
> - new llvm::GlobalVariable(CGM.getModule(), CounterTy, false,
> - llvm::GlobalVariable::PrivateLinkage,
> + new llvm::GlobalVariable(CGM.getModule(), CounterTy, false, FuncLinkage,
> llvm::Constant::getNullValue(CounterTy),
> - "__llvm_pgo_ctr");
> + getFuncVarName("counters"));
> + RegionCounters->setAlignment(8);
> + RegionCounters->setSection(getCountersSection(CGM));
> }
>
> void CodeGenPGO::emitCounterIncrement(CGBuilderTy &Builder, unsigned Counter) {
> diff --git a/lib/CodeGen/CodeGenPGO.h b/lib/CodeGen/CodeGenPGO.h
> index 51d59cf..7820dd6 100644
> --- a/lib/CodeGen/CodeGenPGO.h
> +++ b/lib/CodeGen/CodeGenPGO.h
> @@ -53,6 +53,8 @@ class CodeGenPGO {
> private:
> CodeGenModule &CGM;
> std::string *FuncName;
> + StringRef NormalFuncName;
> + llvm::GlobalValue::LinkageTypes FuncLinkage;
>
> unsigned NumRegionCounters;
> llvm::GlobalVariable *RegionCounters;
> @@ -77,7 +79,11 @@ public:
>
> /// Get the string used to identify this function in the profile data.
> /// For functions with local linkage, this includes the main file name.
> - const StringRef getFuncName() const { return StringRef(*FuncName); }
> + StringRef getFuncName() const { return StringRef(*FuncName); }
> + std::string getFuncVarName(StringRef VarName) const {
> + return Twine("__llvm_pgo_").concat(VarName).concat("_")
> + .concat(NormalFuncName).str();
> + }
>
> /// Return the counter value of the current region.
> uint64_t getCurrentRegionCount() const { return CurrentRegionCount; }
> @@ -124,7 +130,7 @@ public:
> /// counters depending on whether we are generating or using instrumentation.
> void assignRegionCounters(const Decl *D, llvm::Function *Fn);
> /// Emit code to write counts for a given function to disk, if necessary.
> - void emitWriteoutFunction();
> + void emitInstrGenerateData();
Update the doc comment here?
> /// Clean up region counter state. Must be called if assignRegionCounters is
> /// used.
> void destroyRegionCounters();
> @@ -139,6 +145,7 @@ private:
> void applyFunctionAttributes(PGOProfileData *PGOData, llvm::Function *Fn);
> void loadRegionCounts(PGOProfileData *PGOData);
> void emitCounterVariables();
> + llvm::GlobalVariable *buildDataVar();
>
> /// Emit code to increment the counter at the given index
> void emitCounterIncrement(CGBuilderTy &Builder, unsigned Counter);
> diff --git a/test/Profile/c-counter-overflows.c b/test/Profile/c-counter-overflows.c
> index 17cf3a3..7cbe9bb 100644
> --- a/test/Profile/c-counter-overflows.c
> +++ b/test/Profile/c-counter-overflows.c
> @@ -5,7 +5,6 @@
>
> typedef unsigned long long uint64_t;
>
> -// PGOGEN: @[[MAIN:__llvm_pgo_ctr[0-9]*]] = private global [2 x i64] zeroinitializer
> int main(int argc, const char *argv[]) {
> // Need counts higher than 32-bits.
> // CHECK: br {{.*}} !prof ![[FOR:[0-9]+]]
> diff --git a/test/Profile/c-general.c b/test/Profile/c-general.c
> index 7945d8d..e1f83b3 100644
> --- a/test/Profile/c-general.c
> +++ b/test/Profile/c-general.c
> @@ -3,16 +3,16 @@
> // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c %s -o - -emit-llvm -fprofile-instr-generate | FileCheck -check-prefix=PGOGEN %s
> // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c %s -o - -emit-llvm -fprofile-instr-use=%S/Inputs/c-general.profdata | FileCheck -check-prefix=PGOUSE %s
>
> -// PGOGEN: @[[SLC:__llvm_pgo_ctr[0-9]*]] = private global [4 x i64] zeroinitializer
> -// PGOGEN: @[[IFC:__llvm_pgo_ctr[0-9]*]] = private global [11 x i64] zeroinitializer
> -// PGOGEN: @[[EEC:__llvm_pgo_ctr[0-9]*]] = private global [9 x i64] zeroinitializer
> -// PGOGEN: @[[JMC:__llvm_pgo_ctr[0-9]*]] = private global [22 x i64] zeroinitializer
> -// PGOGEN: @[[SWC:__llvm_pgo_ctr[0-9]*]] = private global [19 x i64] zeroinitializer
> -// PGOGEN: @[[BSC:__llvm_pgo_ctr[0-9]*]] = private global [17 x i64] zeroinitializer
> -// PGOGEN: @[[BOC:__llvm_pgo_ctr[0-9]*]] = private global [8 x i64] zeroinitializer
> -// PGOGEN: @[[BLC:__llvm_pgo_ctr[0-9]*]] = private global [9 x i64] zeroinitializer
> -// PGOGEN: @[[MAC:__llvm_pgo_ctr[0-9]*]] = private global [1 x i64] zeroinitializer
> -// PGOGEN: @[[STC:__llvm_pgo_ctr[0-9]*]] = private global [2 x i64] zeroinitializer
> +// PGOGEN: @[[SLC:__llvm_pgo_counters_simple_loops]] = global [4 x i64] zeroinitializer
> +// PGOGEN: @[[IFC:__llvm_pgo_counters_conditionals]] = global [11 x i64] zeroinitializer
> +// PGOGEN: @[[EEC:__llvm_pgo_counters_early_exits]] = global [9 x i64] zeroinitializer
> +// PGOGEN: @[[JMC:__llvm_pgo_counters_jumps]] = global [22 x i64] zeroinitializer
> +// PGOGEN: @[[SWC:__llvm_pgo_counters_switches]] = global [19 x i64] zeroinitializer
> +// PGOGEN: @[[BSC:__llvm_pgo_counters_big_switch]] = global [17 x i64] zeroinitializer
> +// PGOGEN: @[[BOC:__llvm_pgo_counters_boolean_operators]] = global [8 x i64] zeroinitializer
> +// PGOGEN: @[[BLC:__llvm_pgo_counters_boolop_loops]] = global [9 x i64] zeroinitializer
> +// PGOGEN: @[[MAC:__llvm_pgo_counters_main]] = global [1 x i64] zeroinitializer
> +// PGOGEN: @[[STC:__llvm_pgo_counters_static_func]] = internal global [2 x i64] zeroinitializer
>
> // PGOGEN-LABEL: @simple_loops()
> // PGOUSE-LABEL: @simple_loops()
> diff --git a/test/Profile/c-linkage.c b/test/Profile/c-linkage.c
> new file mode 100644
> index 0000000..dddc895
> --- /dev/null
> +++ b/test/Profile/c-linkage.c
> @@ -0,0 +1,31 @@
> +// Check the data structures emitted by instrumentation.
> +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-linkage.c %s -o - -emit-llvm -fprofile-instr-generate | FileCheck %s
> +
> +// CHECK: @__llvm_pgo_counters_foo = global [1 x i64] zeroinitializer, section "__DATA,__llvm_pgo_cnts", align 8
> +// CHECK: @__llvm_pgo_name_foo = constant [3 x i8] c"foo", section "__DATA,__llvm_pgo_names", align 1
> +// CHECK: @__llvm_pgo_data_foo = constant { i32, i32, i8*, i64* } { i32 3, i32 1, i8* getelementptr inbounds ([3 x i8]* @__llvm_pgo_name_foo, i32 0, i32 0), i64* getelementptr inbounds ([1 x i64]* @__llvm_pgo_counters_foo, i32 0, i32 0) }, section "__DATA,__llvm_pgo_data", align 8
> +void foo(void) { }
> +
> +// CHECK: @__llvm_pgo_counters_foo_weak = weak global [5 x i64] zeroinitializer, section "__DATA,__llvm_pgo_cnts", align 8
> +// CHECK: @__llvm_pgo_name_foo_weak = weak constant [8 x i8] c"foo_weak", section "__DATA,__llvm_pgo_names", align 1
> +// CHECK: @__llvm_pgo_data_foo_weak = weak constant { i32, i32, i8*, i64* } { i32 8, i32 5, i8* getelementptr inbounds ([8 x i8]* @__llvm_pgo_name_foo_weak, i32 0, i32 0), i64* getelementptr inbounds ([5 x i64]* @__llvm_pgo_counters_foo_weak, i32 0, i32 0) }, section "__DATA,__llvm_pgo_data", align 8
> +void foo_weak(void) __attribute__((weak));
> +void foo_weak(void) { if (0){} if (0){} if (0){} if (0){} }
> +
> +// CHECK: @__llvm_pgo_counters_main = global [1 x i64] zeroinitializer, section "__DATA,__llvm_pgo_cnts", align 8
> +// CHECK: @__llvm_pgo_name_main = constant [4 x i8] c"main", section "__DATA,__llvm_pgo_names", align 1
> +// CHECK: @__llvm_pgo_data_main = constant { i32, i32, i8*, i64* } { i32 4, i32 1, i8* getelementptr inbounds ([4 x i8]* @__llvm_pgo_name_main, i32 0, i32 0), i64* getelementptr inbounds ([1 x i64]* @__llvm_pgo_counters_main, i32 0, i32 0) }, section "__DATA,__llvm_pgo_data", align 8
> +static void foo_internal(void);
> +int main(void) {
> + foo();
> + foo_internal();
> + foo_weak();
> + return 0;
> +}
> +
> +// CHECK: @__llvm_pgo_counters_foo_internal = internal global [3 x i64] zeroinitializer, section "__DATA,__llvm_pgo_cnts", align 8
> +// CHECK: @__llvm_pgo_name_foo_internal = internal constant [24 x i8] c"c-linkage.c:foo_internal", section "__DATA,__llvm_pgo_names", align 1
> +// CHECK: @__llvm_pgo_data_foo_internal = internal constant { i32, i32, i8*, i64* } { i32 24, i32 3, i8* getelementptr inbounds ([24 x i8]* @__llvm_pgo_name_foo_internal, i32 0, i32 0), i64* getelementptr inbounds ([3 x i64]* @__llvm_pgo_counters_foo_internal, i32 0, i32 0) }, section "__DATA,__llvm_pgo_data", align 8
> +static void foo_internal(void) { if (0){} if (0){} }
> +
> +// CHECK: @llvm.used = appending global [4 x i8*] [i8* bitcast ({ i32, i32, i8*, i64* }* @__llvm_pgo_data_foo to i8*), i8* bitcast ({ i32, i32, i8*, i64* }* @__llvm_pgo_data_foo_weak to i8*), i8* bitcast ({ i32, i32, i8*, i64* }* @__llvm_pgo_data_main to i8*), i8* bitcast ({ i32, i32, i8*, i64* }* @__llvm_pgo_data_foo_internal to i8*)], section "llvm.metadata"
> diff --git a/test/Profile/cxx-class.cpp b/test/Profile/cxx-class.cpp
> index 7432bc3..61befa3 100644
> --- a/test/Profile/cxx-class.cpp
> +++ b/test/Profile/cxx-class.cpp
> @@ -17,7 +17,7 @@ class Simple {
> public:
> // CTRGEN-LABEL: define {{.*}} @_ZN6SimpleC2Ei(
> // CTRUSE-LABEL: define {{.*}} @_ZN6SimpleC2Ei(
> - // CTRGEN: store {{.*}} @[[SCC:__llvm_pgo_ctr[0-9]*]], i64 0, i64 0
> + // CTRGEN: store {{.*}} @[[SCC:__llvm_pgo_counters__ZN6SimpleC2Ei]], i64 0, i64 0
> explicit Simple(int Member) : Member(Member) {
> // CTRGEN: store {{.*}} @[[SCC]], i64 0, i64 1
> // CTRUSE: br {{.*}} !prof ![[SC1:[0-9]+]]
> @@ -30,7 +30,7 @@ public:
>
> // DTRGEN-LABEL: define {{.*}} @_ZN6SimpleD2Ev(
> // DTRUSE-LABEL: define {{.*}} @_ZN6SimpleD2Ev(
> - // DTRGEN: store {{.*}} @[[SDC:__llvm_pgo_ctr[0-9]*]], i64 0, i64 0
> + // DTRGEN: store {{.*}} @[[SDC:__llvm_pgo_counters__ZN6SimpleD2Ev]], i64 0, i64 0
> ~Simple() {
> // DTRGEN: store {{.*}} @[[SDC]], i64 0, i64 1
> // DTRUSE: br {{.*}} !prof ![[SD1:[0-9]+]]
> @@ -43,7 +43,7 @@ public:
>
> // MTHGEN-LABEL: define {{.*}} @_ZN6Simple6methodEv(
> // MTHUSE-LABEL: define {{.*}} @_ZN6Simple6methodEv(
> - // MTHGEN: store {{.*}} @[[SMC:__llvm_pgo_ctr[0-9]*]], i64 0, i64 0
> + // MTHGEN: store {{.*}} @[[SMC:__llvm_pgo_counters__ZN6Simple6methodEv]], i64 0, i64 0
> void method() {
> // MTHGEN: store {{.*}} @[[SMC]], i64 0, i64 1
> // MTHUSE: br {{.*}} !prof ![[SM1:[0-9]+]]
> @@ -57,7 +57,7 @@ public:
>
> // WRPGEN-LABEL: define {{.*}} @_Z14simple_wrapperv(
> // WRPUSE-LABEL: define {{.*}} @_Z14simple_wrapperv(
> -// WRPGEN: store {{.*}} @[[SWC:__llvm_pgo_ctr[0-9]*]], i64 0, i64 0
> +// WRPGEN: store {{.*}} @[[SWC:__llvm_pgo_counters__Z14simple_wrapperv]], i64 0, i64 0
> void simple_wrapper() {
> // WRPGEN: store {{.*}} @[[SWC]], i64 0, i64 1
> // WRPUSE: br {{.*}} !prof ![[SW1:[0-9]+]]
> diff --git a/test/Profile/cxx-linkage.cpp b/test/Profile/cxx-linkage.cpp
> new file mode 100644
> index 0000000..6597e25
> --- /dev/null
> +++ b/test/Profile/cxx-linkage.cpp
> @@ -0,0 +1,30 @@
> +// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9.0 -emit-llvm -main-file-name cxx-linkage.cpp %s -o - -fprofile-instr-generate | FileCheck %s
> +
> +// CHECK: @__llvm_pgo_counters__Z3foov = global [1 x i64] zeroinitializer, section "__DATA,__llvm_pgo_cnts", align 8
> +// CHECK: @__llvm_pgo_name__Z3foov = constant [7 x i8] c"_Z3foov", section "__DATA,__llvm_pgo_names", align 1
> +// CHECK: @__llvm_pgo_data__Z3foov = constant { i32, i32, i8*, i64* } { i32 7, i32 1, i8* getelementptr inbounds ([7 x i8]* @__llvm_pgo_name__Z3foov, i32 0, i32 0), i64* getelementptr inbounds ([1 x i64]* @__llvm_pgo_counters__Z3foov, i32 0, i32 0) }, section "__DATA,__llvm_pgo_data", align 8
> +void foo(void) { }
> +
> +// CHECK: @__llvm_pgo_counters__Z8foo_weakv = weak global [5 x i64] zeroinitializer, section "__DATA,__llvm_pgo_cnts", align 8
> +// CHECK: @__llvm_pgo_name__Z8foo_weakv = weak constant [12 x i8] c"_Z8foo_weakv", section "__DATA,__llvm_pgo_names", align 1
> +// CHECK: @__llvm_pgo_data__Z8foo_weakv = weak constant { i32, i32, i8*, i64* } { i32 12, i32 5, i8* getelementptr inbounds ([12 x i8]* @__llvm_pgo_name__Z8foo_weakv, i32 0, i32 0), i64* getelementptr inbounds ([5 x i64]* @__llvm_pgo_counters__Z8foo_weakv, i32 0, i32 0) }, section "__DATA,__llvm_pgo_data", align 8
> +void foo_weak(void) __attribute__((weak));
> +void foo_weak(void) { if (0){} if (0){} if (0){} if (0){} }
> +
> +// CHECK: @__llvm_pgo_counters_main = global [1 x i64] zeroinitializer, section "__DATA,__llvm_pgo_cnts", align 8
> +// CHECK: @__llvm_pgo_name_main = constant [4 x i8] c"main", section "__DATA,__llvm_pgo_names", align 1
> +// CHECK: @__llvm_pgo_data_main = constant { i32, i32, i8*, i64* } { i32 4, i32 1, i8* getelementptr inbounds ([4 x i8]* @__llvm_pgo_name_main, i32 0, i32 0), i64* getelementptr inbounds ([1 x i64]* @__llvm_pgo_counters_main, i32 0, i32 0) }, section "__DATA,__llvm_pgo_data", align 8
> +inline void foo_inline(void);
> +int main(void) {
> + foo();
> + foo_inline();
> + foo_weak();
> + return 0;
> +}
> +
> +// CHECK: @__llvm_pgo_counters__Z10foo_inlinev = linkonce_odr global [7 x i64] zeroinitializer, section "__DATA,__llvm_pgo_cnts", align 8
> +// CHECK: @__llvm_pgo_name__Z10foo_inlinev = linkonce_odr constant [15 x i8] c"_Z10foo_inlinev", section "__DATA,__llvm_pgo_names", align 1
> +// CHECK: @__llvm_pgo_data__Z10foo_inlinev = linkonce_odr constant { i32, i32, i8*, i64* } { i32 15, i32 7, i8* getelementptr inbounds ([15 x i8]* @__llvm_pgo_name__Z10foo_inlinev, i32 0, i32 0), i64* getelementptr inbounds ([7 x i64]* @__llvm_pgo_counters__Z10foo_inlinev, i32 0, i32 0) }, section "__DATA,__llvm_pgo_data", align 8
> +inline void foo_inline(void) { if (0){} if (0){} if (0){} if (0){} if (0){} if (0){}}
> +
> +// CHECK: @llvm.used = appending global [4 x i8*] [i8* bitcast ({ i32, i32, i8*, i64* }* @__llvm_pgo_data__Z3foov to i8*), i8* bitcast ({ i32, i32, i8*, i64* }* @__llvm_pgo_data__Z8foo_weakv to i8*), i8* bitcast ({ i32, i32, i8*, i64* }* @__llvm_pgo_data_main to i8*), i8* bitcast ({ i32, i32, i8*, i64* }* @__llvm_pgo_data__Z10foo_inlinev to i8*)], section "llvm.metadata"
> diff --git a/test/Profile/cxx-throws.cpp b/test/Profile/cxx-throws.cpp
> index 0a8ebf1..c243947 100644
> --- a/test/Profile/cxx-throws.cpp
> +++ b/test/Profile/cxx-throws.cpp
> @@ -9,8 +9,8 @@
> // RUN: %clang %s -o - -emit-llvm -S -fprofile-instr-use=%S/Inputs/cxx-throws.profdata -target %itanium_abi_triple | FileCheck -check-prefix=PGOUSE %s
> // RUN: %clang %s -o - -emit-llvm -S -fprofile-instr-use=%S/Inputs/cxx-throws.profdata -target %itanium_abi_triple | FileCheck -check-prefix=PGOUSE-EXC %s
>
> -// PGOGEN: @[[THC:__llvm_pgo_ctr[0-9]*]] = private global [9 x i64] zeroinitializer
> -// PGOGEN-EXC: @[[THC:__llvm_pgo_ctr[0-9]*]] = private global [9 x i64] zeroinitializer
> +// PGOGEN: @[[THC:__llvm_pgo_counters__Z6throwsv]] = global [9 x i64] zeroinitializer
> +// PGOGEN-EXC: @[[THC:__llvm_pgo_counters__Z6throwsv]] = global [9 x i64] zeroinitializer
>
> // PGOGEN-LABEL: @_Z6throwsv()
> // PGOUSE-LABEL: @_Z6throwsv()
> diff --git a/test/Profile/objc-general.m b/test/Profile/objc-general.m
> index b5f2673..56d95c2 100644
> --- a/test/Profile/objc-general.m
> +++ b/test/Profile/objc-general.m
> @@ -29,9 +29,9 @@ struct NSFastEnumerationState;
> @end;
> #endif
>
> -// PGOGEN: @[[FRC:__llvm_pgo_ctr[0-9]*]] = private global [2 x i64] zeroinitializer
> -// PGOGEN: @[[BLC:__llvm_pgo_ctr[0-9]*]] = private global [2 x i64] zeroinitializer
> -// PGOGEN: @[[MAC:__llvm_pgo_ctr[0-9]*]] = private global [1 x i64] zeroinitializer
> +// PGOGEN: @[[FRC:"__llvm_pgo_counters_\+\[A foreach:\]"]] = internal global [2 x i64] zeroinitializer
> +// PGOGEN: @[[BLC:"__llvm_pgo_counters___13\+\[A foreach:\]_block_invoke"]] = internal global [2 x i64] zeroinitializer
> +// PGOGEN: @[[MAC:__llvm_pgo_counters_main]] = global [1 x i64] zeroinitializer
>
> @interface A : NSObject
> + (void)foreach: (NSArray *)array;
> commit d3f27b888610f69dd481c8afe741de4ac23ccd8b
> Author: Duncan P. N. Exon Smith <dexonsmith at apple.com>
> Date: Fri Mar 14 16:30:37 2014 -0700
>
> PGO: Statically generate data structures
>
> In instrumentation-based profiling, we need a set of data structures to
> represent the counters. Previously, these were built up during static
> initialization. Now, they're shoved into a specially-named section so
> that they show up as an array.
>
> As a consequence of the reorganizing symbols, instrumentation data
> structures for linkonce functions are now correctly coalesced.
>
> This is the first step in making instrumentation-based profiling stop
> relying on the userspace.
>
> <rdar://problem/15943240>
>
> diff --git a/lib/profile/PGOProfiling.c b/lib/profile/PGOProfiling.c
> index 986fd84..a8540d6 100644
> --- a/lib/profile/PGOProfiling.c
> +++ b/lib/profile/PGOProfiling.c
> @@ -32,68 +32,107 @@ typedef unsigned int uint32_t;
> typedef unsigned long long uint64_t;
> #endif
>
> -static FILE *OutputFile = NULL;
> +typedef struct __llvm_pgo_data {
> + const uint32_t NameSize;
> + const uint32_t NumCounters;
> + const char *const Name;
> + const uint64_t *const Counters;
> +} DataType;
> +
> +/* TODO: Calculate these with linker magic.
> + */
Why is the */ on it's own line? That's unusual for a one-line c-style
comment. This applies to several comments later in the file as well.
> +static DataType *First = NULL;
> +static DataType *Final = NULL;
> +/*! \brief Register an instrumented function.
> + *
> + * Calls to this are inserted by clang with -fprofile-instr-generate. They
> + * should only be required on targets where we can't use linker magic to find
> + * the bounds of the section.
> + */
This comment isn't actually true yet. I'd leave it out until we actually
do the linker magic.
> +void __llvm_pgo_register_function(void *Data_) {
> + /* TODO: Only emit this function if we can't use linker magic.
> + */
> + DataType *Data = (DataType*)Data_;
> + if (!First || Data < First)
> + First = Data;
> + if (!Final || Data > Final)
> + Final = Data;
> +}
>
> -/*
> - * A list of functions to write out the data.
> +/*! \brief Get the first instrumentation record.
> */
> -typedef void (*writeout_fn)();
> -
> -struct writeout_fn_node {
> - writeout_fn fn;
> - struct writeout_fn_node *next;
> -};
> -
> -static struct writeout_fn_node *writeout_fn_head = NULL;
> -static struct writeout_fn_node *writeout_fn_tail = NULL;
> -
> -void llvm_pgo_emit(const char *MangledName, uint32_t NumCounters,
> - uint64_t *Counters) {
> - uint32_t i;
> - fprintf(OutputFile, "%s %u\n", MangledName, NumCounters);
> - for (i = 0; i < NumCounters; ++i)
> - fprintf(OutputFile, "%" PRIu64 "\n", Counters[i]);
> - fprintf(OutputFile, "\n");
> +static DataType *getFirst() {
> + /* TODO: Use extern + linker magic instead of static variable.
> + */
> + return First;
> }
>
> -void llvm_pgo_register_writeout_function(writeout_fn fn) {
> - struct writeout_fn_node *new_node = malloc(sizeof(struct writeout_fn_node));
> - new_node->fn = fn;
> - new_node->next = NULL;
> +/*! \brief Get the last instrumentation record.
> + */
> +static DataType *getLast() {
> + /* TODO: Use extern + linker magic instead of static variable.
> + */
> + return Final + 1;
> +}
>
> - if (!writeout_fn_head) {
> - writeout_fn_head = writeout_fn_tail = new_node;
> - } else {
> - writeout_fn_tail->next = new_node;
> - writeout_fn_tail = new_node;
> - }
> +/* TODO: void __llvm_pgo_get_size_for_buffer(void); */
> +/* TODO: void __llvm_pgo_write_buffer(char *Buffer); */
> +
> +static void writeFunction(FILE *OutputFile, const DataType *Data) {
> + /* TODO: Requires userspace: break requirement by writing directly to a
> + * buffer instead of a FILE stream.
> + */
> + uint32_t I;
> + for (I = 0; I < Data->NameSize; ++I)
> + fputc(Data->Name[I], OutputFile);
> + fprintf(OutputFile, " %u\n", Data->NumCounters);
> + for (I = 0; I < Data->NumCounters; ++I)
> + fprintf(OutputFile, "%" PRIu64 "\n", Data->Counters[I]);
> + fprintf(OutputFile, "\n");
> }
>
> -void llvm_pgo_writeout_files() {
> - const char *OutputName = getenv("LLVM_PROFILE_FILE");
> - if (OutputName == NULL || OutputName[0] == '\0')
> - OutputName = "default.profdata";
> +/*! \brief Write instrumentation data to the given file.
> + */
> +void __llvm_pgo_write_file(const char *OutputName) {
> + /* TODO: Requires userspace: move to separate file.
> + */
> + DataType *I, *E;
> + FILE *OutputFile;
> + if (!OutputName || !OutputName[0])
> + return;
> OutputFile = fopen(OutputName, "w");
> if (!OutputFile) return;
>
> - while (writeout_fn_head) {
> - struct writeout_fn_node *node = writeout_fn_head;
> - writeout_fn_head = writeout_fn_head->next;
> - node->fn();
> - free(node);
> - }
> + /* TODO: mmap file to buffer of size __llvm_pgo_get_size_for_buffer() and
> + * call __llvm_pgo_write_buffer().
> + */
> + for (I = getFirst(), E = getLast(); I != E; ++I)
> + writeFunction(OutputFile, I);
>
> fclose(OutputFile);
> }
>
> -void llvm_pgo_init(writeout_fn wfn) {
> - static int atexit_ran = 0;
> -
> - if (wfn)
> - llvm_pgo_register_writeout_function(wfn);
> +/*! \brief Write instrumentation data to the default file.
> + */
> +void __llvm_pgo_write_default_file() {
> + /* TODO: Requires userspace: move to separate file.
> + */
> + const char *OutputName = getenv("LLVM_PROFILE_FILE");
> + if (OutputName == NULL || OutputName[0] == '\0')
> + OutputName = "default.profdata";
> + __llvm_pgo_write_file(OutputName);
> +}
>
> - if (atexit_ran == 0) {
> - atexit_ran = 1;
> - atexit(llvm_pgo_writeout_files);
> +/*! \brief Register to write instrumentation data to the default file at exit.
> + */
> +void __llvm_pgo_register_write_atexit() {
> + /* TODO: Requires userspace: move to separate file.
> + */
> + static int WriteAtExit = 0;
Past tense was more clear here. If you don't like "ran", something like
"RegisteredAtExit" would be fine.
> +
> + if (!WriteAtExit) {
> + WriteAtExit = 1;
> + atexit(__llvm_pgo_write_default_file);
> }
> }
> +
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list