[CSSPGO] Allow the use of debug-info-for-profiling and pseudo-probe-for-profiling together

[hoy]

Summary

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.

[wenlei]

clang/lib/Driver/ToolChains/Clang.cpp

3896

Would a warning be useful to catch accidental misuse?

[wmi]

Could you remind me the discriminator difference between debug info based AFDO and pseudo probe based AFDO? I forgot it. I am sure it is described in some patch but I havn't found it. Give me a pointer is good enough.

[hoy]

clang/lib/Driver/ToolChains/Clang.cpp

3896

I was thinking about that. It may be useful, but downstream may have to suppress that warning to be treated as error for a clean build. What do you think?

[hoy]

In D107876#2939624, @wmi wrote:

Could you remind me the discriminator difference between debug info based AFDO and pseudo probe based AFDO? I forgot it. I am sure it is described in some patch but I havn't found it. Give me a pointer is good enough.

In short, unlike block pseudo probe, a callsite probe relies on the calliste discriminator to be tracked. This has a couple usage: 1. to track direct call and indirect callsite 2. to model inline contexts with a list of callsite probes.

A probe discriminator is treated as empty or null for AutoFDO since it has lowest three bits as "111".

[wenlei]

lgtm, thanks.

clang/lib/Driver/ToolChains/Clang.cpp

3896

Fair point. We could add warning after transition.

[wmi]

In D107876#2939691, @hoy wrote:

In D107876#2939624, @wmi wrote:

Could you remind me the discriminator difference between debug info based AFDO and pseudo probe based AFDO? I forgot it. I am sure it is described in some patch but I havn't found it. Give me a pointer is good enough.

In short, unlike block pseudo probe, a callsite probe relies on the calliste discriminator to be tracked. This has a couple usage: 1. to track direct call and indirect callsite 2. to model inline contexts with a list of callsite probes.

A probe discriminator is treated as empty or null for AutoFDO since it has lowest three bits as "111".

Thanks for the explanation.

clang/lib/Driver/ToolChains/Clang.cpp

 }
 
 static void renderDebugOptions(const ToolChain &TC, const Driver &D,
                                const llvm::Triple &T, const ArgList &Args,
                                bool EmitCodeView, bool IRInput,
                                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(
           Args.getLastArg(options::OPT_fdebug_info_for_profiling), Args, D, TC))
     CmdArgs.push_back("-fdebug-info-for-profiling");
 
   // The 'g' groups options involve a somewhat intricate sequence of decisions
   // about what to pass from the driver to the frontend, but by the time they

clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp

 /// Normally -g1 does not add linkageName. -fdebug-info-for-profiling adds linkageName.
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-kind=line-tables-only %s -o - | FileCheck %s --check-prefix=LINE
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-kind=line-tables-only -fdebug-info-for-profiling %s -o - | FileCheck %s
 
 // LINE: = distinct !DISubprogram(name: "foo", scope:
 
 // CHECK: = distinct !DICompileUnit({{.*}}, debugInfoForProfiling: true,
 // CHECK: = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope:
 
 /// Add a DWARF discriminators pass for PGO.
 // RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager -O1 -fprofile-instrument-path=a.profdata %s -o - 2>&1 | FileCheck %s --check-prefix=NODISCR
 // RUN: %clang_cc1 -emit-llvm -fno-legacy-pass-manager -fdebug-pass-manager -O1 -fprofile-instrument-path=a.profdata -fdebug-info-for-profiling %s -o - 2>&1 | FileCheck %s --check-prefix=DISCR
 
 // 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() {}

clang/test/Driver/pseudo-probe.c

 // 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

llvm/include/llvm/Passes/PassBuilder.h

PGOOptions(std::string ProfileFile = "", std::string CSProfileGenFile = "",
     // If CSAction is CSIRUse, PGOAction needs to be IRUse as they share
     // a profile.
     assert(this->CSAction != CSIRUse || this->Action == IRUse);
 
     // If neither Action nor CSAction, DebugInfoForProfiling or
     // 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;
   std::string ProfileRemappingFile;
   PGOAction Action;
   CSPGOAction CSAction;
   bool DebugInfoForProfiling;
   bool PseudoProbeForProfiling;

llvm/test/Transforms/SampleProfile/pseudo-probe-discriminator.ll

+; RUN: opt < %s -passes='default<O2>' -new-pm-debug-info-for-profiling -S | FileCheck %s --check-prefix=DEBUG
+; RUN: opt < %s -passes='default<O2>' -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<O2>' -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)

llvm/tools/opt/NewPMDriver.cpp

bool llvm::runPassPipeline(StringRef Arg0, Module &M, TargetMachine *TM,
   case InstrUse:
     P = PGOOptions(ProfileFile, "", ProfileRemappingFile, PGOOptions::IRUse);
     break;
   case SampleUse:
     P = PGOOptions(ProfileFile, "", ProfileRemappingFile,
                    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;
   }
   if (CSPGOKindFlag != NoCSPGO) {
     if (P && (P->Action == PGOOptions::IRInstr ||
               P->Action == PGOOptions::SampleUse))
       errs() << "CSPGOKind cannot be used with IRInstr or SampleUse";
     if (CSPGOKindFlag == CSInstrGen) {