Preparing BOLT for LLVM monorepo

Hi All,

Since the acceptance of the "BOLT framework for binary analysis,

transformation, and optimization" RFC

(https://lists.llvm.org/pipermail/llvm-dev/2020-October/145902.html),

we’ve been working on addressing the feedback and getting ready for

inclusion into the monorepo. We’ve cleaned up interfaces with core LLVM

libraries, refactored our codebase to match LLVM coding standards, and

added more open-source tests. Additionally, we’ve made BOLT faster and

more compact.

As of this writing, BOLT could be built as a separate project in the

LLVM monorepo without any patches other than build configuration

changes. You can view the latest sources at the monorepo fork

https://github.com/facebookincubator/BOLT, under “/bolt”.

During the past year, we were happy to see an ever-growing inflow of

pull requests from external contributors. We believe that accelerating

the monorepo integration will streamline the open-source review process

and make it more transparent.

While we are finishing the final steps before the integration

(https://github.com/facebookincubator/BOLT/issues/248), we would like to

hear from the community and address any remaining concerns. If

everything goes smoothly, we anticipate the merge to happen early next

month and in time for LLVM 14 release.

We are still working on finalizing the exact logistics of the merge.

However, we expect to follow the Flang project’s footsteps and run the

“–no-ff” merge to preserve the history of ~1K commits. We would like to

ask for help and coordination from the release managers Tom Stellard and

Hans Wennborg.

We haven’t made the final decision on including BOLT in

LLVM_ALL_PROJECTS, but we are ready to make BOLT the default project in

the initial merge commit. Currently, we do not support building on

Windows; thus, the project will be automatically disabled with the

warning when building on unsupported platforms. As we anticipate the

increased load on buildbots, we would like to know how we can help with

adding more build machines. Please let us know!

Thank you,

Maksim and BOLT Team

Hi All,

Since the acceptance of the "BOLT framework for binary analysis,

transformation, and optimization" RFC

(https://lists.llvm.org/pipermail/llvm-dev/2020-October/145902.html),

we've been working on addressing the feedback and getting ready for

inclusion into the monorepo. We've cleaned up interfaces with core LLVM

libraries, refactored our codebase to match LLVM coding standards, and

added more open-source tests. Additionally, we've made BOLT faster and

more compact.

As of this writing, BOLT could be built as a separate project in the

LLVM monorepo without any patches other than build configuration

changes. You can view the latest sources at the monorepo fork

https://github.com/facebookincubator/BOLT, under "/bolt".

During the past year, we were happy to see an ever-growing inflow of

pull requests from external contributors. We believe that accelerating

the monorepo integration will streamline the open-source review process

and make it more transparent.

Since you mentioned pull requests, I just wanted to make sure you are aware
that we don't use pull requests in the monorepo, so you community would need
to transition to doing reviews in Phabricator. Do you think this will be an
issue?

-Tom

If it is, then please report in this issue in Github:
https://github.com/llvm/llvm-iwg/issues/73

Hi All,

Since the acceptance of the "BOLT framework for binary analysis,

transformation, and optimization" RFC

(https://lists.llvm.org/pipermail/llvm-dev/2020-October/145902.html),

we’ve been working on addressing the feedback and getting ready for

inclusion into the monorepo. We’ve cleaned up interfaces with core LLVM

libraries, refactored our codebase to match LLVM coding standards, and

added more open-source tests. Additionally, we’ve made BOLT faster and

more compact.

I haven’t reviewed everything in detail, but on a high-level, that looks great!
Thanks for spending the effort on making this happen :slight_smile:

As of this writing, BOLT could be built as a separate project in the

LLVM monorepo without any patches other than build configuration

changes. You can view the latest sources at the monorepo fork

https://github.com/facebookincubator/BOLT, under “/bolt”.

During the past year, we were happy to see an ever-growing inflow of

pull requests from external contributors. We believe that accelerating

the monorepo integration will streamline the open-source review process

and make it more transparent.

While we are finishing the final steps before the integration

(https://github.com/facebookincubator/BOLT/issues/248), we would like to

hear from the community and address any remaining concerns. If

everything goes smoothly, we anticipate the merge to happen early next

month and in time for LLVM 14 release.

We are still working on finalizing the exact logistics of the merge.

However, we expect to follow the Flang project’s footsteps and run the

“–no-ff” merge to preserve the history of ~1K commits. We would like to

ask for help and coordination from the release managers Tom Stellard and

Hans Wennborg.

We haven’t made the final decision on including BOLT in

LLVM_ALL_PROJECTS, but we are ready to make BOLT the default project in

the initial merge commit.

I didn’t quite get this sentence?

Internally we’ve been using a custom version of Phabricator; it wouldn’t be an issue for us.

For the rest of the community, hopefully the upsides of switching to the monorepo will

compensate the potential downsides.

Hi Maksim,

I think this sounds great in general.

Making sure things are LLVM coding style, format, etc are pretty important to me. flang was a useful pilot here in a lot of ways and some of the requests for fixups still don’t really seem to have happened so making sure that happens before is pretty important to me.

One inline question:

We haven’t made the final decision on including BOLT in

LLVM_ALL_PROJECTS, but we are ready to make BOLT the default project in

the initial merge commit. Currently, we do not support building on

Windows; thus, the project will be automatically disabled with the

warning when building on unsupported platforms. As we anticipate the

Oh? Build or work? And why :slight_smile:

Thanks and good luck!

-eric

Sorry, I meant that for the initial merge commit, we could exclude BOLT

from LLVM_ALL_PROJECTS due to the same concerns the Flang team had at

the time of their merge. Namely, the lack of buildbots and Windows

support. The plan, of course, is to build and test BOLT by default, but

we have to figure out the buildbot situation first.

Thanks for your feedback.

Maksim

Making sure things are LLVM coding style, format, etc are pretty

important to me. flang was a useful pilot here in a lot of ways and some

of the requests for fixups still don’t really seem to have happened so

making sure that happens before is pretty important to me.

Following someone’s footsteps has its drawbacks too :slight_smile: Fair enough. We

continue to develop BOLT and have plans for more tight integration with

LLVM, such as sharing the common binary IR. Having a clean and

easy-to-read codebase will be highly beneficial to the community.

We haven’t made the final decision on including BOLT in

LLVM_ALL_PROJECTS, but we are ready to make BOLT the default project in

the initial merge commit. Currently, we do not support building on

Windows; thus, the project will be automatically disabled with the

warning when building on unsupported platforms. As we anticipate the

Oh? Build or work? And why :slight_smile:

The Windows build was disabled due to the usage of threads, IIRC.

Although, we don’t use them directly, only via the C++11 standard

library. I don’t have access to a windows box to test it myself. If the

build issues are resolved, I don’t see why BOLT can’t be used to

optimize ELF binaries on windows.

Thanks!

Maksim

Making sure things are LLVM coding style, format, etc are pretty

important to me. flang was a useful pilot here in a lot of ways and some

of the requests for fixups still don’t really seem to have happened so

making sure that happens before is pretty important to me.

Following someone’s footsteps has its drawbacks too :slight_smile: Fair enough. We

continue to develop BOLT and have plans for more tight integration with

LLVM, such as sharing the common binary IR. Having a clean and

easy-to-read codebase will be highly beneficial to the community.

Excellent. Looking forward to the code review!

We haven’t made the final decision on including BOLT in

LLVM_ALL_PROJECTS, but we are ready to make BOLT the default project in

the initial merge commit. Currently, we do not support building on

Windows; thus, the project will be automatically disabled with the

warning when building on unsupported platforms. As we anticipate the

Oh? Build or work? And why :slight_smile:

The Windows build was disabled due to the usage of threads, IIRC.

Although, we don’t use them directly, only via the C++11 standard

library. I don’t have access to a windows box to test it myself. If the

build issues are resolved, I don’t see why BOLT can’t be used to

optimize ELF binaries on windows.

Makes sense. Might be good to just disable threads on windows rather than disabling the build also.

Thanks for the response!

-eric

Making sure things are LLVM coding style, format, etc are pretty

important to me. flang was a useful pilot here in a lot of ways and some

of the requests for fixups still don’t really seem to have happened so

making sure that happens before is pretty important to me.

Following someone’s footsteps has its drawbacks too :slight_smile: Fair enough. We

continue to develop BOLT and have plans for more tight integration with

LLVM, such as sharing the common binary IR. Having a clean and

easy-to-read codebase will be highly beneficial to the community.

We haven’t made the final decision on including BOLT in

LLVM_ALL_PROJECTS, but we are ready to make BOLT the default project in

the initial merge commit. Currently, we do not support building on

Windows; thus, the project will be automatically disabled with the

warning when building on unsupported platforms. As we anticipate the

Oh? Build or work? And why :slight_smile:

The Windows build was disabled due to the usage of threads, IIRC.

Although, we don’t use them directly, only via the C++11 standard

library. I don’t have access to a windows box to test it myself.

Have you folks been trying to use the threading APIs exposed by libSupport?
They are supposed to work on Windows as well (at least I think lld makes use it these and works on Windows)

C++11 threads should be usable on Windows too, in general.

If building with GCC/libstdc++, there's two compiler configurations, the "win32 thread model" which doesn't provide the full C++11 thread support, and the "posix thread model" which is configured to run libgcc and libstdc++ on top of libwinpthread, which does support C++11 threads. I suspect this is what you've run into.

The mingw cross compilers in debian/ubuntu ship both configurations, but default to the "win32 thread model" without libwinpthread. There, you can choose the other one by invoking e.g. "x86_64-w64-mingw32-g++-posix", i.e. there are compiler frontends with -win32 and -posix suffixes in addition to the default.

I think the LLVM codebase also uses some bits of C++11 threads/mutexes somewhere, because I remember that cross compiling LLVM with the debian/ubuntu provided mingw compilers requires using the -posix suffixed tools. So with that in mind, this shouldn't be any additional limitation compared with what LLVM requires already.

Also, if building for Windows with a toolchain that uses libc++ (which is also usable in mingw configurations) or MSVC STL, then there's no ambiguity, C++11 threads are always available.

// Martin

Hi Maksim,

there are a couple of things to do from the infrastructure perspective. So I started a ticket with the (probably incomplete) list of things to do:
https://github.com/llvm/llvm-iwg/issues/80

Feel free to add more comments/tasks there.

Have you folks been trying to use the threading APIs exposed by

libSupport?

They are supposed to work on Windows as well (at least I think lld makes

use it these and works on Windows)

Makes sense. We will try to switch to synchronization primitives from

libSupport.

Thank you for the great suggestion,

Maksim

Thank you for the detailed background on threads on windows!

We will give it a shot to get BOLT available on the platform.

Maksim

Great! Thank you, Christian, for creating the tracking issue.

Maksim

Hi Maksim,

Hi folks,

https://github.com/facebookincubator/BOLT branch “main” contains a merge proposal of BOLT into llvm-project. This is llvm from Nov 30th with 1016 commits on top of it corresponding to the BOLT project.

These 1016 commits would ideally be committed in a merge commit, merging LLVM as the first parent and BOLT as the second, and would be there only for the purposes of preserving project history. In this way, they should be easily skippable during a bisect of LLVM in the same way as the merge commit of flang. These commits represent the linear history of BOLT on top of rebased LLVM, so most commits are not buildable (since we can’t build a very old version of BOLT on top of a recent LLVM base). That’s why this is for history/blame only.

We have addressed the issues in https://github.com/facebookincubator/BOLT/issues/248 and we are happy to continue working on any extra suggestions.

Would it be better if we put this branch as a PR into llvm-project as a way to make it easier for people to review it? I don’t think we can put this into phabricator. However, I guess github’s bot will probably auto-close the PR. Also feel free to open new issues against our facebookincubator/BOLT project as a way to review it.

Thanks

Your approach sounds reasonable to me and looks good from a glance. One thought to share though.

My read: You take some point in LLVM’s history, then apply a commit which introduces a bolt directory, then have a few thousand bolt commits, then land a merge. Correct?

If so, that means if you check out one of those historic commits, you have an llvm-project directory with BOLT at some commit, and LLVM subprojects always with the same ‘root’ commit.

Presumably that ‘root’ commit does not work with any commit of BOLT, and anyone wanting to experiment with old commits (if this is a legitimate use case) would find that those things are unlikely to work except for maybe recent commits.

Have you considered instead of rebasing onto a recent LLVM commit, taking an empty root? That way, when a user checks out a historic commit, now they will only get a bolt directory, and they can supply their own folders for other llvm subprojects from an appropriate point in time without them conflicting on the filesystem. Additionally, there is no implication for a user to think that those directories at those historic checkouts are meaningfully related to the BOLT commit.

I mention it because that’s what we did for the flang merge in case the same effect is appropriate for you here.

Regards,

Peter

Peter, thanks for your input, that’s an excellent suggestion. Let me try playing with that, I’ll try to change our history to use an empty root as you suggested.

Best,

Rafael

I updated our github repo https://github.com/facebookincubator/BOLT branch “main” with the strategy that Peter mentioned. I put a merge commit with all BOLT commits as parent #2, and these commits have an empty root. Because of that, the merge commit itself does not modify LLVM in any way other than introducing a new bolt folder with all project files and their histories. Then I have a small commit on top of that do change LLVM’s CMakeList.txt to add bolt as a project.

At the moment, I believe the easiest way to review the code remains the one we have been using during these past months/year: checking our “main” branch and opening concerns in our github issues page. In case there are no further suggestions to BOLT, we are targeting pushing the merge commit by the end of next week.