From e52a811c3b643548837b4e630e8293a0b6857ad4 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Fri, 13 Oct 2023 14:40:28 -0700 Subject: [PATCH 1/6] [PGO] Add ability to mark cold functions as optsize/minsize/optnone The performance of cold functions shouldn't matter too much, so if we care about binary sizes, add an option to mark cold functions as optsize/minsize for binary size, or optnone for compile times [1]. Clang patch will be in a future patch Initial version: https://reviews.llvm.org/D149800 [1] https://discourse.llvm.org/t/rfc-new-feature-proposal-de-optimizing-cold-functions-using-pgo-info/56388 --- clang/lib/CodeGen/BackendUtil.cpp | 18 ++-- llvm/include/llvm/Support/PGOOptions.h | 3 + .../Instrumentation/MarkColdFunctions.h | 28 ++++++ llvm/lib/LTO/LTOBackend.cpp | 12 ++- llvm/lib/Passes/PassBuilder.cpp | 1 + llvm/lib/Passes/PassBuilderPipelines.cpp | 12 +++ llvm/lib/Passes/PassRegistry.def | 1 + llvm/lib/Support/PGOOptions.cpp | 7 +- .../Transforms/Instrumentation/CMakeLists.txt | 1 + .../Instrumentation/MarkColdFunctions.cpp | 65 +++++++++++++ .../Transforms/MarkColdFunctions/basic.ll | 97 +++++++++++++++++++ llvm/tools/opt/NewPMDriver.cpp | 24 ++++- .../lib/Transforms/Instrumentation/BUILD.gn | 1 + 13 files changed, 252 insertions(+), 18 deletions(-) create mode 100644 llvm/include/llvm/Transforms/Instrumentation/MarkColdFunctions.h create mode 100644 llvm/lib/Transforms/Instrumentation/MarkColdFunctions.cpp create mode 100644 llvm/test/Transforms/MarkColdFunctions/basic.ll diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index a6142d99f3b688..7e9d3b8ea55a18 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -746,7 +746,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( CodeGenOpts.InstrProfileOutput.empty() ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", "", CodeGenOpts.MemoryProfileUsePath, nullptr, PGOOptions::IRInstr, - PGOOptions::NoCSAction, CodeGenOpts.DebugInfoForProfiling, + PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, + CodeGenOpts.DebugInfoForProfiling, /*PseudoProbeForProfiling=*/false, CodeGenOpts.AtomicProfileUpdate); else if (CodeGenOpts.hasProfileIRUse()) { // -fprofile-use. @@ -755,28 +756,32 @@ void EmitAssemblyHelper::RunOptimizationPipeline( PGOOpt = PGOOptions( CodeGenOpts.ProfileInstrumentUsePath, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, - PGOOptions::IRUse, CSAction, CodeGenOpts.DebugInfoForProfiling); + PGOOptions::IRUse, CSAction, PGOOptions::ColdFuncAttr::None, + CodeGenOpts.DebugInfoForProfiling); } else if (!CodeGenOpts.SampleProfileFile.empty()) // -fprofile-sample-use PGOOpt = PGOOptions( CodeGenOpts.SampleProfileFile, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::SampleUse, - PGOOptions::NoCSAction, CodeGenOpts.DebugInfoForProfiling, - CodeGenOpts.PseudoProbeForProfiling); + PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, + CodeGenOpts.DebugInfoForProfiling, CodeGenOpts.PseudoProbeForProfiling); else if (!CodeGenOpts.MemoryProfileUsePath.empty()) // -fmemory-profile-use (without any of the above options) PGOOpt = PGOOptions("", "", "", CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::NoAction, PGOOptions::NoCSAction, + PGOOptions::ColdFuncAttr::None, CodeGenOpts.DebugInfoForProfiling); else if (CodeGenOpts.PseudoProbeForProfiling) // -fpseudo-probe-for-profiling PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction, PGOOptions::NoCSAction, + PGOOptions::ColdFuncAttr::None, CodeGenOpts.DebugInfoForProfiling, true); else if (CodeGenOpts.DebugInfoForProfiling) // -fdebug-info-for-profiling PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, - PGOOptions::NoAction, PGOOptions::NoCSAction, true); + PGOOptions::NoAction, PGOOptions::NoCSAction, + PGOOptions::ColdFuncAttr::None, true); // Check to see if we want to generate a CS profile. if (CodeGenOpts.hasProfileCSIRInstr()) { @@ -799,7 +804,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction, - PGOOptions::CSIRInstr, CodeGenOpts.DebugInfoForProfiling); + PGOOptions::CSIRInstr, PGOOptions::ColdFuncAttr::None, + CodeGenOpts.DebugInfoForProfiling); } if (TM) TM->setPGOOption(PGOOpt); diff --git a/llvm/include/llvm/Support/PGOOptions.h b/llvm/include/llvm/Support/PGOOptions.h index 87eb29a8de48a0..fe21169a137016 100644 --- a/llvm/include/llvm/Support/PGOOptions.h +++ b/llvm/include/llvm/Support/PGOOptions.h @@ -27,10 +27,12 @@ class FileSystem; struct PGOOptions { enum PGOAction { NoAction, IRInstr, IRUse, SampleUse }; enum CSPGOAction { NoCSAction, CSIRInstr, CSIRUse }; + enum class ColdFuncAttr { None, OptSize, MinSize, OptNone }; PGOOptions(std::string ProfileFile, std::string CSProfileGenFile, std::string ProfileRemappingFile, std::string MemoryProfile, IntrusiveRefCntPtr FS, PGOAction Action = NoAction, CSPGOAction CSAction = NoCSAction, + ColdFuncAttr ColdType = ColdFuncAttr::None, bool DebugInfoForProfiling = false, bool PseudoProbeForProfiling = false, bool AtomicCounterUpdate = false); @@ -44,6 +46,7 @@ struct PGOOptions { std::string MemoryProfile; PGOAction Action; CSPGOAction CSAction; + ColdFuncAttr ColdType; bool DebugInfoForProfiling; bool PseudoProbeForProfiling; bool AtomicCounterUpdate; diff --git a/llvm/include/llvm/Transforms/Instrumentation/MarkColdFunctions.h b/llvm/include/llvm/Transforms/Instrumentation/MarkColdFunctions.h new file mode 100644 index 00000000000000..06aecc911d8827 --- /dev/null +++ b/llvm/include/llvm/Transforms/Instrumentation/MarkColdFunctions.h @@ -0,0 +1,28 @@ +//===- MarkColdFunctions.h - ------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_MARKCOLDFUNCTIONS_H +#define LLVM_TRANSFORMS_INSTRUMENTATION_MARKCOLDFUNCTIONS_H + +#include "llvm/IR/PassManager.h" +#include "llvm/Support/PGOOptions.h" + +namespace llvm { + +struct MarkColdFunctionsPass : public PassInfoMixin { + MarkColdFunctionsPass(PGOOptions::ColdFuncAttr ColdType) + : ColdType(ColdType) {} + PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM); + +private: + PGOOptions::ColdFuncAttr ColdType; +}; + +} // namespace llvm + +#endif // LLVM_TRANSFORMS_INSTRUMENTATION_MARKCOLDFUNCTIONS_H diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp index ccc4276e36dacf..8126b3563e077d 100644 --- a/llvm/lib/LTO/LTOBackend.cpp +++ b/llvm/lib/LTO/LTOBackend.cpp @@ -243,19 +243,23 @@ static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM, if (!Conf.SampleProfile.empty()) PGOOpt = PGOOptions(Conf.SampleProfile, "", Conf.ProfileRemapping, /*MemoryProfile=*/"", FS, PGOOptions::SampleUse, - PGOOptions::NoCSAction, true); + PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, + true); else if (Conf.RunCSIRInstr) { PGOOpt = PGOOptions("", Conf.CSIRProfile, Conf.ProfileRemapping, /*MemoryProfile=*/"", FS, PGOOptions::IRUse, - PGOOptions::CSIRInstr, Conf.AddFSDiscriminator); + PGOOptions::CSIRInstr, PGOOptions::ColdFuncAttr::None, + Conf.AddFSDiscriminator); } else if (!Conf.CSIRProfile.empty()) { PGOOpt = PGOOptions(Conf.CSIRProfile, "", Conf.ProfileRemapping, /*MemoryProfile=*/"", FS, PGOOptions::IRUse, - PGOOptions::CSIRUse, Conf.AddFSDiscriminator); + PGOOptions::CSIRUse, PGOOptions::ColdFuncAttr::None, + Conf.AddFSDiscriminator); NoPGOWarnMismatch = !Conf.PGOWarnMismatch; } else if (Conf.AddFSDiscriminator) { PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, - PGOOptions::NoAction, PGOOptions::NoCSAction, true); + PGOOptions::NoAction, PGOOptions::NoCSAction, + PGOOptions::ColdFuncAttr::None, true); } TM->setPGOOption(PGOOpt); diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp index 19fb136f37563a..a4ea91794b9c97 100644 --- a/llvm/lib/Passes/PassBuilder.cpp +++ b/llvm/lib/Passes/PassBuilder.cpp @@ -167,6 +167,7 @@ #include "llvm/Transforms/Instrumentation/InstrOrderFile.h" #include "llvm/Transforms/Instrumentation/InstrProfiling.h" #include "llvm/Transforms/Instrumentation/KCFI.h" +#include "llvm/Transforms/Instrumentation/MarkColdFunctions.h" #include "llvm/Transforms/Instrumentation/MemProfiler.h" #include "llvm/Transforms/Instrumentation/MemorySanitizer.h" #include "llvm/Transforms/Instrumentation/PGOInstrumentation.h" diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index 525b83dee79b0d..6ddeb8d675b94d 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -73,6 +73,7 @@ #include "llvm/Transforms/Instrumentation/ControlHeightReduction.h" #include "llvm/Transforms/Instrumentation/InstrOrderFile.h" #include "llvm/Transforms/Instrumentation/InstrProfiling.h" +#include "llvm/Transforms/Instrumentation/MarkColdFunctions.h" #include "llvm/Transforms/Instrumentation/MemProfiler.h" #include "llvm/Transforms/Instrumentation/PGOInstrumentation.h" #include "llvm/Transforms/Scalar/ADCE.h" @@ -212,6 +213,12 @@ static cl::opt cl::desc("Enable DFA jump threading"), cl::init(false), cl::Hidden); +// TODO: turn on and remove flag +static cl::opt + EnableMarkColdFunctions("enable-mark-cold-functions", + cl::desc("Enable pass to mark cold functions"), + cl::init(false)); + static cl::opt EnableHotColdSplit("hot-cold-split", cl::desc("Enable hot-cold splitting pass")); @@ -1137,6 +1144,11 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, if (EnableSyntheticCounts && !PGOOpt) MPM.addPass(SyntheticCountsPropagation()); + if (EnableMarkColdFunctions && PGOOpt && + (PGOOpt->Action == PGOOptions::SampleUse || + PGOOpt->Action == PGOOptions::IRUse)) + MPM.addPass(MarkColdFunctionsPass(PGOOpt->ColdType)); + MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/true)); if (EnableModuleInliner) diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def index 5f253020911241..2e3c56b3ccbc77 100644 --- a/llvm/lib/Passes/PassRegistry.def +++ b/llvm/lib/Passes/PassRegistry.def @@ -86,6 +86,7 @@ MODULE_PASS("lower-emutls", LowerEmuTLSPass()) MODULE_PASS("lower-global-dtors", LowerGlobalDtorsPass()) MODULE_PASS("lower-ifunc", LowerIFuncPass()) MODULE_PASS("lowertypetests", LowerTypeTestsPass()) +MODULE_PASS("mark-cold-functions", MarkColdFunctionsPass(PGOOpt ? PGOOpt->ColdType : PGOOptions::ColdFuncAttr::None)) MODULE_PASS("memprof-context-disambiguation", MemProfContextDisambiguation()) MODULE_PASS("memprof-module", ModuleMemProfilerPass()) MODULE_PASS("mergefunc", MergeFunctionsPass()) diff --git a/llvm/lib/Support/PGOOptions.cpp b/llvm/lib/Support/PGOOptions.cpp index 7e57b52e4ba2f5..5dfeafa188980e 100644 --- a/llvm/lib/Support/PGOOptions.cpp +++ b/llvm/lib/Support/PGOOptions.cpp @@ -15,11 +15,12 @@ PGOOptions::PGOOptions(std::string ProfileFile, std::string CSProfileGenFile, std::string ProfileRemappingFile, std::string MemoryProfile, IntrusiveRefCntPtr FS, PGOAction Action, - CSPGOAction CSAction, bool DebugInfoForProfiling, - bool PseudoProbeForProfiling, bool AtomicCounterUpdate) + CSPGOAction CSAction, ColdFuncAttr ColdType, + bool DebugInfoForProfiling, bool PseudoProbeForProfiling, + bool AtomicCounterUpdate) : ProfileFile(ProfileFile), CSProfileGenFile(CSProfileGenFile), ProfileRemappingFile(ProfileRemappingFile), MemoryProfile(MemoryProfile), - Action(Action), CSAction(CSAction), + Action(Action), CSAction(CSAction), ColdType(ColdType), DebugInfoForProfiling(DebugInfoForProfiling || (Action == SampleUse && !PseudoProbeForProfiling)), PseudoProbeForProfiling(PseudoProbeForProfiling), diff --git a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt index 424f1d43360677..b704726dd78037 100644 --- a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt +++ b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt @@ -6,6 +6,7 @@ add_llvm_component_library(LLVMInstrumentation DataFlowSanitizer.cpp GCOVProfiling.cpp BlockCoverageInference.cpp + MarkColdFunctions.cpp MemProfiler.cpp MemorySanitizer.cpp IndirectCallPromotion.cpp diff --git a/llvm/lib/Transforms/Instrumentation/MarkColdFunctions.cpp b/llvm/lib/Transforms/Instrumentation/MarkColdFunctions.cpp new file mode 100644 index 00000000000000..9037607132ef41 --- /dev/null +++ b/llvm/lib/Transforms/Instrumentation/MarkColdFunctions.cpp @@ -0,0 +1,65 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "llvm/Transforms/Instrumentation/MarkColdFunctions.h" +#include "llvm/Analysis/BlockFrequencyInfo.h" +#include "llvm/Analysis/ProfileSummaryInfo.h" +#include "llvm/IR/PassManager.h" + +using namespace llvm; + +PreservedAnalyses MarkColdFunctionsPass::run(Module &M, + ModuleAnalysisManager &AM) { + if (ColdType == PGOOptions::ColdFuncAttr::None) + return PreservedAnalyses::all(); + ProfileSummaryInfo &PSI = AM.getResult(M); + if (!PSI.hasProfileSummary()) + return PreservedAnalyses::all(); + FunctionAnalysisManager &FAM = + AM.getResult(M).getManager(); + bool MadeChange = false; + for (Function &F : M) { + if (F.isDeclaration()) + continue; + BlockFrequencyInfo &BFI = FAM.getResult(F); + if (!PSI.isFunctionColdInCallGraph(&F, BFI)) + continue; + // Add optsize/minsize/optnone if requested. + switch (ColdType) { + case PGOOptions::ColdFuncAttr::None: + assert(false); + break; + case PGOOptions::ColdFuncAttr::OptSize: + if (!F.hasFnAttribute(Attribute::OptimizeNone) && + !F.hasFnAttribute(Attribute::OptimizeForSize) && + !F.hasFnAttribute(Attribute::MinSize)) { + F.addFnAttr(Attribute::OptimizeForSize); + MadeChange = true; + } + break; + case PGOOptions::ColdFuncAttr::MinSize: + // Change optsize to minsize. + if (!F.hasFnAttribute(Attribute::OptimizeNone) && + !F.hasFnAttribute(Attribute::MinSize)) { + F.removeFnAttr(Attribute::OptimizeForSize); + F.addFnAttr(Attribute::MinSize); + MadeChange = true; + } + break; + case PGOOptions::ColdFuncAttr::OptNone: + // Strip optsize/minsize. + F.removeFnAttr(Attribute::OptimizeForSize); + F.removeFnAttr(Attribute::MinSize); + F.addFnAttr(Attribute::OptimizeNone); + F.addFnAttr(Attribute::NoInline); + MadeChange = true; + break; + } + } + return MadeChange ? PreservedAnalyses::none() : PreservedAnalyses::all(); +} diff --git a/llvm/test/Transforms/MarkColdFunctions/basic.ll b/llvm/test/Transforms/MarkColdFunctions/basic.ll new file mode 100644 index 00000000000000..f79e3a8f1dda7d --- /dev/null +++ b/llvm/test/Transforms/MarkColdFunctions/basic.ll @@ -0,0 +1,97 @@ +; RUN: opt < %s -passes=mark-cold-functions -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-attr=none | FileCheck %s --check-prefixes=NONE,CHECK +; RUN: opt < %s -passes=mark-cold-functions -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-attr=optsize | FileCheck %s --check-prefixes=OPTSIZE,CHECK +; RUN: opt < %s -passes=mark-cold-functions -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-attr=minsize | FileCheck %s --check-prefixes=MINSIZE,CHECK +; RUN: opt < %s -passes=mark-cold-functions -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-attr=optnone | FileCheck %s --check-prefixes=OPTNONE,CHECK + +; Should be no changes without profile data +; RUN: opt < %s -passes=mark-cold-functions -S -pgo-cold-func-attr=minsize | FileCheck %s --check-prefixes=NONE,CHECK + +; NONE-NOT: Function Attrs: +; OPTSIZE: Function Attrs: optsize{{$}} +; MINSIZE: Function Attrs: minsize{{$}} +; OPTNONE: Function Attrs: noinline optnone{{$}} +; CHECK: define void @cold() + +; NONE: Function Attrs: optsize{{$}} +; OPTSIZE: Function Attrs: optsize{{$}} +; MINSIZE: Function Attrs: minsize{{$}} +; OPTNONE: Function Attrs: noinline optnone{{$}} +; CHECK-NEXT: define void @cold1() + +; NONE: Function Attrs: minsize{{$}} +; OPTSIZE: Function Attrs: minsize{{$}} +; MINSIZE: Function Attrs: minsize{{$}} +; OPTNONE: Function Attrs: noinline optnone{{$}} +; CHECK-NEXT: define void @cold2() + +; CHECK: Function Attrs: noinline optnone{{$}} +; CHECK-NEXT: define void @cold3() + +; CHECK-NOT: Function Attrs: {{.*}}optsize +; CHECK-NOT: Function Attrs: {{.*}}minsize +; CHECK-NOT: Function Attrs: {{.*}}optnone + +@s = global i32 0 + +define void @cold() !prof !27 { + store i32 1, ptr @s, align 4 + ret void +} + +define void @cold1() optsize !prof !27 { + store i32 1, ptr @s, align 4 + ret void +} + +define void @cold2() minsize !prof !27 { + store i32 1, ptr @s, align 4 + ret void +} + +define void @cold3() noinline optnone !prof !27 { + store i32 1, ptr @s, align 4 + ret void +} + +define void @hot() !prof !28 { + %l = load i32, ptr @s, align 4 + %add = add nsw i32 %l, 4 + store i32 %add, ptr @s, align 4 + ret void +} + +attributes #0 = { optsize } +attributes #1 = { minsize } +attributes #2 = { noinline optnone } + +!llvm.module.flags = !{!0} + +!0 = !{i32 1, !"ProfileSummary", !1} +!1 = !{!2, !3, !4, !5, !6, !7, !8, !9} +!2 = !{!"ProfileFormat", !"InstrProf"} +!3 = !{!"TotalCount", i64 9040} +!4 = !{!"MaxCount", i64 9000} +!5 = !{!"MaxInternalCount", i64 0} +!6 = !{!"MaxFunctionCount", i64 9000} +!7 = !{!"NumCounts", i64 5} +!8 = !{!"NumFunctions", i64 5} +!9 = !{!"DetailedSummary", !10} +!10 = !{!11, !12, !13, !14, !15, !16, !17, !18, !19, !20, !21, !22, !23, !24, !25, !26} +!11 = !{i32 10000, i64 9000, i32 1} +!12 = !{i32 100000, i64 9000, i32 1} +!13 = !{i32 200000, i64 9000, i32 1} +!14 = !{i32 300000, i64 9000, i32 1} +!15 = !{i32 400000, i64 9000, i32 1} +!16 = !{i32 500000, i64 9000, i32 1} +!17 = !{i32 600000, i64 9000, i32 1} +!18 = !{i32 700000, i64 9000, i32 1} +!19 = !{i32 800000, i64 9000, i32 1} +!20 = !{i32 900000, i64 9000, i32 1} +!21 = !{i32 950000, i64 9000, i32 1} +!22 = !{i32 990000, i64 9000, i32 1} +!23 = !{i32 999000, i64 10, i32 5} +!24 = !{i32 999900, i64 10, i32 5} +!25 = !{i32 999990, i64 10, i32 5} +!26 = !{i32 999999, i64 10, i32 5} +!27 = !{!"function_entry_count", i64 10} +!28 = !{!"function_entry_count", i64 9000} diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp index 6ae3f87099afd6..228eda8c3e0665 100644 --- a/llvm/tools/opt/NewPMDriver.cpp +++ b/llvm/tools/opt/NewPMDriver.cpp @@ -202,6 +202,18 @@ static cl::opt cl::desc("Path to the profile remapping file."), cl::Hidden); +static cl::opt PGOColdFuncAttr( + "pgo-cold-func-attr", cl::init(PGOOptions::ColdFuncAttr::None), cl::Hidden, + cl::desc( + "Function attribute to apply to cold functions as determined by PGO"), + cl::values(clEnumValN(PGOOptions::ColdFuncAttr::None, "none", "None"), + clEnumValN(PGOOptions::ColdFuncAttr::OptSize, "optsize", + "Mark cold functions with optsize."), + clEnumValN(PGOOptions::ColdFuncAttr::MinSize, "minsize", + "Mark cold functions with minsize."), + clEnumValN(PGOOptions::ColdFuncAttr::OptNone, "optnone", + "Mark cold functions with optnone."))); + static cl::opt DebugInfoForProfiling( "debug-info-for-profiling", cl::init(false), cl::Hidden, cl::desc("Emit special debug info to enable PGO profile generation.")); @@ -340,22 +352,24 @@ bool llvm::runPassPipeline( switch (PGOKindFlag) { case InstrGen: P = PGOOptions(ProfileFile, "", "", MemoryProfileFile, FS, - PGOOptions::IRInstr); + PGOOptions::IRInstr, PGOOptions::NoCSAction, + PGOColdFuncAttr); break; case InstrUse: P = PGOOptions(ProfileFile, "", ProfileRemappingFile, MemoryProfileFile, FS, - PGOOptions::IRUse); + PGOOptions::IRUse, PGOOptions::NoCSAction, PGOColdFuncAttr); break; case SampleUse: P = PGOOptions(ProfileFile, "", ProfileRemappingFile, MemoryProfileFile, FS, - PGOOptions::SampleUse); + PGOOptions::SampleUse, PGOOptions::NoCSAction, + PGOColdFuncAttr); break; case NoPGO: if (DebugInfoForProfiling || PseudoProbeForProfiling || !MemoryProfileFile.empty()) P = PGOOptions("", "", "", MemoryProfileFile, FS, PGOOptions::NoAction, - PGOOptions::NoCSAction, DebugInfoForProfiling, - PseudoProbeForProfiling); + PGOOptions::NoCSAction, PGOColdFuncAttr, + DebugInfoForProfiling, PseudoProbeForProfiling); else P = std::nullopt; } diff --git a/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn b/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn index 0cce6558e0093c..42112878c93a61 100644 --- a/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn +++ b/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn @@ -23,6 +23,7 @@ static_library("Instrumentation") { "InstrProfiling.cpp", "Instrumentation.cpp", "KCFI.cpp", + "MarkColdFunctions.cpp", "MemProfiler.cpp", "MemorySanitizer.cpp", "PGOInstrumentation.cpp", From e1ae305467d3707b5d39f34c7c2a9f47a3f1ceeb Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Tue, 23 Jan 2024 20:49:04 +0000 Subject: [PATCH 2/6] rename a bunch of stuff --- clang/lib/CodeGen/BackendUtil.cpp | 14 +++++++------- llvm/include/llvm/Support/PGOOptions.h | 6 +++--- ...rkColdFunctions.h => PGOForceFunctionAttrs.h} | 15 ++++++++------- llvm/lib/LTO/LTOBackend.cpp | 10 +++++----- llvm/lib/Passes/PassBuilder.cpp | 2 +- llvm/lib/Passes/PassBuilderPipelines.cpp | 14 +++++++------- llvm/lib/Passes/PassRegistry.def | 2 +- llvm/lib/Support/PGOOptions.cpp | 4 ++-- .../Transforms/Instrumentation/CMakeLists.txt | 2 +- ...ldFunctions.cpp => PGOForceFunctionAttrs.cpp} | 16 ++++++++-------- .../PGOForceFunctionAttrs}/basic.ll | 10 +++++----- llvm/tools/opt/NewPMDriver.cpp | 13 +++++++------ .../llvm/lib/Transforms/Instrumentation/BUILD.gn | 2 +- 13 files changed, 56 insertions(+), 54 deletions(-) rename llvm/include/llvm/Transforms/Instrumentation/{MarkColdFunctions.h => PGOForceFunctionAttrs.h} (54%) rename llvm/lib/Transforms/Instrumentation/{MarkColdFunctions.cpp => PGOForceFunctionAttrs.cpp} (82%) rename llvm/test/{Transforms/MarkColdFunctions => Instrumentation/PGOForceFunctionAttrs}/basic.ll (76%) diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 7e9d3b8ea55a18..4e8e401e749755 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -746,7 +746,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline( CodeGenOpts.InstrProfileOutput.empty() ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", "", CodeGenOpts.MemoryProfileUsePath, nullptr, PGOOptions::IRInstr, - PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, + PGOOptions::NoCSAction, PGOOptions::ColdFuncOpt::Default, CodeGenOpts.DebugInfoForProfiling, /*PseudoProbeForProfiling=*/false, CodeGenOpts.AtomicProfileUpdate); else if (CodeGenOpts.hasProfileIRUse()) { @@ -756,32 +756,32 @@ void EmitAssemblyHelper::RunOptimizationPipeline( PGOOpt = PGOOptions( CodeGenOpts.ProfileInstrumentUsePath, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, - PGOOptions::IRUse, CSAction, PGOOptions::ColdFuncAttr::None, + PGOOptions::IRUse, CSAction, PGOOptions::ColdFuncOpt::Default, CodeGenOpts.DebugInfoForProfiling); } else if (!CodeGenOpts.SampleProfileFile.empty()) // -fprofile-sample-use PGOOpt = PGOOptions( CodeGenOpts.SampleProfileFile, "", CodeGenOpts.ProfileRemappingFile, CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::SampleUse, - PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, + PGOOptions::NoCSAction, PGOOptions::ColdFuncOpt::Default, CodeGenOpts.DebugInfoForProfiling, CodeGenOpts.PseudoProbeForProfiling); else if (!CodeGenOpts.MemoryProfileUsePath.empty()) // -fmemory-profile-use (without any of the above options) PGOOpt = PGOOptions("", "", "", CodeGenOpts.MemoryProfileUsePath, VFS, PGOOptions::NoAction, PGOOptions::NoCSAction, - PGOOptions::ColdFuncAttr::None, + PGOOptions::ColdFuncOpt::Default, CodeGenOpts.DebugInfoForProfiling); else if (CodeGenOpts.PseudoProbeForProfiling) // -fpseudo-probe-for-profiling PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction, PGOOptions::NoCSAction, - PGOOptions::ColdFuncAttr::None, + PGOOptions::ColdFuncOpt::Default, CodeGenOpts.DebugInfoForProfiling, true); else if (CodeGenOpts.DebugInfoForProfiling) // -fdebug-info-for-profiling PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction, PGOOptions::NoCSAction, - PGOOptions::ColdFuncAttr::None, true); + PGOOptions::ColdFuncOpt::Default, true); // Check to see if we want to generate a CS profile. if (CodeGenOpts.hasProfileCSIRInstr()) { @@ -804,7 +804,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline( ? getDefaultProfileGenName() : CodeGenOpts.InstrProfileOutput, "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction, - PGOOptions::CSIRInstr, PGOOptions::ColdFuncAttr::None, + PGOOptions::CSIRInstr, PGOOptions::ColdFuncOpt::Default, CodeGenOpts.DebugInfoForProfiling); } if (TM) diff --git a/llvm/include/llvm/Support/PGOOptions.h b/llvm/include/llvm/Support/PGOOptions.h index fe21169a137016..de981abf187058 100644 --- a/llvm/include/llvm/Support/PGOOptions.h +++ b/llvm/include/llvm/Support/PGOOptions.h @@ -27,12 +27,12 @@ class FileSystem; struct PGOOptions { enum PGOAction { NoAction, IRInstr, IRUse, SampleUse }; enum CSPGOAction { NoCSAction, CSIRInstr, CSIRUse }; - enum class ColdFuncAttr { None, OptSize, MinSize, OptNone }; + enum class ColdFuncOpt { Default, OptSize, MinSize, OptNone }; PGOOptions(std::string ProfileFile, std::string CSProfileGenFile, std::string ProfileRemappingFile, std::string MemoryProfile, IntrusiveRefCntPtr FS, PGOAction Action = NoAction, CSPGOAction CSAction = NoCSAction, - ColdFuncAttr ColdType = ColdFuncAttr::None, + ColdFuncOpt ColdType = ColdFuncOpt::Default, bool DebugInfoForProfiling = false, bool PseudoProbeForProfiling = false, bool AtomicCounterUpdate = false); @@ -46,7 +46,7 @@ struct PGOOptions { std::string MemoryProfile; PGOAction Action; CSPGOAction CSAction; - ColdFuncAttr ColdType; + ColdFuncOpt ColdOptType; bool DebugInfoForProfiling; bool PseudoProbeForProfiling; bool AtomicCounterUpdate; diff --git a/llvm/include/llvm/Transforms/Instrumentation/MarkColdFunctions.h b/llvm/include/llvm/Transforms/Instrumentation/PGOForceFunctionAttrs.h similarity index 54% rename from llvm/include/llvm/Transforms/Instrumentation/MarkColdFunctions.h rename to llvm/include/llvm/Transforms/Instrumentation/PGOForceFunctionAttrs.h index 06aecc911d8827..785448e0dec4d9 100644 --- a/llvm/include/llvm/Transforms/Instrumentation/MarkColdFunctions.h +++ b/llvm/include/llvm/Transforms/Instrumentation/PGOForceFunctionAttrs.h @@ -1,4 +1,4 @@ -//===- MarkColdFunctions.h - ------------------------------------*- C++ -*-===// +//===- PGOForceFunctionAttrs.h - --------------------------------*- C++ -*-===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -6,23 +6,24 @@ // //===----------------------------------------------------------------------===// -#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_MARKCOLDFUNCTIONS_H -#define LLVM_TRANSFORMS_INSTRUMENTATION_MARKCOLDFUNCTIONS_H +#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_PGOFORCEFUNCTIONATTRS_H +#define LLVM_TRANSFORMS_INSTRUMENTATION_PGOFORCEFUNCTIONATTRS_H #include "llvm/IR/PassManager.h" #include "llvm/Support/PGOOptions.h" namespace llvm { -struct MarkColdFunctionsPass : public PassInfoMixin { - MarkColdFunctionsPass(PGOOptions::ColdFuncAttr ColdType) +struct PGOForceFunctionAttrsPass + : public PassInfoMixin { + PGOForceFunctionAttrsPass(PGOOptions::ColdFuncOpt ColdType) : ColdType(ColdType) {} PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM); private: - PGOOptions::ColdFuncAttr ColdType; + PGOOptions::ColdFuncOpt ColdType; }; } // namespace llvm -#endif // LLVM_TRANSFORMS_INSTRUMENTATION_MARKCOLDFUNCTIONS_H +#endif // LLVM_TRANSFORMS_INSTRUMENTATION_PGOFORCEFUNCTIONATTRS_H diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp index 8126b3563e077d..7b3a7590dfa743 100644 --- a/llvm/lib/LTO/LTOBackend.cpp +++ b/llvm/lib/LTO/LTOBackend.cpp @@ -243,23 +243,23 @@ static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM, if (!Conf.SampleProfile.empty()) PGOOpt = PGOOptions(Conf.SampleProfile, "", Conf.ProfileRemapping, /*MemoryProfile=*/"", FS, PGOOptions::SampleUse, - PGOOptions::NoCSAction, PGOOptions::ColdFuncAttr::None, - true); + PGOOptions::NoCSAction, + PGOOptions::ColdFuncOpt::Default, true); else if (Conf.RunCSIRInstr) { PGOOpt = PGOOptions("", Conf.CSIRProfile, Conf.ProfileRemapping, /*MemoryProfile=*/"", FS, PGOOptions::IRUse, - PGOOptions::CSIRInstr, PGOOptions::ColdFuncAttr::None, + PGOOptions::CSIRInstr, PGOOptions::ColdFuncOpt::Default, Conf.AddFSDiscriminator); } else if (!Conf.CSIRProfile.empty()) { PGOOpt = PGOOptions(Conf.CSIRProfile, "", Conf.ProfileRemapping, /*MemoryProfile=*/"", FS, PGOOptions::IRUse, - PGOOptions::CSIRUse, PGOOptions::ColdFuncAttr::None, + PGOOptions::CSIRUse, PGOOptions::ColdFuncOpt::Default, Conf.AddFSDiscriminator); NoPGOWarnMismatch = !Conf.PGOWarnMismatch; } else if (Conf.AddFSDiscriminator) { PGOOpt = PGOOptions("", "", "", /*MemoryProfile=*/"", nullptr, PGOOptions::NoAction, PGOOptions::NoCSAction, - PGOOptions::ColdFuncAttr::None, true); + PGOOptions::ColdFuncOpt::Default, true); } TM->setPGOOption(PGOOpt); diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp index a4ea91794b9c97..0fc5ef046524b4 100644 --- a/llvm/lib/Passes/PassBuilder.cpp +++ b/llvm/lib/Passes/PassBuilder.cpp @@ -167,9 +167,9 @@ #include "llvm/Transforms/Instrumentation/InstrOrderFile.h" #include "llvm/Transforms/Instrumentation/InstrProfiling.h" #include "llvm/Transforms/Instrumentation/KCFI.h" -#include "llvm/Transforms/Instrumentation/MarkColdFunctions.h" #include "llvm/Transforms/Instrumentation/MemProfiler.h" #include "llvm/Transforms/Instrumentation/MemorySanitizer.h" +#include "llvm/Transforms/Instrumentation/PGOForceFunctionAttrs.h" #include "llvm/Transforms/Instrumentation/PGOInstrumentation.h" #include "llvm/Transforms/Instrumentation/PoisonChecking.h" #include "llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h" diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index 6ddeb8d675b94d..a6a90c95847a83 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -73,8 +73,8 @@ #include "llvm/Transforms/Instrumentation/ControlHeightReduction.h" #include "llvm/Transforms/Instrumentation/InstrOrderFile.h" #include "llvm/Transforms/Instrumentation/InstrProfiling.h" -#include "llvm/Transforms/Instrumentation/MarkColdFunctions.h" #include "llvm/Transforms/Instrumentation/MemProfiler.h" +#include "llvm/Transforms/Instrumentation/PGOForceFunctionAttrs.h" #include "llvm/Transforms/Instrumentation/PGOInstrumentation.h" #include "llvm/Transforms/Scalar/ADCE.h" #include "llvm/Transforms/Scalar/AlignmentFromAssumptions.h" @@ -214,10 +214,10 @@ static cl::opt cl::init(false), cl::Hidden); // TODO: turn on and remove flag -static cl::opt - EnableMarkColdFunctions("enable-mark-cold-functions", - cl::desc("Enable pass to mark cold functions"), - cl::init(false)); +static cl::opt EnablePGOForceFunctionAttrs( + "enable-pgo-force-function-attrs", + cl::desc("Enable pass to set function attributes based on PGO profiles"), + cl::init(false)); static cl::opt EnableHotColdSplit("hot-cold-split", @@ -1144,10 +1144,10 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, if (EnableSyntheticCounts && !PGOOpt) MPM.addPass(SyntheticCountsPropagation()); - if (EnableMarkColdFunctions && PGOOpt && + if (EnablePGOForceFunctionAttrs && PGOOpt && (PGOOpt->Action == PGOOptions::SampleUse || PGOOpt->Action == PGOOptions::IRUse)) - MPM.addPass(MarkColdFunctionsPass(PGOOpt->ColdType)); + MPM.addPass(PGOForceFunctionAttrsPass(PGOOpt->ColdOptType)); MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/true)); diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def index 2e3c56b3ccbc77..12c72837caba10 100644 --- a/llvm/lib/Passes/PassRegistry.def +++ b/llvm/lib/Passes/PassRegistry.def @@ -86,7 +86,7 @@ MODULE_PASS("lower-emutls", LowerEmuTLSPass()) MODULE_PASS("lower-global-dtors", LowerGlobalDtorsPass()) MODULE_PASS("lower-ifunc", LowerIFuncPass()) MODULE_PASS("lowertypetests", LowerTypeTestsPass()) -MODULE_PASS("mark-cold-functions", MarkColdFunctionsPass(PGOOpt ? PGOOpt->ColdType : PGOOptions::ColdFuncAttr::None)) +MODULE_PASS("pgo-force-function-attrs", PGOForceFunctionAttrsPass(PGOOpt ? PGOOpt->ColdOptType : PGOOptions::ColdFuncOpt::Default)) MODULE_PASS("memprof-context-disambiguation", MemProfContextDisambiguation()) MODULE_PASS("memprof-module", ModuleMemProfilerPass()) MODULE_PASS("mergefunc", MergeFunctionsPass()) diff --git a/llvm/lib/Support/PGOOptions.cpp b/llvm/lib/Support/PGOOptions.cpp index 5dfeafa188980e..5981dff9e09468 100644 --- a/llvm/lib/Support/PGOOptions.cpp +++ b/llvm/lib/Support/PGOOptions.cpp @@ -15,12 +15,12 @@ PGOOptions::PGOOptions(std::string ProfileFile, std::string CSProfileGenFile, std::string ProfileRemappingFile, std::string MemoryProfile, IntrusiveRefCntPtr FS, PGOAction Action, - CSPGOAction CSAction, ColdFuncAttr ColdType, + CSPGOAction CSAction, ColdFuncOpt ColdType, bool DebugInfoForProfiling, bool PseudoProbeForProfiling, bool AtomicCounterUpdate) : ProfileFile(ProfileFile), CSProfileGenFile(CSProfileGenFile), ProfileRemappingFile(ProfileRemappingFile), MemoryProfile(MemoryProfile), - Action(Action), CSAction(CSAction), ColdType(ColdType), + Action(Action), CSAction(CSAction), ColdOptType(ColdType), DebugInfoForProfiling(DebugInfoForProfiling || (Action == SampleUse && !PseudoProbeForProfiling)), PseudoProbeForProfiling(PseudoProbeForProfiling), diff --git a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt index b704726dd78037..378eaaa4a8ab13 100644 --- a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt +++ b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt @@ -6,7 +6,7 @@ add_llvm_component_library(LLVMInstrumentation DataFlowSanitizer.cpp GCOVProfiling.cpp BlockCoverageInference.cpp - MarkColdFunctions.cpp + PGOForceFunctionAttrs.cpp MemProfiler.cpp MemorySanitizer.cpp IndirectCallPromotion.cpp diff --git a/llvm/lib/Transforms/Instrumentation/MarkColdFunctions.cpp b/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp similarity index 82% rename from llvm/lib/Transforms/Instrumentation/MarkColdFunctions.cpp rename to llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp index 9037607132ef41..25978ce4fd668d 100644 --- a/llvm/lib/Transforms/Instrumentation/MarkColdFunctions.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp @@ -6,16 +6,16 @@ // //===----------------------------------------------------------------------===// -#include "llvm/Transforms/Instrumentation/MarkColdFunctions.h" +#include "llvm/Transforms/Instrumentation/PGOForceFunctionAttrs.h" #include "llvm/Analysis/BlockFrequencyInfo.h" #include "llvm/Analysis/ProfileSummaryInfo.h" #include "llvm/IR/PassManager.h" using namespace llvm; -PreservedAnalyses MarkColdFunctionsPass::run(Module &M, - ModuleAnalysisManager &AM) { - if (ColdType == PGOOptions::ColdFuncAttr::None) +PreservedAnalyses PGOForceFunctionAttrsPass::run(Module &M, + ModuleAnalysisManager &AM) { + if (ColdType == PGOOptions::ColdFuncOpt::Default) return PreservedAnalyses::all(); ProfileSummaryInfo &PSI = AM.getResult(M); if (!PSI.hasProfileSummary()) @@ -31,10 +31,10 @@ PreservedAnalyses MarkColdFunctionsPass::run(Module &M, continue; // Add optsize/minsize/optnone if requested. switch (ColdType) { - case PGOOptions::ColdFuncAttr::None: + case PGOOptions::ColdFuncOpt::Default: assert(false); break; - case PGOOptions::ColdFuncAttr::OptSize: + case PGOOptions::ColdFuncOpt::OptSize: if (!F.hasFnAttribute(Attribute::OptimizeNone) && !F.hasFnAttribute(Attribute::OptimizeForSize) && !F.hasFnAttribute(Attribute::MinSize)) { @@ -42,7 +42,7 @@ PreservedAnalyses MarkColdFunctionsPass::run(Module &M, MadeChange = true; } break; - case PGOOptions::ColdFuncAttr::MinSize: + case PGOOptions::ColdFuncOpt::MinSize: // Change optsize to minsize. if (!F.hasFnAttribute(Attribute::OptimizeNone) && !F.hasFnAttribute(Attribute::MinSize)) { @@ -51,7 +51,7 @@ PreservedAnalyses MarkColdFunctionsPass::run(Module &M, MadeChange = true; } break; - case PGOOptions::ColdFuncAttr::OptNone: + case PGOOptions::ColdFuncOpt::OptNone: // Strip optsize/minsize. F.removeFnAttr(Attribute::OptimizeForSize); F.removeFnAttr(Attribute::MinSize); diff --git a/llvm/test/Transforms/MarkColdFunctions/basic.ll b/llvm/test/Instrumentation/PGOForceFunctionAttrs/basic.ll similarity index 76% rename from llvm/test/Transforms/MarkColdFunctions/basic.ll rename to llvm/test/Instrumentation/PGOForceFunctionAttrs/basic.ll index f79e3a8f1dda7d..ccb2e89b88282c 100644 --- a/llvm/test/Transforms/MarkColdFunctions/basic.ll +++ b/llvm/test/Instrumentation/PGOForceFunctionAttrs/basic.ll @@ -1,10 +1,10 @@ -; RUN: opt < %s -passes=mark-cold-functions -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-attr=none | FileCheck %s --check-prefixes=NONE,CHECK -; RUN: opt < %s -passes=mark-cold-functions -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-attr=optsize | FileCheck %s --check-prefixes=OPTSIZE,CHECK -; RUN: opt < %s -passes=mark-cold-functions -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-attr=minsize | FileCheck %s --check-prefixes=MINSIZE,CHECK -; RUN: opt < %s -passes=mark-cold-functions -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-attr=optnone | FileCheck %s --check-prefixes=OPTNONE,CHECK +; RUN: opt < %s -passes=pgo-force-function-attrs -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-opt=default | FileCheck %s --check-prefixes=NONE,CHECK +; RUN: opt < %s -passes=pgo-force-function-attrs -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-opt=optsize | FileCheck %s --check-prefixes=OPTSIZE,CHECK +; RUN: opt < %s -passes=pgo-force-function-attrs -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-opt=minsize | FileCheck %s --check-prefixes=MINSIZE,CHECK +; RUN: opt < %s -passes=pgo-force-function-attrs -pgo-kind=pgo-instr-use-pipeline -S -pgo-cold-func-opt=optnone | FileCheck %s --check-prefixes=OPTNONE,CHECK ; Should be no changes without profile data -; RUN: opt < %s -passes=mark-cold-functions -S -pgo-cold-func-attr=minsize | FileCheck %s --check-prefixes=NONE,CHECK +; RUN: opt < %s -passes=pgo-force-function-attrs -S -pgo-cold-func-opt=minsize | FileCheck %s --check-prefixes=NONE,CHECK ; NONE-NOT: Function Attrs: ; OPTSIZE: Function Attrs: optsize{{$}} diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp index 228eda8c3e0665..5ef7b99e473cc5 100644 --- a/llvm/tools/opt/NewPMDriver.cpp +++ b/llvm/tools/opt/NewPMDriver.cpp @@ -202,16 +202,17 @@ static cl::opt cl::desc("Path to the profile remapping file."), cl::Hidden); -static cl::opt PGOColdFuncAttr( - "pgo-cold-func-attr", cl::init(PGOOptions::ColdFuncAttr::None), cl::Hidden, +static cl::opt PGOColdFuncAttr( + "pgo-cold-func-opt", cl::init(PGOOptions::ColdFuncOpt::Default), cl::Hidden, cl::desc( "Function attribute to apply to cold functions as determined by PGO"), - cl::values(clEnumValN(PGOOptions::ColdFuncAttr::None, "none", "None"), - clEnumValN(PGOOptions::ColdFuncAttr::OptSize, "optsize", + cl::values(clEnumValN(PGOOptions::ColdFuncOpt::Default, "default", + "Default (no attribute)"), + clEnumValN(PGOOptions::ColdFuncOpt::OptSize, "optsize", "Mark cold functions with optsize."), - clEnumValN(PGOOptions::ColdFuncAttr::MinSize, "minsize", + clEnumValN(PGOOptions::ColdFuncOpt::MinSize, "minsize", "Mark cold functions with minsize."), - clEnumValN(PGOOptions::ColdFuncAttr::OptNone, "optnone", + clEnumValN(PGOOptions::ColdFuncOpt::OptNone, "optnone", "Mark cold functions with optnone."))); static cl::opt DebugInfoForProfiling( diff --git a/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn b/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn index 42112878c93a61..00e1888da64d26 100644 --- a/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn +++ b/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn @@ -23,7 +23,7 @@ static_library("Instrumentation") { "InstrProfiling.cpp", "Instrumentation.cpp", "KCFI.cpp", - "MarkColdFunctions.cpp", + "PGOForceFunctionAttrs.cpp", "MemProfiler.cpp", "MemorySanitizer.cpp", "PGOInstrumentation.cpp", From 7b885fd87c05caa658a9282550c086464505dcdb Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Tue, 23 Jan 2024 21:42:50 +0000 Subject: [PATCH 3/6] run pass in CSFDO use pipeline --- llvm/lib/Passes/PassBuilderPipelines.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index a6a90c95847a83..033288131f2e1a 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -1146,7 +1146,8 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, if (EnablePGOForceFunctionAttrs && PGOOpt && (PGOOpt->Action == PGOOptions::SampleUse || - PGOOpt->Action == PGOOptions::IRUse)) + PGOOpt->Action == PGOOptions::IRUse || + PGOOpt->CSAction == PGOOptions::CSIRUse)) MPM.addPass(PGOForceFunctionAttrsPass(PGOOpt->ColdOptType)); MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/true)); From 02e6603f5f89d972f78e1f97dfacd8be5288aa23 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Tue, 23 Jan 2024 21:46:19 +0000 Subject: [PATCH 4/6] small fixes --- llvm/lib/Transforms/Instrumentation/CMakeLists.txt | 2 +- llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt index 378eaaa4a8ab13..ee9aa73ff03403 100644 --- a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt +++ b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt @@ -6,7 +6,6 @@ add_llvm_component_library(LLVMInstrumentation DataFlowSanitizer.cpp GCOVProfiling.cpp BlockCoverageInference.cpp - PGOForceFunctionAttrs.cpp MemProfiler.cpp MemorySanitizer.cpp IndirectCallPromotion.cpp @@ -14,6 +13,7 @@ add_llvm_component_library(LLVMInstrumentation InstrOrderFile.cpp InstrProfiling.cpp KCFI.cpp + PGOForceFunctionAttrs.cpp PGOInstrumentation.cpp PGOMemOPSizeOpt.cpp PoisonChecking.cpp diff --git a/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp b/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp index 25978ce4fd668d..13d5caf6c6a858 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp @@ -10,6 +10,7 @@ #include "llvm/Analysis/BlockFrequencyInfo.h" #include "llvm/Analysis/ProfileSummaryInfo.h" #include "llvm/IR/PassManager.h" +#include "llvm/Support/ErrorHandling.h" using namespace llvm; @@ -32,7 +33,7 @@ PreservedAnalyses PGOForceFunctionAttrsPass::run(Module &M, // Add optsize/minsize/optnone if requested. switch (ColdType) { case PGOOptions::ColdFuncOpt::Default: - assert(false); + llvm_unreachable("bailed out for default above"); break; case PGOOptions::ColdFuncOpt::OptSize: if (!F.hasFnAttribute(Attribute::OptimizeNone) && From 33022e82df0dccc52a54dee3a70ee807097f544a Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Tue, 23 Jan 2024 22:03:46 +0000 Subject: [PATCH 5/6] always run pass, also apply attrs to cold functions --- llvm/lib/Passes/PassBuilderPipelines.cpp | 5 +---- .../Instrumentation/PGOForceFunctionAttrs.cpp | 15 +++++++++++---- .../PGOForceFunctionAttrs/basic.ll | 11 +++++++++++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp index 033288131f2e1a..e5db47dee4fe8d 100644 --- a/llvm/lib/Passes/PassBuilderPipelines.cpp +++ b/llvm/lib/Passes/PassBuilderPipelines.cpp @@ -1144,10 +1144,7 @@ PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level, if (EnableSyntheticCounts && !PGOOpt) MPM.addPass(SyntheticCountsPropagation()); - if (EnablePGOForceFunctionAttrs && PGOOpt && - (PGOOpt->Action == PGOOptions::SampleUse || - PGOOpt->Action == PGOOptions::IRUse || - PGOOpt->CSAction == PGOOptions::CSIRUse)) + if (EnablePGOForceFunctionAttrs) MPM.addPass(PGOForceFunctionAttrsPass(PGOOpt->ColdOptType)); MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/true)); diff --git a/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp b/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp index 13d5caf6c6a858..13b7be4eae22d5 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp @@ -14,21 +14,28 @@ using namespace llvm; +static bool shouldRunOnFunction(Function &F, ProfileSummaryInfo &PSI, + FunctionAnalysisManager &FAM) { + if (F.hasFnAttribute(Attribute::Cold)) + return true; + if (!PSI.hasProfileSummary()) + return false; + BlockFrequencyInfo &BFI = FAM.getResult(F); + return PSI.isFunctionColdInCallGraph(&F, BFI); +} + PreservedAnalyses PGOForceFunctionAttrsPass::run(Module &M, ModuleAnalysisManager &AM) { if (ColdType == PGOOptions::ColdFuncOpt::Default) return PreservedAnalyses::all(); ProfileSummaryInfo &PSI = AM.getResult(M); - if (!PSI.hasProfileSummary()) - return PreservedAnalyses::all(); FunctionAnalysisManager &FAM = AM.getResult(M).getManager(); bool MadeChange = false; for (Function &F : M) { if (F.isDeclaration()) continue; - BlockFrequencyInfo &BFI = FAM.getResult(F); - if (!PSI.isFunctionColdInCallGraph(&F, BFI)) + if (!shouldRunOnFunction(F, PSI, FAM)) continue; // Add optsize/minsize/optnone if requested. switch (ColdType) { diff --git a/llvm/test/Instrumentation/PGOForceFunctionAttrs/basic.ll b/llvm/test/Instrumentation/PGOForceFunctionAttrs/basic.ll index ccb2e89b88282c..bbf57fa98da79e 100644 --- a/llvm/test/Instrumentation/PGOForceFunctionAttrs/basic.ll +++ b/llvm/test/Instrumentation/PGOForceFunctionAttrs/basic.ll @@ -27,6 +27,12 @@ ; CHECK: Function Attrs: noinline optnone{{$}} ; CHECK-NEXT: define void @cold3() +; NONE: Function Attrs: cold{{$}} +; OPTSIZE: Function Attrs: cold optsize{{$}} +; MINSIZE: Function Attrs: cold minsize{{$}} +; OPTNONE: Function Attrs: cold noinline optnone{{$}} +; CHECK-NEXT: define void @cold4() + ; CHECK-NOT: Function Attrs: {{.*}}optsize ; CHECK-NOT: Function Attrs: {{.*}}minsize ; CHECK-NOT: Function Attrs: {{.*}}optnone @@ -53,6 +59,11 @@ define void @cold3() noinline optnone !prof !27 { ret void } +define void @cold4() cold { + store i32 1, ptr @s, align 4 + ret void +} + define void @hot() !prof !28 { %l = load i32, ptr @s, align 4 %add = add nsw i32 %l, 4 From 38b397de749410457552261bdcb63a9974fe3562 Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Mon, 5 Feb 2024 20:56:13 +0000 Subject: [PATCH 6/6] respect existing attribute --- .../Instrumentation/PGOForceFunctionAttrs.cpp | 28 ++++++------------- .../PGOForceFunctionAttrs/basic.ll | 26 +++++++---------- 2 files changed, 18 insertions(+), 36 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp b/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp index 13b7be4eae22d5..c0ebdd7ed88635 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOForceFunctionAttrs.cpp @@ -16,6 +16,11 @@ using namespace llvm; static bool shouldRunOnFunction(Function &F, ProfileSummaryInfo &PSI, FunctionAnalysisManager &FAM) { + if (F.isDeclaration()) + return false; + // Respect existing attributes. + if (F.hasOptNone() || F.hasOptSize() || F.hasMinSize()) + return false; if (F.hasFnAttribute(Attribute::Cold)) return true; if (!PSI.hasProfileSummary()) @@ -33,39 +38,22 @@ PreservedAnalyses PGOForceFunctionAttrsPass::run(Module &M, AM.getResult(M).getManager(); bool MadeChange = false; for (Function &F : M) { - if (F.isDeclaration()) - continue; if (!shouldRunOnFunction(F, PSI, FAM)) continue; - // Add optsize/minsize/optnone if requested. + MadeChange = true; switch (ColdType) { case PGOOptions::ColdFuncOpt::Default: llvm_unreachable("bailed out for default above"); break; case PGOOptions::ColdFuncOpt::OptSize: - if (!F.hasFnAttribute(Attribute::OptimizeNone) && - !F.hasFnAttribute(Attribute::OptimizeForSize) && - !F.hasFnAttribute(Attribute::MinSize)) { - F.addFnAttr(Attribute::OptimizeForSize); - MadeChange = true; - } + F.addFnAttr(Attribute::OptimizeForSize); break; case PGOOptions::ColdFuncOpt::MinSize: - // Change optsize to minsize. - if (!F.hasFnAttribute(Attribute::OptimizeNone) && - !F.hasFnAttribute(Attribute::MinSize)) { - F.removeFnAttr(Attribute::OptimizeForSize); - F.addFnAttr(Attribute::MinSize); - MadeChange = true; - } + F.addFnAttr(Attribute::MinSize); break; case PGOOptions::ColdFuncOpt::OptNone: - // Strip optsize/minsize. - F.removeFnAttr(Attribute::OptimizeForSize); - F.removeFnAttr(Attribute::MinSize); F.addFnAttr(Attribute::OptimizeNone); F.addFnAttr(Attribute::NoInline); - MadeChange = true; break; } } diff --git a/llvm/test/Instrumentation/PGOForceFunctionAttrs/basic.ll b/llvm/test/Instrumentation/PGOForceFunctionAttrs/basic.ll index bbf57fa98da79e..29ebc0366040e1 100644 --- a/llvm/test/Instrumentation/PGOForceFunctionAttrs/basic.ll +++ b/llvm/test/Instrumentation/PGOForceFunctionAttrs/basic.ll @@ -12,26 +12,20 @@ ; OPTNONE: Function Attrs: noinline optnone{{$}} ; CHECK: define void @cold() -; NONE: Function Attrs: optsize{{$}} -; OPTSIZE: Function Attrs: optsize{{$}} -; MINSIZE: Function Attrs: minsize{{$}} -; OPTNONE: Function Attrs: noinline optnone{{$}} -; CHECK-NEXT: define void @cold1() +; CHECK: Function Attrs: optsize{{$}} +; CHECK-NEXT: define void @cold_optsize() -; NONE: Function Attrs: minsize{{$}} -; OPTSIZE: Function Attrs: minsize{{$}} -; MINSIZE: Function Attrs: minsize{{$}} -; OPTNONE: Function Attrs: noinline optnone{{$}} -; CHECK-NEXT: define void @cold2() +; CHECK: Function Attrs: minsize{{$}} +; CHECK-NEXT: define void @cold_minsize() ; CHECK: Function Attrs: noinline optnone{{$}} -; CHECK-NEXT: define void @cold3() +; CHECK-NEXT: define void @cold_optnone() ; NONE: Function Attrs: cold{{$}} ; OPTSIZE: Function Attrs: cold optsize{{$}} ; MINSIZE: Function Attrs: cold minsize{{$}} ; OPTNONE: Function Attrs: cold noinline optnone{{$}} -; CHECK-NEXT: define void @cold4() +; CHECK-NEXT: define void @cold_attr() ; CHECK-NOT: Function Attrs: {{.*}}optsize ; CHECK-NOT: Function Attrs: {{.*}}minsize @@ -44,22 +38,22 @@ define void @cold() !prof !27 { ret void } -define void @cold1() optsize !prof !27 { +define void @cold_optsize() optsize !prof !27 { store i32 1, ptr @s, align 4 ret void } -define void @cold2() minsize !prof !27 { +define void @cold_minsize() minsize !prof !27 { store i32 1, ptr @s, align 4 ret void } -define void @cold3() noinline optnone !prof !27 { +define void @cold_optnone() noinline optnone !prof !27 { store i32 1, ptr @s, align 4 ret void } -define void @cold4() cold { +define void @cold_attr() cold { store i32 1, ptr @s, align 4 ret void }