Make new PM honor -fdebug-info-for-profiling (clang side)

[danielcdh]

Summary

The new PM needs to invoke add-discriminator pass when building with -fdebug-info-for-profiling.

[danielcdh]

update

[chandlerc]

lib/CodeGen/BackendUtil.cpp

843

Maybe make this an optional to avoid the big predicate below?

844–865

At least some of these seem mutually exclusive, so show that with else if?

I wonder, are all of these mutually exclusive?

[danielcdh]

lib/CodeGen/BackendUtil.cpp

844–865

-fprofile-generate, -fprofile-use and -fprofile-sample-use are mutually exclusive.

[danielcdh]

update

[chandlerc]

LGTM with a tiny tweak below.

Would be good to add a test that this flag is being honored, either in this patch or in a follow-up.

lib/CodeGen/BackendUtil.cpp

862–863

No need for this? The optional should be disengaged by default when constructed as you do above.

[chandlerc]

(Also, sorry for the delay reviewing this, for some reason I thought this already got reviewed and landed...)

[danielcdh]

Thanks for the review!

In D35746#822498, @chandlerc wrote:

LGTM with a tiny tweak below.

Would be good to add a test that this flag is being honored, either in this patch or in a follow-up.

I'll add the test in a follow-up patch. Could you help suggest how to add a test for this? Shall I just invoke "clang -cc1 -fexperimental-new-pass-manager" and check if discriminator is generated? That seems an integration test.

lib/CodeGen/BackendUtil.cpp

void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
   // The new pass manager always makes a target machine available to passes
   // during construction.
   CreateTargetMachine(/*MustCreateTM*/ true);
   if (!TM)
     // This will already be diagnosed, just bail.
     return;
   TheModule->setDataLayout(TM->createDataLayout());
 
-  PGOOptions PGOOpt;
+  Optional<PGOOptions> PGOOpt;
 
+  if (CodeGenOpts.hasProfileIRInstr())
     // -fprofile-generate.
-  PGOOpt.RunProfileGen = CodeGenOpts.hasProfileIRInstr();
-  if (PGOOpt.RunProfileGen)
-    PGOOpt.ProfileGenFile = CodeGenOpts.InstrProfileOutput.empty() ?
-      DefaultProfileGenName : CodeGenOpts.InstrProfileOutput;
-
+    PGOOpt = PGOOptions(CodeGenOpts.InstrProfileOutput.empty()
+                            ? DefaultProfileGenName
+                            : CodeGenOpts.InstrProfileOutput,
+                        "", "", true, CodeGenOpts.DebugInfoForProfiling);
+  else if (CodeGenOpts.hasProfileIRUse())
     // -fprofile-use.
-  if (CodeGenOpts.hasProfileIRUse())
-    PGOOpt.ProfileUseFile = CodeGenOpts.ProfileInstrumentUsePath;
-
-  if (!CodeGenOpts.SampleProfileFile.empty())
-    PGOOpt.SampleProfileFile = CodeGenOpts.SampleProfileFile;
+    PGOOpt = PGOOptions("", CodeGenOpts.ProfileInstrumentUsePath, "", false,
+                        CodeGenOpts.DebugInfoForProfiling);
+  else if (!CodeGenOpts.SampleProfileFile.empty())
+    // -fprofile-sample-use
+    PGOOpt = PGOOptions("", "", CodeGenOpts.SampleProfileFile, false,
+                        CodeGenOpts.DebugInfoForProfiling);
+  else if (CodeGenOpts.DebugInfoForProfiling)
+    // -fdebug-info-for-profiling
+    PGOOpt = PGOOptions("", "", "", false, true);
+  else
+    PGOOpt = None;
 
-  // Only pass a PGO options struct if -fprofile-generate or
-  // -fprofile-use were passed on the cmdline.
-  PassBuilder PB(TM.get(),
-    (PGOOpt.RunProfileGen ||
-      !PGOOpt.ProfileUseFile.empty() ||
-      !PGOOpt.SampleProfileFile.empty()) ?
-        Optional<PGOOptions>(PGOOpt) : None);
+  PassBuilder PB(TM.get(), PGOOpt);
 
   LoopAnalysisManager LAM;
   FunctionAnalysisManager FAM;
   CGSCCAnalysisManager CGAM;
   ModuleAnalysisManager MAM;
 
   // Register the AA manager first so that our version is the one used.
   FAM.registerPass([&] { return PB.buildDefaultAAPipeline(); });