[BOLT] Fold bolt-nfc builder into BOLTBuilder

[Amir]

Summary

With ShellCommand, it's possible to fully unify all BOLT builders. Fold bolt-nfc
annotated builder into BOLTBuilder.

[Amir]

Update

[Amir]

Fix ShellCommand

[Amir]

Remove unrelated changes

[Amir]

Don't flunk on failure in nfc checks

[gkistanova]

Hi Amir,

The patch looks good in general, with a few things to address.
Please see my comments inline.

zorg/buildbot/builders/BOLTBuilder.py

17

This is not a part of this patch, but anyway. This is a bad idea to have a mutable object as a default param value. You may want to set it to None and then assign the list in the body. https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments

96

You may want to use LitTestCommand here to have the test results parsed.

98

You have hardcoded the number of jobs for LIT. And this number is different than the one in the next step. Could you explain why this is a good idea, please?

Shouldn't the number of jobs be specific to a particular worker, and not a build factory?

110

The same is here.

[Amir]

zorg/buildbot/builders/BOLTBuilder.py

17

Thank you for this suggestion. Fixed separately in 66c07102840a2078de282d11642d10a9ae5ee9c1

[Amir]

Use LitTestCommand

[Amir]

zorg/buildbot/builders/BOLTBuilder.py

98

The reason for that is BOLT runs some passes in parallel internally using all available cores, but only during certain passes. With smaller test binaries we can overlap some number of executions, with 4 selected as a reasonable number. Using num-jobs is not reasonable due to this internal parallelism.

110

The reason for smaller number here is that check-large-bolt binaries are larger and we can overlap smaller number of executions.

[gkistanova]

LGTM.

buildbot/osuosl/master/config/builders.py

# Builders for ML-driven compiler optimizations.
 
 
     # BOLT builders managed by Meta
     {'name' : 'bolt-x86_64-ubuntu-nfc',
     'tags'  : ["bolt"],
     'collapseRequests': False,
     'workernames' : ['bolt-worker'],
     'builddir': "bolt-x86_64-ubuntu-nfc",
-    'factory' : AnnotatedBuilder.getAnnotatedBuildFactory(
-                    script="bolt-nfc.py",
-                    depends_on_projects=['llvm', 'bolt'],
-                    warnOnWarnings=True,
+    'factory' : BOLTBuilder.getBOLTCmakeBuildFactory(
+                    bolttests=True,
+                    extra_configure_args=[
+                        "-DLLVM_APPEND_VC_REV=OFF",
+                        "-DLLVM_CCACHE_BUILD=ON",
+                        "-DLLVM_ENABLE_PROJECTS=clang;lld;bolt",
+                        "-DLLVM_TARGETS_TO_BUILD=X86;AArch64",
+                        ],
+                    is_nfc=True,
                     )},
 
     {'name': "bolt-x86_64-ubuntu-clang-bolt-lto-pgo",
     'tags': ["bolt"],
     'workernames':["bolt-worker"],
     'builddir': "bolt-x86_64-ubuntu-clang-bolt-lto-pgo",
     'factory' : BOLTBuilder.getBOLTCmakeBuildFactory(
                     bolttests=False,

zorg/buildbot/builders/BOLTBuilder.py

 from buildbot.plugins import steps
+from buildbot.steps.shell import ShellCommand
+from zorg.buildbot.commands.LitTestCommand import LitTestCommand
 from zorg.buildbot.commands.CmakeCommand import CmakeCommand
 from zorg.buildbot.builders.UnifiedTreeBuilder import getLLVMBuildFactoryAndSourcecodeSteps, addCmakeSteps, addNinjaSteps
 from zorg.buildbot.process.factory import LLVMBuildFactory
 
 def getBOLTCmakeBuildFactory(
            clean = False,
            bolttests = False,
+           is_nfc = False,
            targets = None,
            checks = None,
            cache = None,
            extra_configure_args = None,
            env = None,
            depends_on_projects = None,
            **kwargs):
 
     if env is None:
         env = dict()
 
     bolttests_dir = "bolt-tests"
 
     cleanBuildRequested = lambda step: clean or step.build.getProperty("clean", default=step.build.getProperty("clean_obj"))
def getBOLTCmakeBuildFactory(
 
     addNinjaSteps(
         f,
         targets=targets,
         checks=checks,
         env=env,
         **kwargs)
 
+    if is_nfc:
+        f.addSteps([
+            ShellCommand(
+                name='nfc-check-setup',
+                command=[f"../{f.monorepo_dir}/bolt/utils/nfc-check-setup.py"],
+                description=('Setup NFC testing'),
+                warnOnFailure=True,
+                haltOnFailure=False,
+                flunkOnFailure=False,
+                env=env),
+            LitTestCommand(
+                name='nfc-check-bolt',
+                command=['bin/llvm-lit', '-sv', '-j4',
+                         # bolt-info will always mismatch in NFC mode
+                         '--xfail=bolt-info.test',
+                         # FIXME[aaupov]: https://github.com/llvm/llvm-project/issues/59008
+                         '--filter-out=X86/bb-with-two-tail-calls.s',
+                         'tools/bolt/test'],
+                description=["running", "NFC", "check-bolt"],
+                descriptionDone=["NFC", "check-bolt", "completed"],
+                warnOnFailure=True,
+                haltOnFailure=False,
+                flunkOnFailure=False,
+                env=env),
+            LitTestCommand(
+                name='nfc-check-large-bolt',
+                command=['bin/llvm-lit', '-sv', '-j2',
+                         'tools/bolttests'],
+                description=["running", "NFC", "check-large-bolt"],
+                descriptionDone=["NFC", "check-large-bolt", "completed"],
+                warnOnFailure=True,
+                haltOnFailure=False,
+                flunkOnFailure=False,
+                env=env),
+            ])
+
     return f

zorg/buildbot/builders/annotated/bolt-nfc.py

-#!/usr/bin/python3
-
-import os
-import subprocess
-import sys
-import traceback
-import util
-from contextlib import contextmanager
-
-def main():
-    source_dir = os.path.join('..', 'llvm-project')
-    tests_dir = os.path.join('..', 'bolt-tests')
-
-    with step('fetch large-bolt-tests'):
-        if os.path.exists(tests_dir):
-            run_command(['git', '-C', tests_dir, 'fetch', 'origin'])
-            run_command(['git', '-C', tests_dir, 'reset', '--hard',
-                         'origin/main'])
-        else:
-            run_command(['git', 'clone',
-                         'https://github.com/rafaelauler/bolt-tests',
-                         tests_dir])
-
-    with step('cmake'):
-        cmake_args = ['-GNinja',
-                      '-DCMAKE_BUILD_TYPE=Release',
-                      '-DLLVM_APPEND_VC_REV=OFF',
-                      '-DLLVM_CCACHE_BUILD=ON',
-                      '-DLLVM_ENABLE_ASSERTIONS=ON',
-                      '-DLLVM_ENABLE_LLD=ON',
-                      '-DLLVM_ENABLE_PROJECTS=clang;lld;bolt',
-                      '-DLLVM_TARGETS_TO_BUILD=X86;AArch64',
-                      '-DLLVM_EXTERNAL_PROJECTS=bolttests',
-                      '-DLLVM_EXTERNAL_BOLTTESTS_SOURCE_DIR='+tests_dir,
-                      ]
-
-        util.clean_dir('.')
-        run_command(['cmake', os.path.join(source_dir, 'llvm')] + cmake_args)
-
-    with step('build bolt'):
-        run_command(['ninja', 'bolt'])
-
-    with step('check-bolt'):
-        run_command(['ninja', 'check-bolt'])
-
-    with step('check-large-bolt'):
-        run_command(['ninja', 'check-large-bolt'])
-
-    with step('nfc-check-setup'):
-        run_command([os.path.join(source_dir, 'bolt', 'utils',
-            'nfc-check-setup.py')])
-
-    with step('nfc-check-bolt', warn_on_fail=True, halt_on_fail=False):
-        run_command([os.path.join('bin', 'llvm-lit'), '-sv', '-j2',
-            # bolt-info will always mismatch in NFC mode
-            '--xfail=bolt-info.test',
-            # FIXME[aaupov]: https://github.com/llvm/llvm-project/issues/59008
-            '--filter-out=X86/bb-with-two-tail-calls.s',
-            'tools/bolt/test'])
-
-    with step('nfc-check-large-bolt', warn_on_fail=True, halt_on_fail=False):
-        run_command([os.path.join('bin', 'llvm-lit'), '-sv', '-j2',
-            'tools/bolttests'])
-
-
-@contextmanager
-def step(step_name, warn_on_fail=False, halt_on_fail=True):
-    util.report('@@@BUILD_STEP {}@@@'.format(step_name))
-    if halt_on_fail:
-        util.report('@@@HALT_ON_FAILURE@@@')
-    try:
-        yield
-    except Exception as e:
-        if isinstance(e, subprocess.CalledProcessError):
-            util.report(
-                '{} exited with return code {}.'.format(e.cmd, e.returncode)
-            )
-        util.report('The build step threw an exception...')
-        traceback.print_exc()
-
-        if warn_on_fail:
-            util.report('@@@STEP_WARNINGS@@@')
-        else:
-            util.report('@@@STEP_FAILURE@@@')
-    finally:
-        sys.stdout.flush()
-
-
-def run_command(cmd, directory='.'):
-    util.report_run_cmd(cmd, cwd=directory)
-
-
-if __name__ == '__main__':
-    sys.path.append(os.path.dirname(__file__))
-    sys.exit(main())