diff --git a/ddmd/globals.d b/ddmd/globals.d index dce8ede0721..23b1f07c1bb 100644 --- a/ddmd/globals.d +++ b/ddmd/globals.d @@ -232,7 +232,6 @@ struct Param bool cleanupObjectFiles; // Profile-guided optimization: - bool genInstrProf; // Whether to generate PGO instrumented code const(char)* datafileInstrProf; // Either the input or output file for PGO data // target stuff diff --git a/ddmd/globals.h b/ddmd/globals.h index c51d42ad0aa..242362a39b1 100644 --- a/ddmd/globals.h +++ b/ddmd/globals.h @@ -228,7 +228,6 @@ struct Param bool cleanupObjectFiles; // Profile-guided optimization: - bool genInstrProf; // Whether to generate PGO instrumented code const char *datafileInstrProf; // Either the input or output file for PGO data const llvm::Triple *targetTriple; diff --git a/driver/cl_options_instrumentation.cpp b/driver/cl_options_instrumentation.cpp index ac69150af14..57767f6762b 100644 --- a/driver/cl_options_instrumentation.cpp +++ b/driver/cl_options_instrumentation.cpp @@ -15,22 +15,48 @@ #include "driver/cl_options_instrumentation.h" +#include "errors.h" #include "globals.h" -namespace opts { +namespace { +namespace cl = llvm::cl; -cl::opt - genfileInstrProf("fprofile-instr-generate", cl::value_desc("filename"), - cl::desc("Generate instrumented code to collect a runtime " - "profile into default.profraw (overriden by " - "'=' or LLVM_PROFILE_FILE env var)"), - cl::ZeroOrMore, cl::ValueOptional); +#if LDC_LLVM_VER >= 309 +/// Option for generating IR-based PGO instrumentation (LLVM pass) +cl::opt IRPGOInstrGenFile( + "fprofile-generate", cl::value_desc("filename"), + cl::desc("Generate instrumented code to collect a runtime " + "profile into default.profraw (overriden by " + "'=' or LLVM_PROFILE_FILE env var)"), + cl::ZeroOrMore, cl::ValueOptional); -cl::opt usefileInstrProf( +/// Option for generating IR-based PGO instrumentation (LLVM pass) +cl::opt IRPGOInstrUseFile( + "fprofile-use", cl::ZeroOrMore, cl::value_desc("filename"), + cl::desc("Use instrumentation data for profile-guided optimization"), + cl::ValueRequired); +#endif + +/// Option for generating frontend-based PGO instrumentation +cl::opt ASTPGOInstrGenFile( + "fprofile-instr-generate", cl::value_desc("filename"), + cl::desc("Generate instrumented code to collect a runtime " + "profile into default.profraw (overriden by " + "'=' or LLVM_PROFILE_FILE env var)"), + cl::ZeroOrMore, cl::ValueOptional); + +/// Option for generating frontend-based PGO instrumentation +cl::opt ASTPGOInstrUseFile( "fprofile-instr-use", cl::ZeroOrMore, cl::value_desc("filename"), cl::desc("Use instrumentation data for profile-guided optimization"), cl::ValueRequired); +} // anonymous namespace + +namespace opts { + +PGOKind pgoMode = PGO_None; + cl::opt instrumentFunctions("finstrument-functions", cl::ZeroOrMore, cl::desc("Instrument function entry and exit with " @@ -42,9 +68,9 @@ static cl::opt dmdFunctionTrace( cl::desc("DMD-style runtime performance profiling of generated code")); void initializeInstrumentationOptionsFromCmdline() { - if (genfileInstrProf.getNumOccurrences() > 0) { - global.params.genInstrProf = true; - if (genfileInstrProf.empty()) { + if (ASTPGOInstrGenFile.getNumOccurrences() > 0) { + pgoMode = PGO_ASTBasedInstr; + if (ASTPGOInstrGenFile.empty()) { #if LDC_LLVM_VER >= 309 // profile-rt provides a default filename by itself global.params.datafileInstrProf = nullptr; @@ -52,13 +78,34 @@ void initializeInstrumentationOptionsFromCmdline() { global.params.datafileInstrProf = "default.profraw"; #endif } else { - initFromPathString(global.params.datafileInstrProf, genfileInstrProf); + initFromPathString(global.params.datafileInstrProf, ASTPGOInstrGenFile); } - } else { - global.params.genInstrProf = false; - // If we don't have to generate instrumentation, we could be given a - // profdata file: - initFromPathString(global.params.datafileInstrProf, usefileInstrProf); + } else if (!ASTPGOInstrUseFile.empty()) { + pgoMode = PGO_ASTBasedUse; + initFromPathString(global.params.datafileInstrProf, ASTPGOInstrUseFile); + } +#if LDC_LLVM_VER >= 309 + else if (IRPGOInstrGenFile.getNumOccurrences() > 0) { + pgoMode = PGO_IRBasedInstr; + if (IRPGOInstrGenFile.empty()) { + global.params.datafileInstrProf = "default_%m.profraw"; + } else { + initFromPathString(global.params.datafileInstrProf, IRPGOInstrGenFile); + } + } else if (!IRPGOInstrUseFile.empty()) { + pgoMode = PGO_IRBasedUse; + initFromPathString(global.params.datafileInstrProf, IRPGOInstrUseFile); + } +#endif + + // There is a bug in (our use of?) LLVM where codegen errors with + // PGO_IRBasedInstr for Windows targets. So disable IRBased PGO on Windows for + // now. + assert(global.params.targetTriple); + if ((pgoMode == PGO_IRBasedInstr) && + global.params.targetTriple->isOSWindows()) { + error(Loc(), + "'-fprofile-generate' is not yet supported for Windows targets."); } if (dmdFunctionTrace) diff --git a/driver/cl_options_instrumentation.h b/driver/cl_options_instrumentation.h index 1a1901f8a7d..240da48f9ff 100644 --- a/driver/cl_options_instrumentation.h +++ b/driver/cl_options_instrumentation.h @@ -21,15 +21,35 @@ namespace opts { namespace cl = llvm::cl; -// PGO options -extern cl::opt genfileInstrProf; -extern cl::opt usefileInstrProf; - extern cl::opt instrumentFunctions; /// This initializes the instrumentation options, and checks the validity of the -/// commandline flags. It should be called only once. +/// commandline flags. targetTriple should be initialized before calling this. +/// It should be called only once. void initializeInstrumentationOptionsFromCmdline(); +enum PGOKind { + PGO_None, + PGO_ASTBasedInstr, + PGO_ASTBasedUse, + PGO_IRBasedInstr, + PGO_IRBasedUse, +}; +extern PGOKind pgoMode; +inline bool isInstrumentingForPGO() { + return pgoMode == PGO_ASTBasedInstr || pgoMode == PGO_IRBasedInstr; +} +inline bool isUsingPGOProfile() { + return pgoMode == PGO_ASTBasedUse || pgoMode == PGO_IRBasedUse; +} +inline bool isInstrumentingForASTBasedPGO() { + return pgoMode == PGO_ASTBasedInstr; +} +inline bool isUsingASTBasedPGOProfile() { return pgoMode == PGO_ASTBasedUse; } +inline bool isInstrumentingForIRBasedPGO() { + return pgoMode == PGO_IRBasedInstr; +} +inline bool isUsingIRBasedPGOProfile() { return pgoMode == PGO_IRBasedUse; } + } // namespace opts #endif // LDC_DRIVER_CL_OPTIONS_INSTRUMENTATION_H diff --git a/driver/codegenerator.cpp b/driver/codegenerator.cpp index e911b63e9d3..b61e2308b8f 100644 --- a/driver/codegenerator.cpp +++ b/driver/codegenerator.cpp @@ -14,6 +14,7 @@ #include "module.h" #include "scope.h" #include "driver/cl_options.h" +#include "driver/cl_options_instrumentation.h" #include "driver/linker.h" #include "driver/toobj.h" #include "gen/logger.h" @@ -68,7 +69,7 @@ createAndSetDiagnosticsOutputFile(IRState &irs, llvm::LLVMContext &ctx, llvm::make_unique(diagnosticsOutputFile->os())); // If there is instrumentation data available, also output function hotness - if (!global.params.genInstrProf && global.params.datafileInstrProf) { + if (opts::isUsingPGOProfile()) { #if LDC_LLVM_VER >= 500 ctx.setDiagnosticsHotnessRequested(true); #else diff --git a/driver/linker-gcc.cpp b/driver/linker-gcc.cpp index d90e4d412ca..2996348f5a9 100644 --- a/driver/linker-gcc.cpp +++ b/driver/linker-gcc.cpp @@ -9,6 +9,7 @@ #include "errors.h" #include "driver/cl_options.h" +#include "driver/cl_options_instrumentation.h" #include "driver/cl_options_sanitizers.h" #include "driver/exe_path.h" #include "driver/tool.h" @@ -337,7 +338,7 @@ void ArgsBuilder::build(llvm::StringRef outputPath, } // Link with profile-rt library when generating an instrumented binary. - if (global.params.genInstrProf) { + if (opts::isInstrumentingForPGO()) { #if LDC_LLVM_VER >= 308 if (global.params.targetTriple->isOSLinux()) { // For Linux, explicitly define __llvm_profile_runtime as undefined @@ -394,7 +395,7 @@ void ArgsBuilder::build(llvm::StringRef outputPath, // instrumented binary. The runtime relies on magic sections, which // would be stripped by gc-section on older version of ld, see bug: // https://sourceware.org/bugzilla/show_bug.cgi?id=19161 - if (!opts::disableLinkerStripDead && !global.params.genInstrProf) { + if (!opts::disableLinkerStripDead && !opts::isInstrumentingForPGO()) { addLdFlag("--gc-sections"); } } diff --git a/driver/linker-msvc.cpp b/driver/linker-msvc.cpp index 10267f3f544..dd10b8eefdc 100644 --- a/driver/linker-msvc.cpp +++ b/driver/linker-msvc.cpp @@ -9,6 +9,7 @@ #include "errors.h" #include "driver/cl_options.h" +#include "driver/cl_options_instrumentation.h" #include "driver/tool.h" #include "gen/logger.h" @@ -110,8 +111,7 @@ int linkObjToBinaryMSVC(llvm::StringRef outputPath, bool useInternalLinker, args.push_back(std::string("/DEF:") + global.params.deffile); // Link with profile-rt library when generating an instrumented binary - // profile-rt depends on Phobos (MD5 hashing). - if (global.params.genInstrProf) { + if (opts::isInstrumentingForPGO()) { args.push_back("ldc-profile-rt.lib"); // profile-rt depends on ws2_32 for symbol `gethostname` args.push_back("ws2_32.lib"); diff --git a/driver/main.cpp b/driver/main.cpp index 2af3dbf5d7d..1831dbb8fac 100644 --- a/driver/main.cpp +++ b/driver/main.cpp @@ -431,7 +431,6 @@ void parseCommandLine(int argc, char **argv, Strings &sourceFiles, } #endif - opts::initializeInstrumentationOptionsFromCmdline(); opts::initializeSanitizerOptionsFromCmdline(); processVersions(debugArgs, "debug", DebugCondition::setGlobalLevel, @@ -1067,6 +1066,8 @@ int cppmain(int argc, char **argv) { global.lib_ext = "a"; } + opts::initializeInstrumentationOptionsFromCmdline(); + Strings libmodules; return mars_mainBody(files, libmodules); } diff --git a/gen/funcgenstate.h b/gen/funcgenstate.h index fdde8fd42f4..b909760199c 100644 --- a/gen/funcgenstate.h +++ b/gen/funcgenstate.h @@ -16,7 +16,7 @@ #define LDC_GEN_FUNCGENSTATE_H #include "gen/irstate.h" -#include "gen/pgo.h" +#include "gen/pgo_ASTbased.h" #include "gen/trycatchfinally.h" #include "llvm/ADT/DenseMap.h" #include "llvm/IR/CallSite.h" diff --git a/gen/functions.cpp b/gen/functions.cpp index a9c16d5c705..9f4374795ef 100644 --- a/gen/functions.cpp +++ b/gen/functions.cpp @@ -36,7 +36,7 @@ #include "gen/mangling.h" #include "gen/nested.h" #include "gen/optimizer.h" -#include "gen/pgo.h" +#include "gen/pgo_ASTbased.h" #include "gen/pragma.h" #include "gen/runtime.h" #include "gen/dynamiccompile.h" diff --git a/gen/modules.cpp b/gen/modules.cpp index 602d9efd8ff..7dce15bf43e 100644 --- a/gen/modules.cpp +++ b/gen/modules.cpp @@ -22,6 +22,7 @@ #include "statement.h" #include "target.h" #include "template.h" +#include "driver/cl_options_instrumentation.h" #include "gen/abi.h" #include "gen/arrays.h" #include "gen/functions.h" @@ -611,9 +612,8 @@ void addCoverageAnalysisInitializer(Module *m) { // TODO: This is probably not the right place, we should load it once for all // modules? void loadInstrProfileData(IRState *irs) { - // Only load from datafileInstrProf if we are not generating instrumented - // code. - if (!global.params.genInstrProf && global.params.datafileInstrProf) { + // Only load from datafileInstrProf if we are doing frontend-based PGO. + if (opts::isUsingASTBasedPGOProfile() && global.params.datafileInstrProf) { IF_LOG Logger::println("Read profile data from %s", global.params.datafileInstrProf); diff --git a/gen/optimizer.cpp b/gen/optimizer.cpp index 400dd7ce7db..82e0e5dd4b9 100644 --- a/gen/optimizer.cpp +++ b/gen/optimizer.cpp @@ -14,6 +14,7 @@ #include "gen/logger.h" #include "gen/passes/Passes.h" #include "driver/cl_options.h" +#include "driver/cl_options_instrumentation.h" #include "driver/cl_options_sanitizers.h" #include "driver/targetmachine.h" #include "llvm/LinkAllPasses.h" @@ -206,9 +207,9 @@ static void addSanitizerCoveragePass(const PassManagerBuilder &Builder, } // Adds PGO instrumentation generation and use passes. -static void addPGOPasses(legacy::PassManagerBase &mpm, unsigned optLevel) { - if (global.params.genInstrProf) { - // We are generating PGO instrumented code. +static void addPGOPasses(PassManagerBuilder &builder, + legacy::PassManagerBase &mpm, unsigned optLevel) { + if (opts::isInstrumentingForASTBasedPGO()) { InstrProfOptions options; options.NoRedZone = global.params.disableRedZone; if (global.params.datafileInstrProf) @@ -218,7 +219,7 @@ static void addPGOPasses(legacy::PassManagerBase &mpm, unsigned optLevel) { #else mpm.add(createInstrProfilingPass(options)); #endif - } else if (global.params.datafileInstrProf) { + } else if (opts::isUsingASTBasedPGOProfile()) { // We are generating code with PGO profile information available. #if LDC_LLVM_VER >= 500 // Do indirect call promotion from -O1 @@ -227,6 +228,16 @@ static void addPGOPasses(legacy::PassManagerBase &mpm, unsigned optLevel) { } #endif } +#if LDC_LLVM_VER >= 309 + else if (opts::isInstrumentingForIRBasedPGO()) { +#if LDC_LLVM_VER >= 400 + builder.EnablePGOInstrGen = true; +#endif + builder.PGOInstrGen = global.params.datafileInstrProf; + } else if (opts::isUsingIRBasedPGOProfile()) { + builder.PGOInstrUse = global.params.datafileInstrProf; + } +#endif } /** @@ -328,7 +339,7 @@ static void addOptimizationPasses(legacy::PassManagerBase &mpm, builder.addExtension(PassManagerBuilder::EP_OptimizerLast, addStripExternalsPass); - addPGOPasses(mpm, optLevel); + addPGOPasses(builder, mpm, optLevel); builder.populateFunctionPassManager(fpm); builder.populateModulePassManager(mpm); diff --git a/gen/pgo.cpp b/gen/pgo_ASTbased.cpp similarity index 98% rename from gen/pgo.cpp rename to gen/pgo_ASTbased.cpp index 3677c54e647..06fdccf33f2 100644 --- a/gen/pgo.cpp +++ b/gen/pgo_ASTbased.cpp @@ -1,4 +1,4 @@ -//===-- gen/pgo.cpp ---------------------------------------------*- C++ -*-===// +//===-- gen/pgo_ASTbased.cpp ------------------------------------*- C++ -*-===// // // LDC – the LLVM D compiler // @@ -8,16 +8,18 @@ // //===----------------------------------------------------------------------===// // -// Instrumentation-based profile-guided optimization +// Instrumentation-based profile-guided optimization. This is AST-based PGO, in +// contrast to LLVM's IR-based PGO. // //===----------------------------------------------------------------------===// -#include "gen/pgo.h" +#include "gen/pgo_ASTbased.h" #include "globals.h" #include "init.h" #include "statement.h" #include "llvm.h" +#include "driver/cl_options_instrumentation.h" #include "gen/cl_helpers.h" #include "gen/irstate.h" #include "gen/logger.h" @@ -767,7 +769,7 @@ void CodeGenPGO::setFuncName(llvm::StringRef Name, : llvm::IndexedInstrProf::Version); // If we're generating a profile, create a variable for the name. - if (global.params.genInstrProf && emitInstrumentation) { + if (opts::isInstrumentingForASTBasedPGO() && emitInstrumentation) { FuncNameVar = llvm::createPGOFuncNameVar(gIR->module, Linkage, FuncName); // If Linkage is private, and the function is in a comdat "any" group, set @@ -788,7 +790,7 @@ void CodeGenPGO::setFuncName(llvm::StringRef Name, FuncName = RawFuncName; // If we're generating a profile, create a variable for the name. - if (global.params.genInstrProf && emitInstrumentation) + if (opts::isInstrumentingForASTBasedPGO() && emitInstrumentation) createFuncNameVar(Linkage); #endif } @@ -825,7 +827,7 @@ void CodeGenPGO::createFuncNameVar(llvm::GlobalValue::LinkageTypes Linkage) { void CodeGenPGO::assignRegionCounters(const FuncDeclaration *D, llvm::Function *fn) { llvm::IndexedInstrProfReader *PGOReader = gIR->getPGOReader(); - if (!global.params.genInstrProf && !PGOReader) + if (!opts::isInstrumentingForASTBasedPGO() && !PGOReader) return; emitInstrumentation = D->emitInstrumentation; @@ -867,7 +869,8 @@ void CodeGenPGO::applyFunctionAttributes(llvm::Function *Fn) { } void CodeGenPGO::emitCounterIncrement(const RootObject *S) const { - if (!global.params.genInstrProf || !RegionCounterMap || !emitInstrumentation) + if (!opts::isInstrumentingForASTBasedPGO() || !RegionCounterMap || + !emitInstrumentation) return; auto counter_it = (*RegionCounterMap).find(S); @@ -1080,7 +1083,8 @@ void CodeGenPGO::valueProfile(uint32_t valueKind, llvm::Instruction *valueSite, if (!value || !valueSite) return; - bool instrumentValueSites = global.params.genInstrProf && emitInstrumentation; + bool instrumentValueSites = + opts::isInstrumentingForASTBasedPGO() && emitInstrumentation; if (instrumentValueSites && RegionCounterMap) { // Instrumentation must be inserted just before the valueSite instruction. // Save the current insertion point to be able to restore it later. diff --git a/gen/pgo.h b/gen/pgo_ASTbased.h similarity index 97% rename from gen/pgo.h rename to gen/pgo_ASTbased.h index 27027841757..bd4a936c858 100644 --- a/gen/pgo.h +++ b/gen/pgo_ASTbased.h @@ -1,4 +1,4 @@ -//===-- gen/pgo.h - Code Coverage Analysis ----------------------*- C++ -*-===// +//===-- gen/pgo_ASTbased.h - Code Coverage Analysis -------------*- C++ -*-===// // // LDC – the LLVM D compiler // @@ -9,14 +9,14 @@ //===----------------------------------------------------------------------===// // // This file contains functions to generate instrumentation code for -// profile-guided optimization. +// AST-based profile-guided optimization. // PGO is enabled by compiling first with "-fprofile-instr-generate", // and then with "-fprofile-instr-use=filename.profdata". // //===----------------------------------------------------------------------===// -#ifndef LDC_GEN_PGO_H -#define LDC_GEN_PGO_H +#ifndef LDC_GEN_PGO_ASTBASED_H +#define LDC_GEN_PGO_ASTBASED_H #include "gen/llvm.h" #include "llvm/ProfileData/InstrProf.h" @@ -183,4 +183,4 @@ class CodeGenPGO { const FuncDeclaration *D); }; -#endif // LDC_GEN_PGO_H +#endif // LDC_GEN_PGO_ASTBASED_H diff --git a/tests/PGO/irbased_indirect_calls.d b/tests/PGO/irbased_indirect_calls.d new file mode 100644 index 00000000000..e940e2fa787 --- /dev/null +++ b/tests/PGO/irbased_indirect_calls.d @@ -0,0 +1,62 @@ +// Test instrumentation of indirect calls + +// REQUIRES: atleast_llvm309 + +// There is an LLVM bug, this test currently errors during LLVM codegen for Windows. +// XFAIL: Windows + +// RUN: %ldc -O3 -fprofile-generate=%t.profraw -run %s \ +// RUN: && %profdata merge %t.profraw -o %t.profdata \ +// RUN: && %ldc -O3 -c -output-ll -of=%t.use.ll -fprofile-use=%t.profdata %s \ +// RUN: && FileCheck %s -check-prefix=PROFUSE < %t.use.ll + +import ldc.attributes : weak; + +extern (C) +{ // simplify name mangling for simpler string matching + + @weak // disable reasoning about this function + void hot() + { + } + + void luke() + { + } + + void cold() + { + } + + void function() foo; + + @weak // disable reasoning about this function + void select_func(int i) + { + if (i < 1700) + foo = &hot; + else if (i < 1990) + foo = &luke; + else + foo = &cold; + } + +} // extern C + +// PROFUSE-LABEL: @_Dmain( +int main() +{ + for (int i; i < 2000; ++i) + { + select_func(i); + + // PROFUSE: [[REG1:%[0-9]+]] = load void ()*, void ()** @foo + // PROFUSE: [[REG2:%[0-9]+]] = icmp eq void ()* [[REG1]], @hot + // PROFUSE: call void @hot() + // PROFUSE: call void [[REG1]]() + + foo(); + } + + return 0; +}