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

Leonard Chan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 11 18:38:55 PDT 2019


leonardchan added a comment.

In D62225#1537248 <https://reviews.llvm.org/D62225#1537248>, @chandlerc wrote:

> 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.


Done. I split them into 6 other patches plus this one.

> 1. the LLVM change to the always inliner

Addressed in D63170 <https://reviews.llvm.org/D63170>.

> 2. the Clang change to how we build the always inliner

Addressed in D63153 <https://reviews.llvm.org/D63153>.

> 3. the PGO pipeline changes (which I have to admit I still don't fully understand)

Addressed in D63155 <https://reviews.llvm.org/D63155>. There aren't any logical changes to PGO in this. This just adds `PGOInstrumentationGenCreateVar` to the pipeline such that `./default.profraw` is generated and allows `Profile/gcc-flag-compatibility.c` to pass.

> 4. The additions of `-fno-experimental-new-pass-manager` for test cases that are explicitly testing legacy PM behavior

Addressed in D63156 <https://reviews.llvm.org/D63156>. I also combined #6 in this one since those `-O` tests are pretty much `-O2` tests.

> 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

Addressed in D63174 <https://reviews.llvm.org/D63174>. The tests that were failing due to attribute numbering don't seem to fail anymore, so someone must've addressed that in a previous patch. This one now just contains the tests that produces slightly different IR.

> 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

D63168 <https://reviews.llvm.org/D63168> seems separate from the rest and fixes `CodeGen/split-debug-single-file.c`.

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

The remaining ones here are the flatten tests and optimization remark tests. The flatten tests fail since the new PM AlwaysInliner seems to inline functions only and not calls (based off the description in D23299 <https://reviews.llvm.org/D23299>), so for now they are marked as UNSUPPORTED.

The optimization remark tests fail since it seems that the new PM inliner is not enabled at `-O0` and emit slightly different remarks. This patch forces those breaking runs to run with legacy PM only and adds separate new PM tests with `-O1` to enable the inliner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62225





More information about the cfe-commits mailing list