[zorg] Refactor Libiomp5Builder to OpenMPBuilder

[Hahnfeld]

Summary

Background: We also want to test libomptarget which is the runtime
library for OpenMP offloading and part of the OpenMP repository.

This patch does the following:

• Rename builders from libomp-* to openmp-*.
• Rename Libiomp5Builder to OpenMPBuilder and the function getLibompCMakeBuildFactory to getOpenMPCMakeBuildFactory.
• Cleanup, properly indent function and add support for building and testing libomptarget.
• Build fresh Clang on openmp-clang-ppc64le-linux-debian so that the libomptarget tests have a chance of passing.

[Hahnfeld]

Ping

[gkistanova]

Hi Jonas,

The patch looks Ok, with a small issue to address before committing.

buildbot/osuosl/master/config/builders.py

1274

This change is not relevant to this patch, is it?

zorg/buildbot/builders/OpenMPBuilder.py

19

Please do not use mutable default arguments. The usual pattern to default to None and then handle it accordingly.
See http://docs.python-guide.org/en/latest/writing/gotchas/#mutable-default-arguments for more details.

[Hahnfeld]

Avoid mutable default argument.

buildbot/osuosl/master/config/builders.py

1274

I think my editor removed a trailing whitespace. I can put it back in if you prefer...

zorg/buildbot/builders/OpenMPBuilder.py

19

I think it would be fine here because we don't change env but merge it into merged_env. Nevertheless I agree that we should avoid problems even before they can happen.

I copied this one from getLLVMCMakeFactory, so I'll change that in a follow-up commit if you agree.

[gkistanova]

Let's keep only the changes relevant to this patch. Feel free to make a separate cosmetic NFC patch if you'd like.
Please feel free to commit, assuming you would roll that leading whitespace back.

[Hahnfeld]

In D39070#908036, @gkistanova wrote:

Let's keep only the changes relevant to this patch. Feel free to make a separate cosmetic NFC patch if you'd like.
Please feel free to commit, assuming you would roll that leading whitespace back.

I've committed this patch and two small follow-ups. Can you give me a ping once you restarted the master to pick up the new configuration?

[Hahnfeld]

zorg/trunk/zorg/buildbot/builders/OpenMPBuilder.py

142 ↗

(On Diff #120446)

Looks like this doesn't work for ninja, we have to pass it to CMake. I'm on travel this weekend, will submit a patch next week...

[Hahnfeld]

Could you kindly reconfigure the master to pick up the change?

Thanks
Jonas

zorg/trunk/zorg/buildbot/builders/OpenMPBuilder.py

142 ↗

(On Diff #120446)

I just committed rL316901 that should fix the problem.

[Hahnfeld]

@gkistanova Could you reconfigure the master so that rL316901 becomes active?

buildbot/osuosl/master/config/builders.py

 from zorg.buildbot.builders import ClangBuilder
 from zorg.buildbot.builders import LLVMBuilder
 from zorg.buildbot.builders import PollyBuilder
 from zorg.buildbot.builders import LLDBBuilder
 from zorg.buildbot.builders import LLDBuilder
 from zorg.buildbot.builders import LLGoBuilder
 from zorg.buildbot.builders import ClangAndLLDBuilder
 from zorg.buildbot.builders import SanitizerBuilder
 from zorg.buildbot.builders import SanitizerBuilderII
 from zorg.buildbot.builders import SanitizerBuilderWindows
-from zorg.buildbot.builders import Libiomp5Builder
+from zorg.buildbot.builders import OpenMPBuilder
 from zorg.buildbot.builders import LibcxxAndAbiBuilder
 from zorg.buildbot.builders import SphinxDocsBuilder
 from zorg.buildbot.builders import ABITestsuitBuilder
 from zorg.buildbot.builders import ClangLTOBuilder3Stage
 from zorg.buildbot.builders import ClangLTOBuilder
 from zorg.buildbot.builders import UnifiedTreeBuilder
 from zorg.buildbot.builders import CUDATestsuiteBuilder
 from zorg.buildbot.builders import AOSPBuilder
def _get_sanitizer_builders():
                                              "-DLLVM_DEFAULT_TARGET_TRIPLE=mipsel-unknown-linux-gnu",
                                              "-DLLVM_TARGET_ARCH=Mips"],
                            stage1_upload_directory='clang-cmake-mipsel',
                            env = {'BOTO_CONFIG': '/var/buildbot/llvmlab-build-artifacts.boto'})},
           ]
 
 def _get_openmp_builders():
     return [
-        {'name': "libomp-gcc-x86_64-linux-debian",
+        {'name': "openmp-gcc-x86_64-linux-debian",
          'slavenames':["gribozavr4"],
-         'builddir':"libomp-gcc-x86_64-linux-debian",
-         'factory' : Libiomp5Builder.getLibompCMakeBuildFactory(
+         'builddir':"openmp-gcc-x86_64-linux-debian",
+         'factory' : OpenMPBuilder.getOpenMPCMakeBuildFactory(
                          c_compiler="gcc",
                          cxx_compiler="g++",
+                         libomptarget=True,
                          env={'PATH':'/home/llvmbb/bin/clang-latest/bin:/home/llvmbb/bin:/usr/local/bin:/usr/local/bin:/usr/bin:/bin'})},
 
-        {'name': "libomp-clang-x86_64-linux-debian",
+        {'name': "openmp-clang-x86_64-linux-debian",
          'slavenames':["gribozavr4"],
-         'builddir':"libomp-clang-x86_64-linux-debian",
-         'factory' : Libiomp5Builder.getLibompCMakeBuildFactory(
+         'builddir':"openmp-clang-x86_64-linux-debian",
+         'factory' : OpenMPBuilder.getOpenMPCMakeBuildFactory(
                          c_compiler="clang",
                          cxx_compiler="clang++",
+                         libomptarget=True,
+                         test_libomptarget=True,
                          env={'PATH':'/home/llvmbb/bin/clang-latest/bin:/home/llvmbb/bin:/usr/local/bin:/usr/local/bin:/usr/bin:/bin'})},
 
-        {'name': "libomp-ompt-gcc-x86_64-linux-debian",
+        {'name': "openmp-ompt-gcc-x86_64-linux-debian",
          'slavenames':["gribozavr4"],
-         'builddir':"libomp-ompt-gcc-x86_64-linux-debian",
-         'factory' : Libiomp5Builder.getLibompCMakeBuildFactory(
+         'builddir':"openmp-ompt-gcc-x86_64-linux-debian",
+         'factory' : OpenMPBuilder.getOpenMPCMakeBuildFactory(
                          c_compiler="gcc",
                          cxx_compiler="g++",
                          ompt=True,
                          env={'PATH':'/home/llvmbb/bin/clang-latest/bin:/home/llvmbb/bin:/usr/local/bin:/usr/local/bin:/usr/bin:/bin'})},
 
-        {'name': "libomp-ompt-clang-x86_64-linux-debian",
+        {'name': "openmp-ompt-clang-x86_64-linux-debian",
          'slavenames':["gribozavr4"],
-         'builddir':"libomp-ompt-clang-x86_64-linux-debian",
-         'factory' : Libiomp5Builder.getLibompCMakeBuildFactory(
+         'builddir':"openmp-ompt-clang-x86_64-linux-debian",
+         'factory' : OpenMPBuilder.getOpenMPCMakeBuildFactory(
                          c_compiler="clang",
                          cxx_compiler="clang++",
                          ompt=True,
                          env={'PATH':'/home/llvmbb/bin/clang-latest/bin:/home/llvmbb/bin:/usr/local/bin:/usr/local/bin:/usr/bin:/bin'})},
-                         
-        {'name': "libomp-clang-ppc64le-linux-debian",
+        {'name': "openmp-clang-ppc64le-linux-debian",
          'slavenames':["ppc64le-nvidia-K40"],
-         'builddir':"libomp-clang-ppc64le-linux-debian",
-         'factory' : Libiomp5Builder.getLibompCMakeBuildFactory(
-                         c_compiler="/home/bbot/opt/clang/bin/clang",
-                         cxx_compiler="/home/bbot/opt/clang/bin/clang++",
+         'builddir':"openmp-clang-ppc64le-linux-debian",
+         'factory' : OpenMPBuilder.getOpenMPCMakeBuildFactory(
+                         c_compiler="clang",
+                         cxx_compiler="clang++",
+                         libomptarget=True,
+                         test_libomptarget=True,
                          env={'PATH':'/home/bbot/opt/cmake/bin:/home/bbot/opt/ninja/bin:/usr/local/bin:/usr/bin:/bin'})},
 
         ]
 
 def _get_libcxx_builders():
     ericwf_slaves = ['ericwf-buildslave2', 'ericwf-buildslave-fast']
     return [
-        # gribozavr's builders on gribozavr4 
+        # gribozavr's builders on gribozavr4
         {'name': 'libcxx-libcxxabi-x86_64-linux-debian',
          'slavenames': ['gribozavr4'],
          'builddir': 'libcxx-libcxxabi-x86_64-linux-debian',
          'factory': LibcxxAndAbiBuilder.getLibcxxAndAbiBuilder(
              env={'CC': 'clang', 'CXX': 'clang++'},
              lit_extra_args=['--shuffle'],
              check_libcxx_abilist=True),
          'category': 'libcxx'},
return [
                  'CC': 'clang', 'CXX': 'clang++'},
             cmake_extra_opts={'LLVM_BUILD_32_BITS': 'ON'},
             lit_extra_opts={'std': 'c++1z', 'enable_warnings': 'true'},
             check_libcxx_abilist=False),
         'category': 'libcxx'},
 
         {'name': 'libcxx-libcxxabi-x86_64-linux-ubuntu-asan',
          'slavenames': ericwf_slaves,
          'builddir' : 'libcxx-libcxxabi-x86_64-linux-ubuntu-asan',
          'factory': LibcxxAndAbiBuilder.getLibcxxAndAbiBuilder(
             env={'PATH': '/usr/local/bin:/usr/bin:/bin',
                  'CC': 'clang', 'CXX': 'clang++'},
             cmake_extra_opts={'LLVM_USE_SANITIZER': 'Address'},
             lit_extra_opts={'std':'c++1z'}),
         'category': 'libcxx'},
 
         {'name': 'libcxx-libcxxabi-x86_64-linux-ubuntu-ubsan',

zorg/buildbot/builders/Libiomp5Builder.py

-import os
-
-import buildbot
-import buildbot.process.factory
-from buildbot.steps.source import SVN
-from buildbot.steps.shell import ShellCommand
-from buildbot.steps.shell import WarningCountingShellCommand
-from buildbot.process.properties import WithProperties
-from zorg.buildbot.commands.LitTestCommand import LitTestCommand
-from zorg.buildbot.commands.NinjaCommand import NinjaCommand
-
-
-def getLibompCMakeBuildFactory(clean=True, env=None, ompt=False, test=True, c_compiler="gcc", cxx_compiler="g++"):
-
-    # Prepare environmental variables. Set here all env we want everywhere.
-    merged_env = {
-        'TERM' : 'dumb' # Make sure Clang doesn't use color escape sequences.
-                 }
-    if env is not None:
-        # Overwrite pre-set items with the given ones, so user can set anything.
-        merged_env.update(env)
-
-    openmp_srcdir = "openmp.src"
-    openmp_builddir = "openmp.build"
-    llvm_srcdir = "llvm.src"
-    llvm_builddir = "llvm.build"
-
-    f = buildbot.process.factory.BuildFactory()
-
-    # Get libomp
-    f.addStep(SVN(name='svn-libomp',
-                  mode='update',
-                  baseURL='http://llvm.org/svn/llvm-project/openmp/',
-                  defaultBranch='trunk',
-                  workdir=openmp_srcdir))
-
-    # Get llvm to build llvm-lit
-    f.addStep(SVN(name='svn-llvm',
-                  mode='update',
-                  baseURL='http://llvm.org/svn/llvm-project/llvm/',
-                  defaultBranch='trunk',
-                  workdir=llvm_srcdir))
-
-    # Get clang  if c/c++ compilers is clang/clang++
-    if c_compiler == "clang":
-        f.addStep(SVN(name='svn-clang',
-                      mode='update',
-                      baseURL='http://llvm.org/svn/llvm-project/cfe/',
-                      defaultBranch='trunk',
-                      workdir='%s/tools/clang' % llvm_srcdir))
-
-    # Clean directory, if requested.
-    cleanBuildRequested = lambda step: clean or step.build.getProperty("clean")
-
-    f.addStep(ShellCommand(name="clean",
-                           command=["rm", "-rf",openmp_builddir,llvm_builddir],
-                           warnOnFailure=True,
-                           description=["clean"],
-                           doStepIf=cleanBuildRequested,
-                           workdir='.',
-                           env=merged_env))
-
-    # CMake llvm
-    f.addStep(ShellCommand(name='cmake llvm',
-                           command=["cmake", "../"+llvm_srcdir,
-                                    "-G", "Ninja",
-                                    "-DCMAKE_BUILD_TYPE=Release",
-                                    "-DLLVM_ENABLE_ASSERTIONS=ON",
-                                    "-DCMAKE_C_COMPILER="+c_compiler,
-                                    "-DCMAKE_CXX_COMPILER="+cxx_compiler],
-                           haltOnFailure=True,
-                           description='cmake llvm',
-                           workdir=llvm_builddir,
-                           env=merged_env))
-
-    # Make clang build or just llvm utils build
-    if c_compiler == "clang": 
-        f.addStep(NinjaCommand(name="make clang build",
-                               haltOnFailure=True,
-                               description=["make clang build"],
-                               workdir=llvm_builddir,
-                               env=merged_env))
-    else:
-        f.addStep(NinjaCommand(name="make llvm utils build",
-                               targets=["LLVMX86Utils"],
-                               haltOnFailure=True,
-                               description=["make llvm utils build"],
-                               workdir=llvm_builddir,
-                               env=merged_env))
-
-    if ompt:
-        f.addStep(NinjaCommand(name="make ompt test utils",
-                               targets=["FileCheck"],
-                               haltOnFailure=True,
-                               description=["make ompt test utils"],
-                               workdir=llvm_builddir,
-                               env=merged_env))
-
-    # Add clang/llvm-lit to PATH
-    merged_env.update( { 'PATH' : WithProperties("%(workdir)s/" + llvm_builddir + "/bin:" + "${PATH}")} )
-
-    # CMake libomp
-    command=["cmake", "../"+openmp_srcdir,
-             "-DCMAKE_C_COMPILER="+c_compiler,
-             "-DCMAKE_CXX_COMPILER="+cxx_compiler]
-    if ompt:
-        command.append("-DLIBOMP_OMPT_SUPPORT=ON")
-    f.addStep(ShellCommand(name='cmake libomp',
-                           command=command,
-                           haltOnFailure=True,
-                           description='cmake libomp',
-                           workdir=openmp_builddir,
-                           env=merged_env))
-
-    # Make libomp
-    f.addStep(WarningCountingShellCommand(name='make libomp build',
-                                          command=['make'],
-                                          haltOnFailure=True,
-                                          description='make libomp build',
-                                          workdir=openmp_builddir,
-                                          env=merged_env))
-
-    # Test, if requested
-    if test:
-        f.addStep(LitTestCommand(name="make check-libomp",
-                                 command=["make", "check-libomp"],
-                                 haltOnFailure=True,
-                                 description=["make check-libomp"],
-                                 descriptionDone=["build checked"],
-                                 workdir=openmp_builddir,
-                                 env=merged_env))
-
-    return f

zorg/buildbot/builders/OpenMPBuilder.py

+import os
+
+import buildbot
+import buildbot.process.factory
+from buildbot.steps.source import SVN
+from buildbot.steps.shell import Configure, ShellCommand
+from buildbot.steps.shell import WarningCountingShellCommand
+from buildbot.process.properties import WithProperties
+from zorg.buildbot.commands.LitTestCommand import LitTestCommand
+from zorg.buildbot.commands.NinjaCommand import NinjaCommand
+
+
+def getOpenMPCMakeBuildFactory(
+        c_compiler          = 'gcc',        # The C compiler to use.
+        cxx_compiler        = 'g++',        # The C++ compiler to use.
+        jobs                = '%(jobs)s',   # Number of concurrent jobs.
+        clean               = True,         # "clean" step is requested if true
+        env                 = None,         # Environmental variables for all steps.
+        libomptarget        = False,        # Build libomptarget.
+        ompt                = False,        # Whether to enable the OpenMP Tools Interface.
+        test                = True,         # Test the built libraries.
+        test_libomptarget   = False,        # Test the built libraries.
+    ):
+
+    # Prepare environmental variables. Set here all env we want everywhere.
+    merged_env = {
+        'TERM' : 'dumb' # Make sure Clang doesn't use color escape sequences.
+                 }
+    # Overwrite pre-set items with the given ones, so user can set anything.
+    if env is not None:
+        merged_env.update(env)
+
+    openmp_srcdir = 'openmp.src'
+    openmp_builddir = 'openmp.build'
+    llvm_srcdir = 'llvm.src'
+    llvm_builddir = 'llvm.build'
+    build_clang = c_compiler == 'clang'
+
+    f = buildbot.process.factory.BuildFactory()
+
+    # Checkout sources.
+    f.addStep(
+        SVN(
+            name            = 'svn-openmp',
+            mode            = 'update',
+            baseURL         = 'http://llvm.org/svn/llvm-project/openmp/',
+            defaultBranch   = 'trunk',
+            workdir         = openmp_srcdir))
+
+    # LLVM sources are needed to build utilities for testing.
+    f.addStep(
+        SVN(
+            name            = 'svn-llvm',
+            mode            = 'update',
+            baseURL         = 'http://llvm.org/svn/llvm-project/llvm/',
+            defaultBranch   = 'trunk',
+            workdir         = llvm_srcdir))
+
+    # Get Clang sources, if requested.
+    if build_clang == 'clang':
+        f.addStep(
+            SVN(
+                name            = 'svn-clang',
+                mode            = 'update',
+                baseURL         = 'http://llvm.org/svn/llvm-project/cfe/',
+                defaultBranch   = 'trunk',
+                workdir         = '%s/tools/clang' % llvm_srcdir))
+
+    cleanBuildRequested = lambda step: clean or step.build.getProperty("clean")
+    f.addStep(
+        ShellCommand(
+            name            = 'clean',
+            command         = ['rm', '-rf', openmp_builddir, llvm_builddir],
+            warnOnFailure   = True,
+            description     = ['clean'],
+            doStepIf        = cleanBuildRequested,
+            workdir         = '.',
+            env             = merged_env))
+
+    # Configure LLVM (and Clang, if requested).
+    cmake_args  = ['cmake', '-G', 'Ninja', '../%s' % llvm_srcdir]
+    cmake_args += ['-DCMAKE_BUILD_TYPE=Release', '-DLLVM_ENABLE_ASSERTIONS=ON']
+    cmake_args += ['-DCMAKE_C_COMPILER=%s' % c_compiler]
+    cmake_args += ['-DCMAKE_CXX_COMPILER=%s' % cxx_compiler]
+    f.addStep(
+        Configure(
+            name        = 'configure-llvm',
+            command     = cmake_args,
+            description = 'configure llvm',
+            workdir     = llvm_builddir,
+            env         = merged_env))
+
+    # Build LLVM utils.
+    f.addStep(
+        NinjaCommand(
+            name        = 'compile-llvm-utils',
+            targets     = ['FileCheck'],
+            description = 'compile llvm utils',
+            workdir     = llvm_builddir,
+            env         = merged_env))
+
+    # Build Clang if requested.
+    if build_clang:
+        f.addStep(
+            NinjaCommand(
+                name        = 'compile-clang',
+                description = 'compile clang',
+                workdir     = llvm_builddir,
+                env         = merged_env))
+
+    # Add llvm-lit and clang (if built) to PATH
+    merged_env.update({
+        'PATH': WithProperties('%(workdir)s/' + llvm_builddir + '/bin:${PATH}')})
+
+    # Configure OpenMP runtime libraries.
+    cmake_args  = ['cmake', '-G', 'Ninja', '../%s' % openmp_srcdir]
+    cmake_args += ['-DCMAKE_C_COMPILER=%s' % c_compiler]
+    cmake_args += ['-DCMAKE_CXX_COMPILER=%s' % cxx_compiler]
+    if libomptarget:
+        cmake_args += ['-DOPENMP_ENABLE_LIBOMPTARGET=ON']
+    if ompt:
+        cmake_args += ['-DLIBOMP_OMPT_SUPPORT=ON']
+    f.addStep(
+        Configure(
+            name        = 'configure-openmp',
+            command     = cmake_args,
+            description = 'configure openmp',
+            workdir     = openmp_builddir,
+            env         = merged_env))
+
+    # Build OpenMP runtime libraries.
+    f.addStep(
+        NinjaCommand(
+            name        = 'compile-openmp',
+            description = 'compile openmp',
+            workdir     = openmp_builddir,
+            env         = merged_env))
+
+    # Test OpenMP runtime libraries, if requested.
+    if test:
+        ninja_test_args  = ['ninja', WithProperties('-j %s' % jobs)]
+        ninja_test_args += [WithProperties('LIT_ARGS=-v -j %s' % jobs)]
+        f.addStep(
+            LitTestCommand(
+                name        = 'test-libomp',
+                command     = ninja_test_args + ['check-libomp'],
+                description = 'test libomp',
+                workdir     = openmp_builddir,
+                env         = merged_env))
+
+        if test_libomptarget:
+            f.addStep(
+                LitTestCommand(
+                    name        = 'test-libomptarget',
+                    command     = ninja_test_args + ['check-libomptarget'],
+                    description = 'test libomptarget',
+                    workdir     = openmp_builddir,
+                    env         = merged_env))
+
+    return f