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

Hongtao Yu via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 12 08:53:11 PDT 2021


Author: Hongtao Yu
Date: 2021-08-12T08:52:49-07:00
New Revision: ccb5b9bbfb5cef1aa2982481894f30c8f81d5253

URL: https://github.com/llvm/llvm-project/commit/ccb5b9bbfb5cef1aa2982481894f30c8f81d5253
DIFF: https://github.com/llvm/llvm-project/commit/ccb5b9bbfb5cef1aa2982481894f30c8f81d5253.diff

LOG: [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

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

Modified: 
    clang/lib/Driver/ToolChains/Clang.cpp
    clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp
    clang/test/Driver/pseudo-probe.c
    llvm/include/llvm/Passes/PassBuilder.h
    llvm/tools/opt/NewPMDriver.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index ceeae94e56678..e19e1222f702e 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 0a66818b23be3..421195db83408 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 79b23df557a6c..76c4364e609d0 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 35f791f9c2609..9ab7bd4664f59 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 
diff erent
-    // 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 0000000000000..c88db45ae355b
--- /dev/null
+++ b/llvm/test/Transforms/SampleProfile/pseudo-probe-discriminator.ll
@@ -0,0 +1,71 @@
+; 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
+
+
+ at 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 4617df0c28632..d88636a4d784c 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;
   }


        


More information about the cfe-commits mailing list