Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GitHub Actions: Extend PGO to C++ parts of LDC #3982

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

kinke
Copy link
Member

@kinke kinke commented May 15, 2022

[on top of #3980]

@kinke kinke force-pushed the pgo2 branch 5 times, most recently from fd03b0f to e35cace Compare May 16, 2022 16:08
@kinke
Copy link
Member Author

kinke commented May 17, 2022

  • Doesn't work on macOS yet, because it's not using a host clang with the same LLVM version.
  • Loads of hash mismatches on both Windows and Linux. The main difference between instrumented and PGO'd build is that the former does not use LTO at the moment. This wasn't an issue for the frontend, but that's compiled to a single object file.

@kinke
Copy link
Member Author

kinke commented May 18, 2022

Loads of hash mismatches on both Windows and Linux.

Using LTO for the instrumented binary too hasn't gotten rid of those, although the relevant CMake flags should be identical for both instrumented and PGO'd builds now. Perhaps due to vanilla clang 14.0.0 vs. LDC-LLVM v14.0.3?!

@kinke
Copy link
Member Author

kinke commented May 18, 2022

Ah, so -fprofile-instr-* != -fprofile-* for clang either - it works now on Linux. On Windows, clang-cl supports -fprofile-generate, but not -fprofile-use (edit: llvm/llvm-project#55564)...

@kinke kinke force-pushed the pgo2 branch 3 times, most recently from 004e093 to 2335c2d Compare May 18, 2022 18:12
@JohanEngelen
Copy link
Member

Ah, so -fprofile-instr-* != -fprofile-* for clang either

Sorry for not being able to help you with the issues. But yes, I did not reinvent anything new, the LDC commandline options are meant to do exactly the thing that clang does.

@kinke
Copy link
Member Author

kinke commented May 19, 2022

There are some WTF moments here. So first of all, using vanilla clang 14 as C(++) compiler for the macOS jobs worked fine, even for arm64 cross-compilation. I was expecting trouble with the Xcode SDK, but the only problem was that it's somehow using llvm-ranlib when specifying that clang, which supports no cmdline switches at all and required disabling a (in that case superfluous) CMake workaround. So macOS is basically fine too.

For Windows, instrumentation works fine, but the C++ parts don't actually make use of the profile later on, due to the filed clang-cl oversight. The unsupported cmdline switch is only a warning, not an error, so it'll automatically work as soon as clang-cl supports it.

So everything was looking pretty great, until I tried a CI run without assertions. Windows is still fine, but on Linux and macOS x86_64, there are now lots of hash mismatches, apparently just for the D parts.

@kinke
Copy link
Member Author

kinke commented May 19, 2022

Okay, that's sorted now too; those hash mismatches originated from the D parts of the ldc2-unittest executable, which don't use the PGO profiles anymore now.

@kinke
Copy link
Member Author

kinke commented May 19, 2022

For some Symmetry code and a Linux debug build, I'm seeing some ~1% reduction in compile times over frontend-PGO only.

As DFLAGS_LDC affect the ldc2-unittest executable too, which is built
without optimizations (but with `-g -unittest`).
@kinke kinke marked this pull request as ready for review May 19, 2022 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants