hoy created this revision.
Herald added subscribers: modimo, wenlei.
hoy requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107876

Files:
  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/test/Transforms/SampleProfile/pseudo-probe-discriminator.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===================================================================
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -259,12 +259,9 @@
                    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;
   }
Index: llvm/test/Transforms/SampleProfile/pseudo-probe-discriminator.ll
===================================================================
--- /dev/null
+++ 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
+
+
+@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)
Index: llvm/include/llvm/Passes/PassBuilder.h
===================================================================
--- llvm/include/llvm/Passes/PassBuilder.h
+++ llvm/include/llvm/Passes/PassBuilder.h
@@ -65,14 +65,6 @@
     // 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;
Index: clang/test/Driver/pseudo-probe.c
===================================================================
--- clang/test/Driver/pseudo-probe.c
+++ 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
Index: clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp
===================================================================
--- clang/test/CodeGenCXX/fdebug-info-for-profiling.cpp
+++ 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() {}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3892,12 +3892,6 @@
                                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(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to