[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

16

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

95

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

97

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?

109

The same is here.

[Amir]

zorg/buildbot/builders/BOLTBuilder.py

16

Thank you for this suggestion. Fixed separately in 66c07102840a2078de282d11642d10a9ae5ee9c1

[Amir]

Use LitTestCommand

[Amir]

zorg/buildbot/builders/BOLTBuilder.py

97

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.

109

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

[gkistanova]

LGTM.

.gitignore

 *~
 *.pyc
+*.swp
+*.orig
+twistd.log*
 lnt/tests/*/Output
 lnt/tests/*/*/Output
 lnt/docs/_build
 lnt/build
 lnt/dist
 lnt/LNT.egg-info
 llvmlab/build
 llvmlab/dist
 llvmlab/docs/_build
 llvmlab/llvmlab.egg-info
 # Visual Studio Code IDE configuration
 .vscode
 No newline at end of file

buildbot/osuosl/master/config/builders.py

'factory': UnifiedTreeBuilder.getCmakeWithNinjaBuildFactory(
         extra_configure_args=[
              "-DCMAKE_EXPORT_COMPILE_COMMANDS:BOOL=ON",
              "-DLLVM_ENABLE_ASSERTIONS:BOOL=ON",
              "-DLLVM_TOOL_CLANG_TOOLS_EXTRA_BUILD=0",
              "-DLLVM_ENABLE_LIBPFM=OFF",
              "-DLLVM_TARGETS_TO_BUILD=X86",
              "-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=ARC",
          ])},
+]
 
-
+all = [
     # 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,

buildbot/osuosl/master/config/release_builders.py

 from zorg.buildbot.builders import AnnotatedBuilder
 from zorg.buildbot.builders import LLDPerformanceTestsuite
 from zorg.buildbot.builders import FuchsiaBuilder
 from zorg.buildbot.builders import XToolchainBuilder
 
 
 # Release builders.
 
-all = [
+all = []
 
+'''
 # Clang builders.
 
     {'name' : "llvm-clang-x86_64-win-release",
     'tags'  : ["clang", "release"],
     'workernames' : ["as-builder-3"],
     'builddir': "llvm-clang-x86_64-win-rel",
     'factory' : UnifiedTreeBuilder.getCmakeWithNinjaWithMSVCBuildFactory(
                     vs="autodetect",
'factory' : OpenMPBuilder.getOpenMPCMakeBuildFactory(
                         '-DLLVM_CCACHE_BUILD=ON',
                     ],
                     env={
                         'PATH':'/home/llvmbb/bin/clang-latest/bin:/home/llvmbb/bin:/usr/local/bin:/usr/local/bin:/usr/bin:/bin',
                         'CC': 'clang', 'CXX': 'clang++',
                     })},
 
 ]
+'''

buildbot/osuosl/master/config/status.py

 from buildbot.process.properties import Interpolate
 from buildbot.plugins import reporters
 
 import config
 from zorg.buildbot.util.InformativeMailNotifier import LLVMInformativeMailNotifier
 
 # Should be a single e-mail address
 status_email = str(config.options.get('Master Options', 'status_email')).split(',')
 
-all = [
+all = []
+'''
 
     # Report github status for all the release builders,
     # i.e. those with the "release" tag.
     reporters.GitHubStatusPush(
         str(config.options.get('GitHub Status', 'token')),
         context = Interpolate("%(prop:buildername)s"),
         verbose = True, # TODO: Turn off the verbosity once this is working reliably.
         builders = [
reporters.MailNotifier(
         fromaddr = "[e-mail]",
         sendToInterestedUsers = False,
         extraRecipients = ["[e-mail]"],
         subject = "Build %(builder)s Failure",
         mode = "failing",
         builders = ["standalone-build-x86_64"]),
 
 ]
+'''
 
 
 from twisted.python import log
 
 # Returns a list of Status Targets. The results of each build will be
 # pushed to these targets. buildbot.plugins reporters has a variety
 # to choose from, including email senders, and IRC bots.
 def getReporters():
     if config.options.getboolean('Master Options', 'is_production'):
         log.msg(">>> getReporters: Production mode. All reporters are registered.")
         return all
     else:
         # Staging buildbot does not report issues.
         log.msg(">>> getReporters: Staging buildbot does not report issues.")
         return []

buildbot/osuosl/master/config/workers.py

 # TODO: Rename workers with "slave" as a part of the name.
 # TODO: Refactor to define a list of tuples, each of which describes a worker, then enumerate and create as data driven.
 
 from buildbot.plugins import worker
 import config
 
 def create_worker(name, *args, **kwargs):
     password = config.options.get('Worker Passwords', name)
     return worker.Worker(name, password=password, *args, **kwargs)
 
+'''
 def get_all():
     return [
         # FreeBSD
         create_worker("as-worker-4", properties={'jobs' : 24}, max_builds=2),
 
         # Linux Ubuntu
         create_worker("as-worker-5", properties={'jobs' : 16}),
 
return [
         create_worker("bolt-worker", max_builds=1),
         create_worker("bolt-worker-aarch64", max_builds=1),
 
         # Fedora worker
         create_worker("standalone-build-x86_64", max_builds=1),
 
         # CSKY T-HEAD workers
         create_worker("thead-clang-csky", properties={'jobs' : 32}, max_builds=1),
+'''
+def get_all():
+    return [
+        # BOLT worker
+        create_worker("bolt-worker", properties={'jobs' : 16, 'loadaverage': 16}, max_builds=1),
         ]

buildbot/osuosl/master/master.cfg

 from buildbot.plugins import changes
 
 c['change_source'] = []
 # TODO: Do we want to support a push mode for the changes? Probably not.
 #c['change_source'].append(PBChangeSource())
 c['change_source'].append(
     LLVMPoller(repourl='https://github.com/llvm/llvm-project.git',
                workdir='gitpoller-workdir',
-               pollinterval=120))
+               pollinterval=60*5))
 c['change_source'].append(
     changes.GitPoller(repourl='https://github.com/llvm/llvm-lnt.git',
                       project='lnt',
                       branch='master',
                       workdir='gitpoller-llvm-lnt',
                       pollinterval=60*60))
 
 c['collapseRequests'] = buildrequest.collapseRequests

 
 ####### BUILDBOT SERVICES
 
 # 'services' is a list of BuildbotService items like reporter targets. The
 # status of each build will be pushed to these targets. buildbot/reporters/*.py
 # has a variety to choose from, like IRC bots.
 
 # Configure MailNotifier, IRC, buildbot.reporters.github.GitHubStatusPush, buildbot.reporters.github.GitHubCommentPush
-c['services'] = config.status.getReporters()
+#c['services'] = config.status.getReporters()
 
 ####### PROJECT IDENTITY
 
 c['title'] = config.options.get('Buildbot', 'title')
 c['titleURL'] = config.options.get('Buildbot', 'title_url')
 c['buildbotURL'] = config.options.get('Buildbot', 'my_url')
 
 # minimalistic config to activate new web UI
 c['www'] = dict(port=8011,
                 plugins=dict(waterfall_view={}, console_view={}, grid_view={}), # TODO: badges
                 default_page='console',
-                auth=config.auth.getAuth(),
-                authz=config.auth.getAuthz(),
+                #auth=config.auth.getAuth(),
+                #authz=config.auth.getAuthz(),
                 #logRotateLength=
                 #maxRotatedFiles=
                 #versions=
             )
 
 ####### DB URL
 
 c['db'] = {

zorg/buildbot/builders/BOLTBuilder.py

 from buildbot.plugins import steps
+from buildbot.steps.shell import ShellCommand
 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 = ['bolt', 'llvm'],
            **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,
+                env=env),
+
+            ShellCommand(
+                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=('Run check-bolt in NFC mode'),
+                warnOnFailure=True,
+                haltOnFailure=False,
+                env=env),
+
+            ShellCommand(
+                name='nfc-check-large-bolt',
+                command=['bin/llvm-lit', '-sv', '-j2',
+                'tools/bolttests'],
+                description=('Run check-large-bolt in NFC mode'),
+                warnOnFailure=True,
+                haltOnFailure=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())

zorg/buildbot/commands/AnnotatedCommand.py

class AnnotatedBuildStep(buildstep.BuildStep):
     @defer.inlineCallbacks
     def finishStep(self):
         hidden = False
         debuglog("AnnotatedBuildStep: finish step '{}': stepid={}, buildid={}, results={}".format(
                     self.name, self.stepid, self.build.buildid, self.results))
         yield self.master.data.updates.finishStep(self.stepid, self.results,
                                                   hidden)
         # finish unfinished logs
-        yield self.finishUnfinishedLogs()
+        yield self._cleanup_logs()
 
     @defer.inlineCallbacks
     def startStep(self, remote, done=False):
         debuglog("AnnotatedBuildStep::startStep() starting '{}': "
                  "buildid={}, done={}".format(
                  self.name, self.build.buildid, done))
 
         try:
             yield self.addStep()
 
             try:
-                self.realUpdateSummary()
+                self.updateSummary()
                 # The "main" step has finished already.
                 # Do only necessary work.
                 #NOTE: we can get 'request finish' flag before the step started.
                 self._request_finish = self._request_finish or done
                 self._running = True
                 self.results = yield self.run()
             finally:
                 self._running = False
def startStep(self, remote, done=False):
             self.results = results.EXCEPTION
             why = failure.Failure()
             logging.err(why, "AnnotatedBuildStep.failed; traceback follows")
             yield self.addLogWithFailure(why)
 
         finally:
             # update the summary one last time, make sure that completes,
             # and then don't update it any more.
-            self.realUpdateSummary()
-            yield self.realUpdateSummary.stop()
+            self.updateSummary()
+            yield self.updateSummary.stop()
 
             # Update step status in the database.
             yield self.finishStep()
 
         debuglog("AnnotatedBuildStep::startStep() completed '{}': "
                  "stepid={}, buildid={}, results={}".format(
                  self.name, self.stepid, self.build.buildid, self.results))
 

zorg/buildbot/commands/LitTestCommand.py

resultNames = {'FAIL':'unexpected failures',
                  'TIMEOUT':'timeout waiting for results',
                  'NOEXE':'test executable is missing'}
 
   def __init__(self, ignore=[], flaky=[], max_logs=20, parseSummaryOnly=False,
                *args, **kwargs):
     super().__init__(*args, **kwargs)
     self.maxLogs = int(max_logs)
     self.logObserver = LitLogObserver(self.maxLogs, parseSummaryOnly)
-    self.addFactoryArguments(max_logs=max_logs)
-    self.addFactoryArguments(parseSummaryOnly=parseSummaryOnly)
+    #self.addFactoryArguments(max_logs=max_logs)
+    #self.addFactoryArguments(parseSummaryOnly=parseSummaryOnly)
     self.addLogObserver('stdio', self.logObserver)
 
   def evaluateCommand(self, cmd):
     # Always report failure if the command itself failed.
     if cmd.rc != 0:
       return FAILURE
 
     # Otherwise, report failure if there were failures in the log.

zorg/buildbot/util/phasedbuilderutils.py

 
 class NamedTrigger(Trigger):
     """Trigger subclass which allows overriding the trigger name, and also
     allows attaching a log to link to the triggered builds."""
     def __init__(self, name, triggeredBuilders = [], **kwargs):
         Trigger.__init__(self, **kwargs)
         self.name = name
         self.triggeredBuilders = triggeredBuilders
-        self.addFactoryArguments(name = name,
-                                 triggeredBuilders = triggeredBuilders)
+        #self.addFactoryArguments(name = name,
+        #                         triggeredBuilders = triggeredBuilders)
     def start(self):
         # Add a log linking to the triggered builders, if supplied.
         if self.triggeredBuilders:
             logText = StringIO.StringIO()
             for builder in self.triggeredBuilders:
                 print >>logText, ('<b><a href="../../../../../../%s">%s'
                                   '</a></b><br>' % (builder, builder))
             self.addHTMLLog('triggered builds', str(logText.getvalue()))