Out-of-tree flang builds

Out-of-tree builds are a way to build only the flang code using a pre-existing full build of llvm. Out-of-tree flang builds are four times faster than full builds. Thus, for those of us actively developing code and reviewing other people’s changes, out-of-tree builds significantly improve our productivity.

Unfortunately, out-of-tree builds are not currently supported by the llvm buildbots. Thus, people changing build files must test this feature themselves. The process of doing an out-of-tree build is described in the flang overview document – https://github.com/llvm/llvm-project/tree/master/flang. Alternatively, people changing build files could ask someone familiar with out-of-tree builds to review their changes.

A recent update from David Truby broke out-of-tree builds – https://reviews.llvm.org/D84022. In the conversation in the Phabricator review, David states that he understands that “out-of-tree builds are considered a “best effort” feature that isn’t guaranteed to work”. But since out-of-tree builds are critical for my development, I don’t consider them optional.

I’m writing this note to bring this issue to the broader flang community. I consider out-of-tree builds to be a critical feature that no change should break. What do you think?

Support for out-of-tree builds, like BUILD_SHARED_LIBS or gn builds, are helpful for developers, but not relevant for release builds. Requiring to maintain all combinations of build configurations is not feasible before every commit, this is why they are only best effort. When you notice a out-of-tree build breaking, you are free to fix the problem (and assuming the fix is straightforward, low-risk, without review).

However, I am wondering, why isn’t an iterative build sufficient? Assuming you are not touching any non-flang files, make/ninja should not try to rebuild them.

Michael

Out-of-tree builds are useful when you’re working on more than one thing at once (including trying out other people’s changes). You don’t need multiple builds of llvm and mlir, just of flang.

Iterative builds are very useful and work well.

But when I’m reviewing a change from someone else or starting on a new change for myself, I build all of flang. In the last six hours, I’ve done four non-iterative builds, for example.

Pete

Should by “incremental build”, not “iterative build”

Michael

ccache can help as well, even for a standalone build in the non-iterative (it won’t provide as much benefits as the standalone build of course, I’m not saying this can replace them).

+1 on Pete’s and Tim’s reply. I am often building multiple branches at the same time.

Support for out-of-tree builds, like BUILD_SHARED_LIBS or gn builds, are helpful for developers, but not relevant for release builds.

I disagree. Out-of-tree builds are also very useful for downstream distributors when packaging individual components of the LLVM project.

Isuru

Hi all,

I agree with Michael here: having every possible build configuration fixed in every commit is an impossible task. I submitted the patch in question because earlier builds broke my build configuration (and any other builds with multi-config generators, e.g. msbuild), and I believe submitting such a patch to fix these configurations was reasonable, just as submitting patches to fix out-of-tree builds is reasonable.

I do not believe that this patch should have been reverted, as such a revert seems to suggest that one (non-standard?) build configuration (out of tree builds) is more important than another (non-standard?) build configuration (multi-config builds). The patch in question didn’t fail any build bots and doesn’t (I believe) fall short of the LLVM Developer Guidelines guidance on quality of patches. I believe a better course of action here would have been a patch from someone that requires out-of-tree builds to work, and I submitted some suggestions on places to look to develop an easy fix.

In response to out of tree builds being a critical feature that no patch should break: I think in general it is unreasonable to state that any feature that is not defended by buildbots is a critical feature that no patch should break. This puts a really unreasonable expectation on the developer of every patch to manually test with every possible configuration that some people might care about, rather than using the automated structure we already have to detect regressions.

Thanks

David Truby

Sorry that this has been disruptive to people's workflows. When I was approving that patch I hoped that there was enough time for people to review, test or raise further objections.

Clearly some people rely on of out-of-tree builds while others require support for multi-config generators (IIUC, this is a prerequisite for Windows support). We should support both.

I also feel that out-of-tree builds are rather non-standard and rarely tested by developers (I never use it and AFAIK nobody on our team does). It sounds like Flang could benefit from a public build-bot that prevents this capability from future breakage.

In the meantime, I uploaded David's patch with the suggested fixes for out of tree builds here: ⚙ D85078 [flang] Fix multi-config generator builds. I'm not a CMake expert, but both out-of-tree and multiconfig-generator builds worked on my machine. HTH

-Andrzej

Hi all, I meant to send this mail yesterday morning and instead managed to
direct reply to a limited number of people instead of the list. Sorry about that!

Just to clarify, I’m not in any way opposed to out-of-tree builds and
if people rely on them then obviously we should accept patches to keep
them working. What I am opposed to here is the idea that everyone must
test every patch both in and out of tree or have their patch reverted,
especially when out of tree configurations are not being built by any
buildbots.

To take this particular case as an example, I posted a patch fixing a
build configuration that was broken previously (note: I did not revert
the patch that broke my build configuration, I simply posted a new
patch to fix it). This patch underwent a review for 2 weeks before
being approved and passed all the buildbots we have and as such I
believe did not violate the criteria for a patch to be reverted
according to the LLVM Developer Policy:
https://llvm.org/docs/DeveloperPolicy.html#quality.

I did however attempt to address the issue by providing pointers to
what a quick fix might look like (such a fix has now kindly been
applied by Andrzej Warzynski in https://reviews.llvm.org/D85078).
However the patch was reverted rather than an attempt at a fix being
made, which I believe would have been by far the more preferable option
(such a fix could even have been committed without review given how
simple it was).

Obviously the revert broke my build configuration again, so the revert
somewhat makes it feel that a decision was made off-list to consider
one build configuration as more important than another. I think if we
want out-of-tree builds to be considered more important than multi-
config builds then that should be a decision made by the community not
unilaterally. I also think if that decision is made then it must be
defended by buildbots; if my patch had broken a buildbot I would have
reverted it myself!

Thanks
David Truby

Hi All,

I also made the same mistake with this mail when replying to Pete.
Again, apologies about that! I didn’t mean to take the conversation
privately off-list, I think it should be had publicly.

Hi, David,

I realize that this is frustrating, but in general, we do realize that buildbots don’t cover everything and we do revert actively even if the breakage is not observable on the buildbots. It is incumbent that every reasonable attempt is made by the reporter to provide a reproducer for the problem, and failing that, to proactively test future versions. Otherwise, forward progress will stall. It seems as though, in this case, we have a reproducer that can be widely tested.

I would not view this as treating one configuration as more important than another. It looks like our general practice of trying not to allow the fixing of one thing to break some currently-working thing.

We really do need to have a buildbot that tests out of tree builds. I was just on a call yesterday within my organization where we were discussing how to setup more builders with more configurations. Nick, we should make note of this in particular.

Regardless, if it is our consensus that out-of-tree builds are supported, and it seems like it is, then it is everyone’s responsibility to test sufficiently and make sure that they work. Reverting as needed.

-Hal

Flang out-of-tree builds were discussed in today’s flang technical call.

In case it was not clear from the discussion, flang out-of-tree builds have been supported since the project was originally started. The functionality was upstreamed into llvm-project along with the instructions, which are still prominently featured in llvm-project on the flang the project page [1].

  • Steve

[1] https://github.com/llvm/llvm-project/tree/master/flang#building-flang-out-of-tree

Noted your request Hal

For the record, I’ll be working on an AArch64 buildbot for the out-of-tree build. I don’t have an ETA for this since I’m also working on other tasks, but if anyone else is fiddling with zorg we should probably coordinate :slight_smile: