Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move first RISC-V builder over to cross-compiler + execute tests under qemu-system setup #279

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Move first RISC-V builder over to cross-compiler + execute tests under qemu-system setup #279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec9391e
Add ability to use a CMake toolchains file for specifying details of …
asb Oct 15, 2024
6f37f5b
[RISCV] Move clang-riscv-rva20-2stage to cross-build and then test un…
asb Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions buildbot/osuosl/master/config/builders.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3151,7 +3151,9 @@
'CXX': 'clang++',
})},

## RISC-V RVA20 profile check-all 2-stage
## RISC-V RVA20 profile check-all 2-stage. The second stage is
# cross-compiled on the x86 host and then lit runs under a qemu-system image
# using the just-build artifacts.
{'name' : "clang-riscv-rva20-2stage",
'tags' : ["clang"],
'workernames' : ["rise-clang-riscv-rva20-2stage"],
Expand All @@ -3161,6 +3163,7 @@
useTwoStage=True,
runTestSuite=False,
testStage1=False,
checkout_compiler_rt=False,
extra_cmake_args=[
"-DCMAKE_C_COMPILER=clang",
"-DCMAKE_CXX_COMPILER=clang++",
Expand All @@ -3169,9 +3172,23 @@
"-DCMAKE_C_COMPILER_LAUNCHER=ccache",
"-DCMAKE_CXX_COMPILER_LAUNCHER=ccache"],
extra_stage2_cmake_args=[
"-DLLVM_ENABLE_LLD=True",
"-DCMAKE_C_FLAGS='-march=rva20u64'",
"-DCMAKE_CXX_FLAGS='-march=rva20u64'"]
util.Interpolate(f"-DLLVM_NATIVE_TOOL_DIR=%(prop:builddir)s/stage1.install/bin"),
"-DLLVM_BUILD_TESTS=True",
"-DLLVM_EXTERNAL_LIT=/home/buildbot-worker/lit-on-qemu-system-rva20.py",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably a temporary path but where can this script be placed?

I think given that it's in stage 2 you could have it in llvm-project somewhere, as long as you hardcode the ../<whatever buildbot calls the repo folder because sometimes it uses non-default names...>. I'm pretty sure the workers don't get a copy of llvm-zorg, otherwise that would be the perfect place for it.

(this is a not an important detail though, as long as the script is available somewhere for debugging purposes)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick review and for picking this out. I'd actually meant to call out this hardcoding in the PR as something I'd appreciate input on. Happy to put the script wherever is helpful, and I can document the setup I use for the VM - though in reality a bit of work will be needed to precisely replicate the setup (as is the case for most builders). I'd probably be better restructuring so there's a common lit-on-qemu-system.py helper that takes/selects any additional arguments via environment variables (in this case, just different args for -cpu for qemu).

],
stage2_toolchain_options=[
"set(CMAKE_SYSTEM_NAME Linux)",
"set(CMAKE_SYSROOT /home/buildbot-worker/rvsysroot)",
"set(CMAKE_C_COMPILER_TARGET riscv64-linux-gnu)",
"set(CMAKE_CXX_COMPILER_TARGET riscv64-linux-gnu)",
"set(CMAKE_C_FLAGS_INIT '-march=rva20u64')",
"set(CMAKE_CXX_FLAGS_INIT '-march=rva20u64')",
"set(CMAKE_LINKER_TYPE LLD)",
"set(CMAKE_FIND_ROOT_PATH_MODE_PROGRAM NEVER)",
"set(CMAKE_FIND_ROOT_PATH_MODE_LIBRARY ONLY)",
"set(CMAKE_FIND_ROOT_PATH_MODE_INCLUDE ONLY)",
"set(CMAKE_FIND_ROOT_PATH_MODE_PACKAGE ONLY)",
]
)},

## RISC-V RVA23 profile check-all 2-stage
Expand Down
40 changes: 35 additions & 5 deletions zorg/buildbot/builders/ClangBuilder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ def getClangCMakeBuildFactory(
# CMake arguments to use for stage2 instead of extra_cmake_args.
extra_stage2_cmake_args=None,

# If set, use a toolchains file for the stage 2 build and add
# these options to it.
stage2_toolchain_options=None,

# Extra repositories
checkout_clang_tools_extra=True,
checkout_compiler_rt=True,
Expand All @@ -180,6 +184,7 @@ def getClangCMakeBuildFactory(
submitURL=submitURL, testerName=testerName,
env=env, extra_cmake_args=extra_cmake_args,
extra_stage2_cmake_args=extra_stage2_cmake_args,
stage2_toolchain_options=stage2_toolchain_options,
checkout_clang_tools_extra=checkout_clang_tools_extra,
checkout_lld=checkout_lld,
checkout_compiler_rt=checkout_compiler_rt,
Expand Down Expand Up @@ -219,6 +224,10 @@ def _getClangCMakeBuildFactory(
# CMake arguments to use for stage2 instead of extra_cmake_args.
extra_stage2_cmake_args=None,

# If set, use a toolchains file for the stage 2 build and add
# these options to it.
stage2_toolchain_options=None,

# Extra repositories
checkout_clang_tools_extra=True,
checkout_compiler_rt=True,
Expand Down Expand Up @@ -427,9 +436,31 @@ def _getClangCMakeBuildFactory(
# Note: Backslash path separators do not work well with cmake and ninja.
# Forward slash path separator works on Windows as well.
stage1_cc = InterpolateToPosixPath(
f"-DCMAKE_C_COMPILER=%(prop:builddir)s/{stage1_install}/bin/{cc}")
f"%(prop:builddir)s/{stage1_install}/bin/{cc}")
stage1_cxx = InterpolateToPosixPath(
f"-DCMAKE_CXX_COMPILER=%(prop:builddir)s/{stage1_install}/bin/{cxx}")
f"%(prop:builddir)s/{stage1_install}/bin/{cxx}")

# If stage2_toolchain_options is set when we'll use a toolchain file
# to specify the compiler being used (the just-built stage1) and add
# any stage2_toolchain_options to it. Otherwise, just set
# -DCMAKE_{C,CXX}_COMPILER.
if stage2_toolchain_options is None:
compiler_args = [
f"-DCMAKE_C_COMPILER={stage1_cc}",
f"-DCMAKE_CXX_COMPILER={stage1_cxx}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought maybe we should use a toolchain file for all 2 stage builds but:

  • It's not recommended practice if you aren't cross compiling.
  • CMAKE_C_COMPILER and friends would not be visible in the log files for stage 2.
  • Reproducing a build now has the extra step of writing the toolchain file, even if it was just copy paste.

So I agree with the decision to not change how the existing builds work.

If transparency were our only goal here, I'd say to pass all the toolchain file contents as -D... but you said:

(notably, some CMake variables can't be set outside of a toolchain file)

Which answers that (and I have had similar experiences with CMake).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crazy idea though, could the toolchain file's contents be pulled from a file here in the llvm-zorg repo?

It could be like:

<the options you set in the builder already>
# Set your host compiler here.
# set(CMAKE_C_COMPILER ....)
# set(CMAKE_CXX_COMPILER ...)

Then when someone wants to reproduce, there is a starting point that's already in the right form to use.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In not actual Python terms:

stage2_toolchain_options=[open(file).splitlines()]

]
else:
toolchain_file = InterpolateToPosixPath(
f"%(prop:builddir)s/{stage2_build}/stage1-toolchain.cmake")
with open(toolchain_file, 'w') as file:
file.write(f"set(CMAKE_C_COMPILER {stage1_cc})\n")
file.write(f"set(CMAKE_CXX_COMPILER {stage1_cxx})\n")
for option in stage2_toolchain_options:
file.write(f"{option}\n")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether this runs on the build master or the worker. If its the former, it might need to be a step "write toolchain file" that echos the contents into a file on the worker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I think it's likely this isn't writing to a worker-local file as I hope.


compiler_args = [
"-DCMAKE_TOOLCHAIN_FILE={toolchain_file}"
]

# If we have a separate stage2 cmake arg list, then ensure we re-apply
# enable_projects and enable_runtimes if necessary.
Expand All @@ -445,13 +476,12 @@ def _getClangCMakeBuildFactory(

rel_src_dir = LLVMBuildFactory.pathRelativeTo(f.llvm_srcdir, stage2_build)
cmake_cmd2 = [cmake, "-G", "Ninja", rel_src_dir,
stage1_cc,
stage1_cxx,
f"-DCMAKE_BUILD_TYPE={stage2_config}",
"-DLLVM_ENABLE_ASSERTIONS=True",
f"-DLLVM_LIT_ARGS={lit_args}",
f"-DCMAKE_INSTALL_PREFIX=../{stage2_install}"
] + (extra_stage2_cmake_args or extra_cmake_args)
] + (extra_stage2_cmake_args or extra_cmake_args) \
+ compiler_args

f.addStep(ShellCommand(name='cmake stage 2',
command=cmake_cmd2,
Expand Down