[buildbot] Fix worker for ThinLTO whole program devirtualization

[tejohnson]

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?

[tejohnson]

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

[tejohnson]

In D91474#2405842, @tejohnson wrote:

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]

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.

[tejohnson]

Implement suggestion

[tejohnson]

In D91474#2441799, @gkistanova wrote:

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.

[llvm-commits]

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.

• msg-17060-574.txt163 BDownload

[tejohnson]

In D91474#2444087, @llvm-commits wrote:

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?

buildbot/osuosl/master/config/builders.py

# LTO and ThinLTO builders.
 
     {'name' : "clang-with-thin-lto-wpd-ubuntu",
     'tags'  : ["clang","lld","LTO"],
     'workernames' : ["thinlto-x86-64-bot1"],
     'builddir': "clang-with-thin-lto-wpd-ubuntu",
     'factory' : ClangLTOBuilder.getClangWithLTOBuildFactory(
                     jobs=72,
                     lto='thin',
-                    extra_configure_args=[
+                    extra_configure_args_lto_stage=[
                         '-DLLVM_CCACHE_BUILD=ON',
-                        '-DCMAKE_CXX_FLAGS="-O3 -Xclang -fwhole-program-vtables -fno-split-lto-unit"',
-                        '-DCMAKE_C_FLAGS="-O3 -Xclang -fwhole-program-vtables -fno-split-lto-unit"',
-                        '-DCMAKE_EXE_LINKER_FLAGS="-Wl,--lto-whole-program-visibility"'])},
+                        '-DCMAKE_CXX_FLAGS=-O3 -Xclang -fwhole-program-vtables -fno-split-lto-unit',
+                        '-DCMAKE_C_FLAGS=-O3 -Xclang -fwhole-program-vtables -fno-split-lto-unit',
+                        '-DCMAKE_EXE_LINKER_FLAGS=-Wl,--lto-whole-program-visibility'])},
 
     {'name' : "clang-with-lto-ubuntu",
     'tags'  : ["clang","lld","LTO"],
     'workernames' : ["as-worker-91"],
     'builddir': "clang-with-lto-ubuntu",
     'factory' : ClangLTOBuilder.getClangWithLTOBuildFactory(jobs=72)},
 
 # Builders for MLIR.

zorg/buildbot/builders/ClangLTOBuilder.py

f.addStep(NinjaCommand(name="install-stage%s-compiler"% stage_num,
                            ))
 
 
 def getClangWithLTOBuildFactory(
            depends_on_projects = None,
            clean = False,
            jobs  = None,
            extra_configure_args = None,
+           extra_configure_args_lto_stage = None,
            compare_last_2_stages = True,
            lto = None, # The string gets passed to -flto flag as is. Like -flto=thin.
            env = None,
            **kwargs):
 
     # Set defaults
     if depends_on_projects:
         depends_on_projects = list(depends_on_projects)
     else:
         # By default we link with LLD.
         depends_on_projects = ['llvm', 'clang', 'lld']
 
     if lto is None:
         lto = 'ON'
 
     if extra_configure_args is None:
         extra_configure_args = []
     else:
         extra_configure_args = list(extra_configure_args)
 
+    if extra_configure_args_lto_stage is None:
+        extra_configure_args_lto_stage = []
+    else:
+        extra_configure_args_lto_stage = list(extra_configure_args_lto_stage)
+
     # Make sure CMAKE_INSTALL_PREFIX and -G are not specified
     # in the extra_configure_args. We set them internally as needed.
     # TODO: assert extra_configure_args.
-    install_prefix_specified = any(a.startswith('-DCMAKE_INSTALL_PREFIX=') for a in extra_configure_args)
+    install_prefix_specified = (any(a.startswith('-DCMAKE_INSTALL_PREFIX=') for a in extra_configure_args) or
+                                any(a.startswith('-DCMAKE_INSTALL_PREFIX=') for a in extra_configure_args_lto_stage))
     assert not install_prefix_specified, "Please do not explicitly specify the install prefix for multi-stage build."
 
     # Prepare environmental variables. Set here all env we want everywhere.
     merged_env = {
         'TERM' : 'dumb' # Be cautious and disable color output from all tools.
     }
     if env is not None:
         # Overwrite pre-set items with the given ones, so user can set anything.
if 'lld' in depends_on_projects:
         CmakeCommand.applyRequiredOptions(extra_configure_args, [
             ('-DLLVM_ENABLE_LLD=', 'ON'),
             ])
 
     # The rest are test stages, which depend on the staged compiler we are ultimately after.
     s = f.staged_compiler_idx + 1
     staged_install = f.stage_installdirs[f.staged_compiler_idx]
     for i in range(s, len(f.stage_objdirs[s:]) + s):
-        configure_args = extra_configure_args[:]
+      configure_args = extra_configure_args[:] + extra_configure_args_lto_stage[:]
 
         configure_args.append(
             WithProperties(
                 "-DCMAKE_AR=%(builddir)s/" + staged_install + "/bin/llvm-ar"
             ))
         configure_args.append(
             WithProperties(
                 "-DCMAKE_RANLIB=%(builddir)s/" + staged_install + "/bin/llvm-ranlib"