[NFC] Do not run CGProfilePass when not using integrated assembler

[zhizhouy]

Summary

CGProfilePass is run by default in certain new pass manager optimization pipeline. Assemblers other than llvm as (such as gnu as) cannot recognize the .cgprofile entries generated and emitted from this pass, causing build time error.

This patch adds new options in clang CodeGenOpts and PassBuilder options so that we can turn cgprofile off when not using integrated assembler.

[Bigcheese]

I find it a bit odd to have clang care about this, but I suppose it is the driver that knows which features tools support. I also have an issue with the name as CGProfilePass doesn't actually do the sorting. I would just keep the option as -enable-call-graph-profile-emission or something like that.

[zhizhouy]

IIUC, user will always call clang to pass integrated as flag. Actually I also used to consider checking UseIntegratedAS-ish in PassBuilder,
but looking for a codegen options in PassBuilder seems going to break the integrity. Solutions for this is appreciated.

In D62627#1522048, @Bigcheese wrote:

I also have an issue with the name as CGProfilePass doesn't actually do the sorting

Sorry I mis-used the "sort" here, changing it to "enable-npm-call-graph-profile" to match the other newpm options in PassBuilder.

[zhizhouy]

Renaming the opt.

[manojgupta]

clang/lib/Driver/ToolChains/Clang.cpp

4000 ↗

(On Diff #202057)

llvm MC assembler? llvm-as is for converting text ll to bitcode files.

[zhizhouy]

Fixed comment.

clang/lib/Driver/ToolChains/Clang.cpp

4000 ↗

(On Diff #202057)

Thanks for correction.

[Bigcheese]

In D62627#1522081, @zhizhouy wrote:

IIUC, user will always call clang to pass integrated as flag. Actually I also used to consider checking UseIntegratedAS-ish in PassBuilder,
but looking for a codegen options in PassBuilder seems going to break the integrity. Solutions for this is appreciated.

There are frontends and users of llvm other than clang.

Also all of the other options there are useful for experimenting with new passes, none of them are required for correctness, and none of them are ever set by clang.

Adding Chandler to see if he has a good solution.

[zhizhouy]

Friendly ping :)

[chandlerc]

clang/lib/Driver/ToolChains/Clang.cpp

3999–4003 ↗

(On Diff #202065)

the call garph profile stuff needs to be a Clang CodeGenOpt IMO. Then it can be used to set a bool in the pass builder's option struct (see my other comment).

We can then set the CodeGenOpt based on whether we are using the integrated assembler.

We could also have a separate flag for setting the codegen opt for getting this profile information, and make the driver diagnose an incompatible combination of no-integrated-as and that flag. But we'll still end up threading that flag through to Clang's codegen layer, and then into the pass manager.

llvm/lib/Passes/PassBuilder.cpp

1068–1069

We shouldn't just use the cl::opt here. We need a flag in the pass builder's options struct. We can default it from the cl::opt.

You should add a test that uses the cl::opt to flip the default back and forth and get the different behavior in LLVM directly against the opt tool. Then the LLVM part of this can land.

[manojgupta]

Thanks,

Noticed a few typos. Rest lgtm but deferring to other reviewers for now for approval.

clang/lib/CodeGen/BackendUtil.cpp

1111

Typo: PTO.CallGrpahProfile -> PTO.CallGraphProfile

1522

Same typo.

llvm/include/llvm/Passes/PassBuilder.h

111

CallGrpahProfile -> CallGraphProfile

[xur]

look good to me with the fix of comments.

llvm/include/llvm/Passes/PassBuilder.h

110

The default value is specified by -enable-npm-call-graph-profile, rather than true, right?

[zhizhouy]

Thanks for the comments.

[void]

Friendly ping. :-)

[MaskRay]

I do not see a strong argument for -enable-npm-call-graph-profile. We just need Opts.CallGraphProfile = !Opts.DisableIntegratedAS
The option does not appear to be very useful to me.

clang/include/clang/Basic/CodeGenOptions.def

 CODEGENOPT(UnrollLoops       , 1, 0) ///< Control whether loops are unrolled.
 CODEGENOPT(RerollLoops       , 1, 0) ///< Control whether loops are rerolled.
 CODEGENOPT(NoUseJumpTables   , 1, 0) ///< Set when -fno-jump-tables is enabled.
 CODEGENOPT(UnsafeFPMath      , 1, 0) ///< Allow unsafe floating point optzns.
 CODEGENOPT(UnwindTables      , 1, 0) ///< Emit unwind tables.
 CODEGENOPT(VectorizeLoop     , 1, 0) ///< Run loop vectorizer.
 CODEGENOPT(VectorizeSLP      , 1, 0) ///< Run SLP vectorizer.
 CODEGENOPT(ProfileSampleAccurate, 1, 0) ///< Sample profile is accurate.
+CODEGENOPT(CallGraphProfile  , 1, 0) ///< Run call graph profile.
 
   /// Attempt to use register sized accesses to bit-fields in structures, when
   /// possible.
 CODEGENOPT(UseRegisterSizedBitfieldAccess , 1, 0)
 
 CODEGENOPT(VerifyModule      , 1, 1) ///< Control whether the module should be run
                                      ///< through the LLVM Verifier.
 

clang/lib/CodeGen/BackendUtil.cpp

void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
 
   PipelineTuningOptions PTO;
   PTO.LoopUnrolling = CodeGenOpts.UnrollLoops;
   // For historical reasons, loop interleaving is set to mirror setting for loop
   // unrolling.
   PTO.LoopInterleaving = CodeGenOpts.UnrollLoops;
   PTO.LoopVectorization = CodeGenOpts.VectorizeLoop;
   PTO.SLPVectorization = CodeGenOpts.VectorizeSLP;
+  PTO.CallGraphProfile = CodeGenOpts.CallGraphProfile;
   PTO.Coroutines = LangOpts.Coroutines;
 
   PassInstrumentationCallbacks PIC;
   StandardInstrumentations SI;
   SI.registerCallbacks(PIC);
   PassBuilder PB(TM.get(), PTO, PGOOpt, &PIC);
 
   // Attempt to load pass plugins and register their callbacks with PB.
static void runThinLTOBackend(ModuleSummaryIndex *CombinedIndex, Module *M,
   initTargetOptions(Conf.Options, CGOpts, TOpts, LOpts, HeaderOpts);
   Conf.SampleProfile = std::move(SampleProfile);
   Conf.PTO.LoopUnrolling = CGOpts.UnrollLoops;
   // For historical reasons, loop interleaving is set to mirror setting for loop
   // unrolling.
   Conf.PTO.LoopInterleaving = CGOpts.UnrollLoops;
   Conf.PTO.LoopVectorization = CGOpts.VectorizeLoop;
   Conf.PTO.SLPVectorization = CGOpts.VectorizeSLP;
+  Conf.PTO.CallGraphProfile = CGOpts.CallGraphProfile;
 
   // Context sensitive profile.
   if (CGOpts.hasProfileCSIRInstr()) {
     Conf.RunCSIRInstr = true;
     Conf.CSIRProfile = std::move(CGOpts.InstrProfileOutput);
   } else if (CGOpts.hasProfileCSIRUse()) {
     Conf.RunCSIRInstr = false;
     Conf.CSIRProfile = std::move(CGOpts.ProfileInstrumentUsePath);

clang/lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   if (Opts.SimplifyLibCalls)
     getAllNoBuiltinFuncValues(Args, Opts.NoBuiltinFuncs);
   Opts.UnrollLoops =
       Args.hasFlag(OPT_funroll_loops, OPT_fno_unroll_loops,
                    (Opts.OptimizationLevel > 1));
   Opts.RerollLoops = Args.hasArg(OPT_freroll_loops);
 
   Opts.DisableIntegratedAS = Args.hasArg(OPT_fno_integrated_as);
+  Opts.CallGraphProfile = !Opts.DisableIntegratedAS;
   Opts.Autolink = !Args.hasArg(OPT_fno_autolink);
   Opts.SampleProfileFile =
       std::string(Args.getLastArgValue(OPT_fprofile_sample_use_EQ));
   Opts.DebugInfoForProfiling = Args.hasFlag(
       OPT_fdebug_info_for_profiling, OPT_fno_debug_info_for_profiling, false);
   Opts.DebugNameTable = static_cast<unsigned>(
       Args.hasArg(OPT_ggnu_pubnames)
           ? llvm::DICompileUnit::DebugNameTableKind::GNU

llvm/include/llvm/Passes/PassBuilder.h

public:
 
   /// Tuning option to cap the number of calls to retrive clobbering accesses in
   /// MemorySSA, in LICM.
   unsigned LicmMssaOptCap;
 
   /// Tuning option to disable promotion to scalars in LICM with MemorySSA, if
   /// the number of access is too large.
   unsigned LicmMssaNoAccForPromotionCap;
+
+  /// Tuning option to enable/disable call graph profile. Its default value is
+  /// that of the flag: `-enable-npm-call-graph-profile`.
+  bool CallGraphProfile;
 };
 
 /// This class provides access to building LLVM's passes.
 ///
 /// Its members provide the baseline state available to passes during their
 /// construction. The \c PassRegistry.def file specifies how to construct all
 /// of the built-in passes, and those may reference these members during
 /// construction.

llvm/lib/Passes/PassBuilder.cpp

static const Regex DefaultAliasRegex(
     "^(default|thinlto-pre-link|thinlto|lto-pre-link|lto)<(O[0123sz])>$");
 
 // This option is used in simplifying testing SampleFDO optimizations for
 // profile loading.
 static cl::opt<bool>
     EnableCHR("enable-chr-npm", cl::init(true), cl::Hidden,
               cl::desc("Enable control height reduction optimization (CHR)"));
 
+static cl::opt<bool> EnableCallGraphProfile(
+    "enable-npm-call-graph-profile", cl::init(true), cl::Hidden,
+    cl::desc("Enable call graph profile pass for the new PM (default = on)"));
+
 PipelineTuningOptions::PipelineTuningOptions() {
   LoopInterleaving = EnableLoopInterleaving;
   LoopVectorization = EnableLoopVectorization;
   SLPVectorization = RunSLPVectorization;
   LoopUnrolling = true;
   ForgetAllSCEVInLoopUnroll = ForgetSCEVInLoopUnroll;
   Coroutines = false;
   LicmMssaOptCap = SetLicmMssaOptCap;
   LicmMssaNoAccForPromotionCap = SetLicmMssaNoAccForPromotionCap;
+  CallGraphProfile = EnableCallGraphProfile;
 }
 
 extern cl::opt<bool> EnableHotColdSplit;
 extern cl::opt<bool> EnableOrderFileInstrumentation;
 
 extern cl::opt<bool> FlattenedProfileUsed;
 
 const PassBuilder::OptimizationLevel PassBuilder::OptimizationLevel::O0 = {
if (PTO.Coroutines)
     OptimizePM.addPass(CoroCleanupPass());
 
   for (auto &C : OptimizerLastEPCallbacks)
     C(OptimizePM, Level);
 
   // Add the core optimizing pipeline.
   MPM.addPass(createModuleToFunctionPassAdaptor(std::move(OptimizePM)));
 
+  if (PTO.CallGraphProfile)
     MPM.addPass(CGProfilePass());
 
   // Now we need to do some global optimization transforms.
   // FIXME: It would seem like these should come first in the optimization
   // pipeline and maybe be the bottom of the canonicalization pipeline? Weird
   // ordering here.
   MPM.addPass(GlobalDCEPass());
   MPM.addPass(ConstantMergePass());
 

llvm/test/Other/new-pm-cgprofile.ll

+; RUN: opt -debug-pass-manager -passes='default<O2>' %s 2>&1 |FileCheck %s --check-prefixes=DEFAULT
+; RUN: opt -debug-pass-manager -passes='default<O2>' -enable-npm-call-graph-profile=0 %s 2>&1 |FileCheck %s --check-prefixes=OFF
+; RUN: opt -debug-pass-manager -passes='default<O2>' -enable-npm-call-graph-profile=1 %s 2>&1 |FileCheck %s --check-prefixes=ON
+;
+; DEFAULT: Running pass: CGProfilePass
+; OFF-NOT: Running pass: CGProfilePass
+; ON: Running pass: CGProfilePass
+
+define void @foo() {
+  ret void
+}