[Driver] Add -f[no-]legacy-pass-manager to supersede -f[no-]experimental-new-pass-manager

[MaskRay]

Summary

The new PM is considered stable and many downstream groups have adopted it (some
have adopted it for more than two years). Add -f[no-]legacy-pass-manager to reflect the
fact that it is no longer experimental and the legacy pass manager is something we strive to retire.

In the future, when the legacy PM eventually goes away,
-fno-experimental-new-pass-manager and -flegacy-pass-manager will be removed.

This patch also changes -f[no-]legacy-pass-manager to pass -plugin-opt={new,legacy}-pass-manager to the linker (supported by both ld.lld and LLVMgold.so) when -flto/-flto=thin is specified

[MaskRay]

Improve tests

[aeubanks]

Makes sense to me, thanks!

[rsmith]

I don't like having new in the name of the flag; I think that's going to age badly (and I would argue it already has done; the "new pass manager" isn't really all that new any more).

Given that the new pass manager is considered stable and not experimental, can we change our default and instead add a flag to request the old behavior? If we're not ready for that yet, then perhaps the "experimental" label still fits.

[MaskRay]

Add -f[no-]legacy-pass-manager instead

[MaskRay]

Thanks!

@rsmith highlit the coroutine problem https://bugs.llvm.org/show_bug.cgi?id=48190 (" ... it breaks support for a C++20 language feature (coroutines)")

Perhaps this means that "EXPERIMENTAL" in ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER should not be removed until it is fixed?

[aeubanks]

In D92915#2444242, @MaskRay wrote:

Thanks!

@rsmith highlit the coroutine problem https://bugs.llvm.org/show_bug.cgi?id=48190 (" ... it breaks support for a C++20 language feature (coroutines)")

Perhaps this means that "EXPERIMENTAL" in ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER should not be removed until it is fixed?

I'm looking at this right now, it's the last issue, aside from AMDGPU passes not being ported and some code size regressions, blocking the new pass manager.

[MaskRay]

-plugin-opt=no-new-pass-manager => -plugin-opt=legacy-pass-manager

[thakis]

Seems to break tests: http://45.33.8.238/linux/34907/step_7.txt

[aeubanks]

In D92915#2444287, @thakis wrote:

Seems to break tests: http://45.33.8.238/linux/34907/step_7.txt

that test fails when the NPM is turned on and lit isn't aware of it :)

clang/include/clang/Driver/Options.td

1354–1361

this turns on the new pass manager for clang

[MaskRay]

clang/include/clang/Driver/Options.td

1354–1361

My bad. Has been fixed in 872bf680011a71b73e9bd353449d9c42ebf6b8f6

clang/include/clang/Driver/Options.td

def finline_functions : Flag<["-"], "finline-functions">, Group<f_clang_Group>, Flags<[CC1Option]>,
   HelpText<"Inline suitable functions">;
 def finline_hint_functions: Flag<["-"], "finline-hint-functions">, Group<f_clang_Group>, Flags<[CC1Option]>,
   HelpText<"Inline functions which are (explicitly or implicitly) marked inline">;
 def finline : Flag<["-"], "finline">, Group<clang_ignored_f_Group>;
 def fglobal_isel : Flag<["-"], "fglobal-isel">, Group<f_clang_Group>,
   HelpText<"Enables the global instruction selector">;
 def fexperimental_isel : Flag<["-"], "fexperimental-isel">, Group<f_clang_Group>,
   Alias<fglobal_isel>;
-defm experimental_new_pass_manager : BooleanMarshalledFFlag<"experimental-new-pass-manager", "CodeGenOpts.ExperimentalNewPassManager",
-  "static_cast<unsigned>(LLVM_ENABLE_NEW_PASS_MANAGER)", "Enables an experimental new pass manager in LLVM.",
-  "Disables an experimental new pass manager in LLVM.">, Group<f_clang_Group>, Flags<[CC1Option]>;
+defm new_pass_manager : BooleanMarshalledFFlag<"new-pass-manager", "CodeGenOpts.ExperimentalNewPassManager",
+  "static_cast<unsigned>(LLVM_ENABLE_NEW_PASS_MANAGER)", "Use the new pass manager in LLVM",
+  "Use the legacy pass manager in LLVM">, Group<f_clang_Group>, Flags<[CC1Option]>;
+def fexperimental_new_pass_manager : Flag<["-"], "fexperimental-new-pass-manager">,
+  Group<f_clang_Group>, Flags<[CC1Option]>, Alias<fnew_pass_manager>;
+def fno_experimental_new_pass_manager : Flag<["-"], "fno-experimental-new-pass-manager">,
+  Group<f_clang_Group>, Flags<[CC1Option]>, Alias<fno_new_pass_manager>;
 def fexperimental_strict_floating_point : Flag<["-"], "fexperimental-strict-floating-point">,
   Group<f_clang_Group>, Flags<[CC1Option]>,
   HelpText<"Enables experimental strict floating point in LLVM.">,
   MarshallingInfoFlag<"LangOpts->ExpStrictFP">;
 def finput_charset_EQ : Joined<["-"], "finput-charset=">, Group<f_Group>;
 def fexec_charset_EQ : Joined<["-"], "fexec-charset=">, Group<f_Group>;
 def finstrument_functions : Flag<["-"], "finstrument-functions">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Generate calls to instrument function entry and exit">;
 def finstrument_functions_after_inlining : Flag<["-"], "finstrument-functions-after-inlining">, Group<f_Group>, Flags<[CC1Option]>,

clang/lib/Driver/ToolChains/Clang.cpp

if (Args.hasFlag(options::OPT_fpch_instantiate_templates,
     CmdArgs.push_back("-fpch-instantiate-templates");
   if (Args.hasFlag(options::OPT_fpch_codegen, options::OPT_fno_pch_codegen,
                    false))
     CmdArgs.push_back("-fmodules-codegen");
   if (Args.hasFlag(options::OPT_fpch_debuginfo, options::OPT_fno_pch_debuginfo,
                    false))
     CmdArgs.push_back("-fmodules-debuginfo");
 
-  Args.AddLastArg(CmdArgs, options::OPT_fexperimental_new_pass_manager,
-                  options::OPT_fno_experimental_new_pass_manager);
+  Args.AddLastArg(CmdArgs, options::OPT_fnew_pass_manager,
+                  options::OPT_fno_new_pass_manager);
 
   ObjCRuntime Runtime = AddObjCRuntimeArgs(Args, Inputs, CmdArgs, rewriteKind);
   RenderObjCOptions(TC, D, RawTriple, Args, Runtime, rewriteKind != RK_None,
                     Input, CmdArgs);
 
   if (Args.hasFlag(options::OPT_fapplication_extension,
                    options::OPT_fno_application_extension, false))
     CmdArgs.push_back("-fapplication-extension");

clang/lib/Driver/ToolChains/CommonArgs.cpp

SmallString<128> Path(
         ProfileUseArg->getNumValues() == 0 ? "" : ProfileUseArg->getValue());
     if (Path.empty() || llvm::sys::fs::is_directory(Path))
       llvm::sys::path::append(Path, "default.profdata");
     CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-opt=cs-profile-path=") +
                                          Path));
   }
 
   // Need this flag to turn on new pass manager via Gold plugin.
-  if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
-                   options::OPT_fno_experimental_new_pass_manager,
+  if (Args.hasFlag(options::OPT_fnew_pass_manager,
+                   options::OPT_fno_new_pass_manager,
                    /* Default */ LLVM_ENABLE_NEW_PASS_MANAGER)) {
     CmdArgs.push_back("-plugin-opt=new-pass-manager");
   }
 
   // Setup statistics file output.
   SmallString<128> StatsFile = getStatsFileName(Args, Output, Input, D);
   if (!StatsFile.empty())
     CmdArgs.push_back(

clang/test/Driver/clang_f_opts.c

 // CHECK-WCHAR1-NOT: -fwchar-type=short
 // CHECK-WCHAR2: -fwchar-type=short
 // CHECK-WCHAR2-NOT: -fwchar-type=int
 // DELIMITERS: {{^ *"}}
 
 // RUN: %clang -### -fno-experimental-new-pass-manager -fexperimental-new-pass-manager %s 2>&1 | FileCheck --check-prefix=CHECK-PM --check-prefix=CHECK-NEW-PM %s
 // RUN: %clang -### -fexperimental-new-pass-manager -fno-experimental-new-pass-manager %s 2>&1 | FileCheck --check-prefix=CHECK-PM --check-prefix=CHECK-NO-NEW-PM %s
 // CHECK-PM-NOT: argument unused
-// CHECK-NEW-PM: -fexperimental-new-pass-manager
-// CHECK-NEW-PM-NOT: -fno-experimental-new-pass-manager
-// CHECK-NO-NEW-PM: -fno-experimental-new-pass-manager
-// CHECK-NO-NEW-PM-NOT: -fexperimental-new-pass-manager
+// CHECK-NEW-PM: -fnew-pass-manager
+// CHECK-NEW-PM-NOT: -fno-new-pass-manager
+// CHECK-NO-NEW-PM: -fno-new-pass-manager
+// CHECK-NO-NEW-PM-NOT: -fnew-pass-manager
 
 // RUN: %clang -### -S -fstrict-return %s 2>&1 | FileCheck -check-prefix=CHECK-STRICT-RETURN %s
 // RUN: %clang -### -S -fno-strict-return %s 2>&1 | FileCheck -check-prefix=CHECK-NO-STRICT-RETURN %s
 // CHECK-STRICT-RETURN-NOT: "-fno-strict-return"
 // CHECK-NO-STRICT-RETURN: "-fno-strict-return"
 
 // RUN: %clang -### -S -fno-debug-info-for-profiling -fdebug-info-for-profiling %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-DEBUG %s
 // RUN: %clang -### -S -fdebug-info-for-profiling -fno-debug-info-for-profiling %s 2>&1 | FileCheck -check-prefix=CHECK-NO-PROFILE-DEBUG %s

clang/test/Driver/fnew-pass-manager.c

+// RUN: %clang -### -c -fno-new-pass-manager -fnew-pass-manager %s 2>&1 | FileCheck --check-prefixes=NOWARN,NEW %s
+// RUN: %clang -### -c -fnew-pass-manager -fno-new-pass-manager %s 2>&1 | FileCheck --check-prefixes=NOWARN,LEGACY %s
+
+/// -f[no-]experimental-new-pass-manager are legacy aliases when the new PM was still experimental.
+// RUN: %clang -### -c -fno-experimental-new-pass-manager -fexperimental-new-pass-manager %s 2>&1 | FileCheck --check-prefixes=NOWARN,NEW %s
+// RUN: %clang -### -c -fexperimental-new-pass-manager -fno-experimental-new-pass-manager %s 2>&1 | FileCheck --check-prefixes=NOWARN,LEGACY %s
+
+// NOWARN-NOT: warning: argument unused
+
+// NEW:        -fnew-pass-manager
+// NEW-NOT:    -fno-new-pass-manager
+
+// LEGACY:     -fno-new-pass-manager
+// LEGACY-NOT: -fnew-pass-manager
+
+/// For full/Thin LTO, -fnew-pass-manager passes -plugin-opt=new-pass-manager to the linker (which may not be LLD).
+// RUN: %clang -### -target x86_64-linux -flto -fnew-pass-manager %s 2>&1 | FileCheck --check-prefix=LTO_NEW %s
+// RUN: %clang -### -target x86_64-linux -flto=thin -fexperimental-new-pass-manager %s 2>&1 | FileCheck --check-prefix=LTO_NEW %s
+
+// LTO_NEW:    "-plugin-opt=new-pass-manager"
+
+/// TODO Add the negative option and pass it to the linker.
+// RUN: %clang -### -target x86_64-linux -flto -fno-new-pass-manager %s 2>&1 | FileCheck --check-prefix=LTO_LEGACY %s
+// RUN: %clang -### -target x86_64-linux -flto=thin -fno-experimental-new-pass-manager %s 2>&1 | FileCheck --check-prefix=LTO_LEGACY %s
+
+// LTO_LEGACY-NOT: "-plugin-opt=new-pass-manager"

clang/test/Driver/gold-lto-new-pass-man.c

-// RUN: touch %t.o
-//
-// RUN: %clang -target ppc64le-unknown-linux -### %t.o -flto 2>&1 \
-// RUN:     -Wl,-plugin-opt=foo -O3 \
-// RUN:     -fexperimental-new-pass-manager \
-// RUN:     | FileCheck %s
-// CHECK: "-plugin-opt=new-pass-manager"