From ccb5b9bbfb5cef1aa2982481894f30c8f81d5253 Mon Sep 17 00:00:00 2001 From: Hongtao Yu Date: Tue, 10 Aug 2021 18:19:21 -0700 Subject: [PATCH] [CSSPGO] Allow the use of debug-info-for-profiling and pseudo-probe-for-profiling together Previoulsy debug-info-for-profiling and pseudo-probe-for-profiling are mutual exclusive because they compete the dwarf discrimnator for callsites on the IR. This changes allows to use the two switches together. The side effect is that callsite discriminators will be taken by pseudo probe, while discriminators for other instructions are still available for AutoFDO use. This is less than ideal, however, it still allows us a chance to smoothly transition from AutoFDO to CSSPGO, by collecting both profiles from a CSSPGO binary. Reviewed By: wenlei, wmi Differential Revision: https://reviews.llvm.org/D107876 --- clang/lib/Driver/ToolChains/Clang.cpp | 6 -- .../test/CodeGenCXX/fdebug-info-for-profiling.cpp | 3 + clang/test/Driver/pseudo-probe.c | 4 +- llvm/include/llvm/Passes/PassBuilder.h | 8 --- .../SampleProfile/pseudo-probe-discriminator.ll | 71 ++++++++++++++++++++++ llvm/tools/opt/NewPMDriver.cpp | 7 +-- 6 files changed, 78 insertions(+), 21 deletions(-) create mode 100644 llvm/test/Transforms/SampleProfile/pseudo-probe-discriminator.ll diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index ceeae94e..e19e122 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -3892,12 +3892,6 @@ static void renderDebugOptions(const ToolChain &TC, const Driver &D, ArgStringList &CmdArgs, codegenoptions::DebugInfoKind &DebugInfoKind, DwarfFissionKind &DwarfFission) { - // These two forms of profiling info can't be used together. - if (const Arg *A1 = Args.getLastArg(options::OPT_fpseudo_probe_for_profiling)) - if (const Arg *A2 = Args.getLastArg(options::OPT_fdebug_info_for_profiling)) - D.Diag(diag::err_drv_argument_not_allowed_with) - << A1->getAsString(Args) << A2->getAsString(Args); - if (Args.hasFlag(options::OPT_fdebug_info_for_profiling, options::OPT_fno_debug_info_for_profiling, false) && checkDebugInfoOption( diff --git a/clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp b/clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp index 0a66818..421195d 100644 --- a/clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp +++ b/clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp @@ -14,8 +14,11 @@ // RUN: echo > %t.proftext // RUN: llvm-profdata merge %t.proftext -o %t.profdata // RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager -O1 -fprofile-instrument-use-path=%t.profdata -fdebug-info-for-profiling %s -o - 2>&1 | FileCheck %s --check-prefix=DISCR +// RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager -O1 -fdebug-info-for-profiling -fpseudo-probe-for-profiling %s -o - 2>&1 | FileCheck %s --check-prefix=PROBE // NODISCR-NOT: Running pass: AddDiscriminatorsPass // DISCR: Running pass: AddDiscriminatorsPass on {{.*}} +// PROBE: Running pass: AddDiscriminatorsPass on {{.*}} +// PROBE: Running pass: SampleProfileProbePass on {{.*}} void foo() {} diff --git a/clang/test/Driver/pseudo-probe.c b/clang/test/Driver/pseudo-probe.c index 79b23df..76c4364 100644 --- a/clang/test/Driver/pseudo-probe.c +++ b/clang/test/Driver/pseudo-probe.c @@ -1,13 +1,13 @@ // RUN: %clang -### -fpseudo-probe-for-profiling %s 2>&1 | FileCheck %s --check-prefix=YESPROBE // RUN: %clang -### -fno-pseudo-probe-for-profiling %s 2>&1 | FileCheck %s --check-prefix=NOPROBE -// RUN: %clang -### -fpseudo-probe-for-profiling -fdebug-info-for-profiling %s 2>&1 | FileCheck %s --check-prefix=CONFLICT +// RUN: %clang -### -fpseudo-probe-for-profiling -fdebug-info-for-profiling %s 2>&1 | FileCheck %s --check-prefix=YESPROBE --check-prefix=YESDEBUG // RUN: %clang -### -fpseudo-probe-for-profiling -funique-internal-linkage-names %s 2>&1 | FileCheck %s --check-prefix=YESPROBE // RUN: %clang -### -fpseudo-probe-for-profiling -fno-unique-internal-linkage-names %s 2>&1 | FileCheck %s --check-prefix=NONAME +// YESDEBUG: -fdebug-info-for-profiling // YESPROBE: -fpseudo-probe-for-profiling // YESPROBE: -funique-internal-linkage-names // NOPROBE-NOT: -fpseudo-probe-for-profiling // NOPROBE-NOT: -funique-internal-linkage-names // NONAME: -fpseudo-probe-for-profiling // NONAME-NOT: -funique-internal-linkage-names -// CONFLICT: invalid argument diff --git a/llvm/include/llvm/Passes/PassBuilder.h b/llvm/include/llvm/Passes/PassBuilder.h index 35f791f..9ab7bd4 100644 --- a/llvm/include/llvm/Passes/PassBuilder.h +++ b/llvm/include/llvm/Passes/PassBuilder.h @@ -65,14 +65,6 @@ struct PGOOptions { // PseudoProbeForProfiling needs to be true. assert(this->Action != NoAction || this->CSAction != NoCSAction || this->DebugInfoForProfiling || this->PseudoProbeForProfiling); - - // Pseudo probe emission does not work with -fdebug-info-for-profiling since - // they both use the discriminator field of debug lines but for different - // purposes. - if (this->DebugInfoForProfiling && this->PseudoProbeForProfiling) { - report_fatal_error( - "Pseudo probes cannot be used with -debug-info-for-profiling", false); - } } std::string ProfileFile; std::string CSProfileGenFile; diff --git a/llvm/test/Transforms/SampleProfile/pseudo-probe-discriminator.ll b/llvm/test/Transforms/SampleProfile/pseudo-probe-discriminator.ll new file mode 100644 index 0000000..c88db45 --- /dev/null +++ b/llvm/test/Transforms/SampleProfile/pseudo-probe-discriminator.ll @@ -0,0 +1,71 @@ +; RUN: opt < %s -passes='default' -new-pm-debug-info-for-profiling -S | FileCheck %s --check-prefix=DEBUG +; RUN: opt < %s -passes='default' -new-pm-debug-info-for-profiling -new-pm-pseudo-probe-for-profiling -S | FileCheck %s --check-prefix=PROBE +; RUN: opt < %s -passes='thinlto-pre-link' -new-pm-debug-info-for-profiling -new-pm-pseudo-probe-for-profiling -S | FileCheck %s --check-prefix=PROBE + + +@a = dso_local global i32 0, align 4 + +; Function Attrs: uwtable +define void @_Z3foov(i32 %x) #0 !dbg !4 { +bb0: + %cmp = icmp eq i32 %x, 0, !dbg !10 + br i1 %cmp, label %bb1, label %bb2 + +bb1: +; DEBUG: call void @_Z3barv(), !dbg ![[CALL1:[0-9]+]] +; PROBE: call void @_Z3barv(), !dbg ![[CALL1:[0-9]+]] + call void @_Z3barv(), !dbg !10 +; DEBUG: call void @_Z3barv(), !dbg ![[CALL2:[0-9]+]] +; PROBE: call void @_Z3barv(), !dbg ![[CALL2:[0-9]+]] + call void @_Z3barv(), !dbg !11 + ret void, !dbg !13 + +bb2: +; DEBUG: store i32 8, i32* @a, align 4, !dbg ![[INST:[0-9]+]] +; PROBE: store i32 8, i32* @a, align 4, !dbg ![[INST:[0-9]+]] + store i32 8, i32* @a, align 4, !dbg !12 + br label %bb3 + +bb3: + ret void, !dbg !12 +} + +declare void @_Z3barv() #1 +declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture) nounwind argmemonly +declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture) nounwind argmemonly + +attributes #0 = { uwtable "disable-tail-calls"="false" "less-precise-fpmad"="false" "frame-pointer"="all" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2" "unsafe-fp-math"="false" "use-soft-float"="false" } +attributes #1 = { "disable-tail-calls"="false" "less-precise-fpmad"="false" "frame-pointer"="all" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2" "unsafe-fp-math"="false" "use-soft-float"="false" } + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!7, !8} +!llvm.ident = !{!9} + +!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 3.8.0 (trunk 250915) (llvm/trunk 251830)", isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug, enums: !2) +!1 = !DIFile(filename: "c.cc", directory: "/tmp") +!2 = !{} +!4 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !1, file: !1, line: 3, type: !5, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2) +!5 = !DISubroutineType(types: !6) +!6 = !{null} +!7 = !{i32 2, !"Dwarf Version", i32 4} +!8 = !{i32 2, !"Debug Info Version", i32 3} +!9 = !{!"clang version 3.8.0 (trunk 250915) (llvm/trunk 251830)"} +!10 = !DILocation(line: 4, column: 3, scope: !4) +!11 = !DILocation(line: 4, column: 9, scope: !4) +!12 = !DILocation(line: 4, column: 15, scope: !4) +!13 = !DILocation(line: 5, column: 1, scope: !4) + +; DEBUG: ![[CALL1]] = !DILocation(line: 4, column: 3, scope: ![[CALL1BLOCK:[0-9]+]]) +; DEBUG: ![[CALL1BLOCK]] = !DILexicalBlockFile({{.*}} discriminator: 2) +; DEBUG: ![[CALL2]] = !DILocation(line: 4, column: 9, scope: ![[CALL2BLOCK:[0-9]+]]) +; DEBUG: ![[CALL2BLOCK]] = !DILexicalBlockFile({{.*}} discriminator: 8) +; DEBUG: ![[INST]] = !DILocation(line: 4, column: 15, scope: ![[INSTBLOCK:[0-9]+]]) +; DEBUG: ![[INSTBLOCK]] = !DILexicalBlockFile({{.*}} discriminator: 4) + + +; PROBE: ![[CALL1]] = !DILocation(line: 4, column: 3, scope: ![[CALL1BLOCK:[0-9]+]]) +; PROBE: ![[CALL1BLOCK]] = !DILexicalBlockFile({{.*}} discriminator: 186646575) +; PROBE: ![[CALL2]] = !DILocation(line: 4, column: 9, scope: ![[CALL2BLOCK:[0-9]+]]) +; PROBE: ![[CALL2BLOCK]] = !DILexicalBlockFile({{.*}} discriminator: 186646583) +; PROBE: ![[INST]] = !DILocation(line: 4, column: 15, scope: ![[INSTBLOCK:[0-9]+]]) +; PROBE: ![[INSTBLOCK]] = !DILexicalBlockFile({{.*}} discriminator: 4) diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp index 4617df0..d88636a 100644 --- a/llvm/tools/opt/NewPMDriver.cpp +++ b/llvm/tools/opt/NewPMDriver.cpp @@ -259,12 +259,9 @@ bool llvm::runPassPipeline(StringRef Arg0, Module &M, TargetMachine *TM, PGOOptions::SampleUse); break; case NoPGO: - if (DebugInfoForProfiling) + if (DebugInfoForProfiling || PseudoProbeForProfiling) P = PGOOptions("", "", "", PGOOptions::NoAction, PGOOptions::NoCSAction, - true); - else if (PseudoProbeForProfiling) - P = PGOOptions("", "", "", PGOOptions::NoAction, PGOOptions::NoCSAction, - false, true); + DebugInfoForProfiling, PseudoProbeForProfiling); else P = None; } -- 2.7.4