Page MenuHomePhabricator

Add flang out of tree buildbot
ClosedPublic

Authored by rovka on Sep 3 2020, 5:13 AM.

Details

Summary

This builder has 2 stages: one for building llvm and mlir, and one for
building flang itself in a different build directory. In order to do an
out of tree build of flang, we need to run cmake with the path to the
flang subproject as source directory.

Diff Detail

Event Timeline

rovka created this revision.Sep 3 2020, 5:13 AM
rovka requested review of this revision.Sep 3 2020, 5:13 AM
gkistanova requested changes to this revision.Sep 8 2020, 3:39 PM

Thanks for working on this, Diana!

It does not seem getFlangOutOfTreeBuildFactory belongs to the UnifiedTreeBuilder.

You either need to teach cmake to configure flang from the root of the unified tree; then you likely would not need a custom build factory, as the current UnifiedTreeBuilder would build your configuration just fine.
Or you need to add a custom builder for flang.

I personally prefer the first option, since it would simplify the build of this configuration in general for everyone, not just for CI.

This revision now requires changes to proceed.Sep 8 2020, 3:39 PM
rovka updated this revision to Diff 291509.Sep 14 2020, 1:36 AM
rovka edited the summary of this revision. (Show Details)

Hi Galina, thanks for having a look!

I think running cmake from the top level directory would defeat the purpose of what this buildbot is trying to test. The whole point is to be able to build flang using only the flang source tree. I have therefore updated the patch to create a new FlangBuilder, but it still imports some things from the UnifiedTreeBuilder. We could also move those common functions from UnifiedTreeBuilder to Util.py if you think that would be cleaner. Please let me know your thoughts.

Cheers!

Thanks for changing, Diana!

Almost there. Please see my inline comments.

zorg/buildbot/builders/FlangBuilder.py
17

UnifiedTreeBuilder would do this for you. You can just pass the llvm_extra_configure_args down to getCmakeWithNinjaBuildFactory as is.

45

Just a cosmetic.
How about renaming this to something like flang_obj_dir and flang_src_dir respectively?
Otherwise it might look a bit confusing with f.obj_dir etc.

53

The naming of pathRelativeToBuild method is not great. I refactored it to be pathRelativeTo instead.

Could you rebase the patch, please?
Otherwise it looks confusing, when the comment says we want paths relative to the source and then there are pathRelativeToBuild calls.

rovka updated this revision to Diff 294022.Thu, Sep 24, 5:11 AM

Hi Galina, those are all good comments, thanks! I think I fixed them up, does it look ok now?

rovka marked 3 inline comments as done.Thu, Sep 24, 5:11 AM
gkistanova accepted this revision.Sun, Sep 27, 11:55 PM

Thanks, Diana!
Looks good now.

This revision is now accepted and ready to land.Sun, Sep 27, 11:55 PM
This revision was automatically updated to reflect the committed changes.

Does this bot trigger for Flang project as of now?
I might not be very much familiar with phabricator but not sure how do we see if the bot triggers and the build results from the bots.
I was expecting it to trigger for https://reviews.llvm.org/D87774 review.

Also in phabricator when I click on buildbots in home screen of phabricator I get redirected to http://lab.llvm.org:8011/
but the page never opens.

Hi Sameeran,

The bot is currently on the silent master [1], so it doesn't trigger.
It had some strange python exceptions that I hadn't seen on my
buildmaster during testing. I think they might be gone now, but I'll
leave it on silent for a couple more days until it looks stable
enough.

Cheers,
Diana

[1] http://lab.llvm.org:8014/#/builders/114