Page MenuHomePhabricator

[buildbot] Fix worker for ThinLTO whole program devirtualization
ClosedPublic

Authored by tejohnson on Nov 13 2020, 8:04 PM.

Details

Summary

This fixes a syntax error in the extra_configure_args provided to
getClangWithLTOBuildFactory, we shouldn't be surrounding the options
with double quotes.

Once that is fixed, I realized from testing the failing command locally
that it will still fail, because the clang-specific extra_configure_args
are also used by the initial stage1 build with the system compiler,
which is gcc. We only want these for the last stage, where LTO is
performed. Added a new parameter to getClangWithLTOBuildFactory,
extra_configure_args_lto_stage, to pass the options only to that LTO
stage.

Is there a way to test my change to ClangLTOBuilder.py?

Diff Detail

Event Timeline

tejohnson created this revision.Nov 13 2020, 8:04 PM
tejohnson requested review of this revision.Nov 13 2020, 8:04 PM

Ping - @gkistanova can you take a look and also let me know if there is a way to test the LTO builder change?

Ping - @gkistanova can you take a look and also let me know if there is a way to test the LTO builder change?

ping

gkistanova accepted this revision.Dec 8 2020, 9:18 PM

Hi Teresa,

LGTM with a nit pick. Please see my comment inline.

if there is a way to test the LTO builder change?

Once the patch is ready we will apply it to the staging. Then you could test it by moving your bot there.

zorg/buildbot/builders/ClangLTOBuilder.py
265–266

Could you make this a multi-line statement, please? It is quite long.

This revision is now accepted and ready to land.Dec 8 2020, 9:18 PM
tejohnson updated this revision to Diff 310547.Dec 9 2020, 8:35 AM

Implement suggestion

Hi Teresa,

LGTM with a nit pick. Please see my comment inline.

if there is a way to test the LTO builder change?

Once the patch is ready we will apply it to the staging. Then you could test it by moving your bot there.

Ok thanks. I have committed. Please point me at what I need to do to switch to the staging.

Point your worker to the port 9994, instead of 9990. Everything is the same
as for the production bot.

This is described in https://llvm.org/docs/HowToAddABuilder.html but I
guess that's buried in other details.

Point your worker to the port 9994, instead of 9990. Everything is the same
as for the production bot.

This is described in https://llvm.org/docs/HowToAddABuilder.html but I
guess that's buried in other details.

Ok great, I'm already using port 9994 in any case since this is experimental. It looks like the changes haven't taken effect yet - does something need to happen to apply these changes once I've committed them? How will I know when they are live?