[PATCH] D62225: [clang][NewPM] Fixing -O0 tests that are broken under new PM

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

I think this ultimately needs to be split up into smaller patches. A bunch of these things can be landed independently. Here is my first cut at things to split out, each one into its own patch.

1. the LLVM change to the always inliner
2. the Clang change to how we build the always inliner
3. the PGO pipeline changes (which I have to admit I still don't fully understand)
4. The additions of `-fno-experimental-new-pass-manager` for test cases that are explicitly testing legacy PM behavior
5. switching tests to be resilient to changes in attribute group numbering (and adding a RUN line w/ the new PM to ensure we don't regress)

for #5 (or others) where *some* testing needs to be working before they can land, just sequence them after whatever they depend on

Other things I think can also be split out, but I suspect into *different* changes from what you have here:

6. Instead of passing `-fno-experimental-new-pass-manager` for tests that use `-O` but don't specify a number, Clang should pick a consistent value for the level I think

I'd be interested to then see what is left here.



================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1104-1105
       // which is just that always inlining occurs.
-      MPM.addPass(AlwaysInlinerPass());
+      // We always pass false here since according to the legacy PM logic for
+      // enabling lifetime intrinsics, we should not be compiling with O0.
+      MPM.addPass(AlwaysInlinerPass(/*InsertLifetimeIntrinsics=*/false));
----------------
leonardchan wrote:
> serge-sans-paille wrote:
> > echristo wrote:
> > > Can you elaborate more here? We do turn on the always inliner at O0 which makes this comment a bit confusing.
> > I guess he means 
> >     
> >     We always pass false here since according to the legacy PM logic for enabling lifetime intrinsics, they are not required with O0
> >     
> Yup, my bad. This is what I meant with this comment. Always inlining is used. It's the lifetime intrinsics that aren't always used.
I don't think explaining it in terms of one pass manager or another is the righ thing to do.

Instead, I'd say what the desired result is:

```
Build a minimal pipeline based on the semantics required by Clang,
which is just that always inlining occurs. Further, disable generating
lifetime intrinsics to avoid enabling further optimizations during
code generation.
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62225/new/

https://reviews.llvm.org/D62225