Re: r290450 - [PM] Introduce options to enable the (still experimental) new pass

[Chandler Carruth via cfe-commits]

(explicitly adding Richard so he sees this discussion as some of this
involves a discussion between myself and him)

On Tue, Jan 10, 2017 at 4:36 PM Justin Bogner via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Chandler Carruth via cfe-commits  writes:
> > Author: chandlerc
> > Date: Fri Dec 23 14:44:01 2016
> > New Revision: 290450
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=290450=rev
> > Log:
> > [PM] Introduce options to enable the (still experimental) new pass
> > manager, and a code path to use it.
> >
> > The option is actually a top-level option but does contain
> > 'experimental' in the name. This is the compromise suggested by Richard
> > in discussions. We expect this option will be around long enough and
> > have enough users towards the end that it merits not being relegated to
> > CC1, but it still needs to be clear that this option will go away at
> > some point.
>
> I don't really understand why this is a driver option and not just a CC1
> option. Using a driver flag makes me think we expect people to be using
> this in production before we make the new PM the default, which seems
> kind of questionable to me,


I tried to explain the thinking here, but sorry if it wasn't clear. And
maybe its just the wrong tradeoff.

My thought process is this: until we get users of Clang and LLVM using the
new PM in production, I think it's going to be between hard and impossible
to make it the default. So I've been trying to make sure we have a healthy
state for asking users to enable this including in production. Speaking
just for myself as a user, I will need to enable this in production prior
to it being the default.


> and I don't see how adding a new
> "-fexperimental" is any better than just using the "-Xclang" that people
> are already familiar with the implications of.
>

The "experimental" thing was not really intended to be anything like the
CC1 interface.

It communicates two things IMO: that the functionality is less mature at
this point, and that there will (eventually) be changes.

I would still expect us to go through a very slow process to remove this
flag, the same as I would expect for any other driver-level flag.

To give an idea of the kind of timeframe I'm trying to prepare for
(although it being faster would of course be a pleasant surprise): I
suspect we will need there to be an open source release with the
functionality finished and reasonably well tested but still not the
default. And in that release we'll want to in the release notes advertise
that this is coming and that the next release will flip the default. And
then I think we'll need to do *another* release where the old pass manager
is still around, but is not the default.

Maybe things will go substantially better/faster than I've described, but
I'm trying to have a plan that is viable even if things move that slowly.


Anyways, if the decision is to go back to a CC1 flag, we can do that. I
just really want to avoid that if possible as I'd like to avoid deploying a
CC1 flag to my users (we work actively to avoid doing this where possible).

-Chandler
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r290450 - [PM] Introduce options to enable the (still experimental) new pass

[Chandler Carruth via cfe-commits]

Author: chandlerc
Date: Fri Dec 23 14:44:01 2016
New Revision: 290450

URL: http://llvm.org/viewvc/llvm-project?rev=290450=rev
Log:
[PM] Introduce options to enable the (still experimental) new pass
manager, and a code path to use it.

The option is actually a top-level option but does contain
'experimental' in the name. This is the compromise suggested by Richard
in discussions. We expect this option will be around long enough and
have enough users towards the end that it merits not being relegated to
CC1, but it still needs to be clear that this option will go away at
some point.

The backend code is a fresh codepath dedicated to handling the flow with
the new pass manager. This was also Richard's suggested code structuring
to essentially leave a clean path for development rather than carrying
complexity or idiosyncracies of how we do things just to share code with
the parts of this in common with the legacy pass manager. And it turns
out, not much is really in common even though we use the legacy pass
manager for codegen at this point.

I've switched a couple of tests to run with the new pass manager, and
they appear to work. There are still plenty of bugs that need squashing
(just with basic experiments I've found two already!) but they aren't in
this code, and the whole point is to expose the necessary hooks to start
experimenting with the pass manager in more realistic scenarios.

That said, I want to *strongly caution* anyone itching to play with
this: it is still *very shaky*. Several large components have not yet
been shaken down. For example I have bugs in both the always inliner and
inliner that I have already spotted and will be fixing independently.

Still, this is a fun milestone. =D

One thing not in this patch (but that might be very reasonable to add)
is some level of support for raw textual pass pipelines such as what
Sean had a patch for some time ago. I'm mostly interested in the more
traditional flow of getting the IR out of Clang and then running it
through opt, but I can see other use cases so someone may want to add
it.

And of course, *many* features are not yet supported!
- O1 is currently more like O2
- None of the sanitizers are wired up
- ObjC ARC optimizer isn't wired up
- ...

So plenty of stuff still lef to do!

Differential Revision: https://reviews.llvm.org/D28077

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/CodeGen/arm64_crypto.c
cfe/trunk/test/CodeGen/inline-optim.c
cfe/trunk/test/Driver/clang_f_opts.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=290450=290449=290450=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Fri Dec 23 14:44:01 2016
@@ -823,6 +823,9 @@ def finline_functions : Flag<["-"], "fin
 def finline_hint_functions: Flag<["-"], "finline-hint-functions">, 
Group, Flags<[CC1Option]>,
   HelpText<"Inline functions wich are (explicitly or implicitly) marked 
inline">;
 def finline : Flag<["-"], "finline">, Group;
+def fexperimental_new_pass_manager : Flag<["-"], 
"fexperimental-new-pass-manager">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Enables an experimental new pass manager in LLVM.">;
 def finput_charset_EQ : Joined<["-"], "finput-charset=">, Group;
 def fexec_charset_EQ : Joined<["-"], "fexec-charset=">, Group;
 def finstrument_functions : Flag<["-"], "finstrument-functions">, 
Group, Flags<[CC1Option]>,
@@ -1000,6 +1003,9 @@ def fno_exceptions : Flag<["-"], "fno-ex
 def fno_gnu_keywords : Flag<["-"], "fno-gnu-keywords">, Group, 
Flags<[CC1Option]>;
 def fno_inline_functions : Flag<["-"], "fno-inline-functions">, 
Group, Flags<[CC1Option]>;
 def fno_inline : Flag<["-"], "fno-inline">, Group, 
Flags<[CC1Option]>;
+def fno_experimental_new_pass_manager : Flag<["-"], 
"fno-experimental-new-pass-manager">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Disables an experimental new pass manager in LLVM.">;
 def fveclib : Joined<["-"], "fveclib=">, Group, Flags<[CC1Option]>,
 HelpText<"Use the given vector functions library">;
 def fno_lax_vector_conversions : Flag<["-"], "fno-lax-vector-conversions">, 
Group,

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=290450=290449=290450=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Fri Dec 23 14:44:01 2016
@@ -52,6 +52,8 @@ CODEGENOPT(DisableGCov   , 1, 0) ///
 CODEGENOPT(DisableLLVMPasses , 1, 0)