Skip to content

Conversation

jtb20
Copy link
Contributor

@jtb20 jtb20 commented Jul 18, 2025

This is an experimental patch to incrementally refactor the build scripts. The approach taken is particularly relevant to scripts which build multiple configurations of a component.

There are several ideas here. Firstly, build scripts can be introspected so you can see what they are doing:

$ ./build_offload.sh list_configs
asan
perf
perf+asan
debug
debug+asan
$ AOMP_BUILD_SANITIZER=0 ./build_offload.sh list_configs
perf
debug

Build steps are split into different tasks, each of which is wrapped in a "task_{foo}" function (which is passed the configuration name):

$ ./build_offload.sh list_tasks perf
clean
cmake
build
install

You can list all tasks like this:

$ ./build_offload.sh list
task_precheck
task_patch
task_clean asan
task_cmake asan
task_build asan
task_install asan
task_clean perf
task_cmake perf
task_build perf
task_install perf
task_clean perf+asan
task_cmake perf+asan
task_build perf+asan
task_install perf+asan
task_clean debug
task_cmake debug
task_build debug
task_install debug
task_postinstall debug
task_clean debug+asan
task_cmake debug+asan
task_build debug+asan
task_install debug+asan
task_unpatch

Or e.g. see what will happen for different environment variable settings:

$ AOMP_BUILD_DEBUG=0 ./build_offload.sh list
task_precheck
task_patch
task_clean asan
task_cmake asan
task_build asan
task_install asan
task_clean perf
task_cmake perf
task_build perf
task_install perf
task_clean perf+asan
task_cmake perf+asan
task_build perf+asan
task_install perf+asan
task_unpatch

You can still invoke the build scripts the same way as at present:

$ ./build_offload.sh
[...builds everything...]

$ ./build_offload.sh cmake
[...just runs cmake steps...]

$ ./build_offload.sh nocmake
[...just runs build steps...]

$ ./build_offload.sh install
[...just runs install steps...]

Only the build_offload.sh script has been done so far, but this backward compatibility allows other scripts to be refactored in a similar way incrementally.

The aim (not yet realised) is to then provide a new or alternate front-end (to build_aomp.sh) using exactly the same build scripts, allowing fine-grained control over which components to build/rebuild, and so on.

Internally, the refactoring already done on build_offload.sh demonstrates very much deduplication, particularly around the setting of cmake options. Further work will allow much of the "boilerplate" in each of the build scripts to be replaced by a central implementation.

Environment variables used to control the build are now read via two functions (get_config_var_string and get_config_var_bool). There are several reasons for this indirection:

  • It allows us to have a single location to check which environment variables are being used to control the build.

  • (For the alternate frontend) the options can be changed via
    e.g. command-line options instead of environment variables.

  • The _bool function emits the string 'true' or 'false'. This allows
    the use of:

    if $Result; then ... fi

    instead of

    if [ "$Result" -eq 1 ]; then ... fi

    which is shorter/somewhat easier to read.

"Actions" (the emulated "nocmake", etc. options, and the new "list", etc. options) are implemented as functions named "do_{foo}", invoked by the new "command_dispatcher" function.

There are a few other actions, e.g.:

$ ./build_offload.sh show_build_dir perf
/home/julbrown/git/aomp21.0/build/offload_perf

These will be more useful once more scripts are "taskified".

@jtb20
Copy link
Contributor Author

jtb20 commented Jul 18, 2025

This probably hasn't been tested well enough so far to merge just yet (and apparently needs rebasing already), but I'm interested in feedback on the approach.

The build_offload.sh script should do the same thing it does at present, but one thing to note is the build steps take place in a different order -- clean/cmake/build/install for each configuration in turn, rather than cmake/cmake/cmake build/build/build install/install/install. So the build log can't be compared directly.

@jtb20
Copy link
Contributor Author

jtb20 commented Jul 18, 2025

I just noticed the show_build_dir output is completely bogus. Oops!
(edit: fixed now)

@jtb20 jtb20 marked this pull request as draft July 18, 2025 18:30
@saiislam
Copy link
Member

What is the motivation/benefit in changing the current approach?

@jtb20
Copy link
Contributor Author

jtb20 commented Jul 18, 2025

Several things really, e.g.:

  • finer-grained control over the build process (say, how do you rebuild just the ASAN libraries at the moment, without going "outside" the build script environment and running make/ninja manually?).
  • deduplication of cmake options -- in build_offload.sh at least, it's hard to tell at a glance which options vary between the different build configurations (asan, debug, perf, etc.). There's also a bit too much reliance at the moment on "latest option takes precedence", i.e. we have things like -DOPTION=OFF followed by -DOPTION=ON.
  • generally, moving "copy/pasted" control logic out of each individual build script and into a single location.
  • per-task logs, friendly progress output ("running task N of M", etc.)

...though admittedly, some of these are planned features, rather than things present in the actual patches so far. Think of this more as preparatory work.

(The idea is that each script would implement the "introspection" functions -- list_configs and list_tasks in particular. Then some centralised mechanism would enumerate the complete list of tasks, allowing the user to select which are run for some given build or rebuild.)

This patch splits aomp_utils (shell utility functions) out
of aomp_common_vars, in preparation for adding lots more
non-configuration-related stuff to the new file.

This is mostly straightfoward, but build scripts, etc. need to source
the new file also as well as aomp_common_vars.
This is an experimental patch to incrementally refactor the build scripts.
The approach taken is particularly relevant to scripts which build
multiple configurations of a component.

There are several ideas here. Firstly, build scripts can be introspected
so you can see what they are doing:

$ ./build_offload.sh list_configs
asan
perf
perf+asan
debug
debug+asan

$ AOMP_BUILD_SANITIZER=0 ./build_offload.sh list_configs
perf
debug

Build steps are split into different tasks, each of which is wrapped in a
"task_foo" function (which is passed the configuration name):

$ ./build_offload.sh list_tasks perf
clean
cmake
build
install

You can list all tasks list this:

$ ./build_offload.sh list
task_precheck
task_patch
task_clean asan
task_cmake asan
task_build asan
task_install asan
task_clean perf
task_cmake perf
task_build perf
task_install perf
task_clean perf+asan
task_cmake perf+asan
task_build perf+asan
task_install perf+asan
task_clean debug
task_cmake debug
task_build debug
task_install debug
task_postinstall debug
task_clean debug+asan
task_cmake debug+asan
task_build debug+asan
task_install debug+asan
task_unpatch

Or e.g. see what will happen for different env var settings:

$ AOMP_BUILD_DEBUG=0 ./build_offload.sh list
task_precheck
task_patch
task_clean asan
task_cmake asan
task_build asan
task_install asan
task_clean perf
task_cmake perf
task_build perf
task_install perf
task_clean perf+asan
task_cmake perf+asan
task_build perf+asan
task_install perf+asan
task_unpatch

You can still invoke the build scripts the same way as at present:

$ ./build_offload.sh
[...builds everything...]

$ ./build_offload.sh cmake
[...just runs cmake steps...]

$ ./build_offload.sh nocmake
[...just runs build steps...]

$ ./build_offload.sh install
[...just runs install steps...]

Only the build_offload.sh script has been done so far, but this
backward compatibility allows other scripts to be refactored in a similar
way incrementally.

The aim (not yet realised) is to then provide a new or alternate front-end
(to build_aomp.sh) using exactly the same build scripts, allowing
fine-grained control over which components to build/rebuild, and so on.

Internally, the refactoring already done on build_offload.sh demonstrates
very much deduplication, particularly around the setting of cmake
options. Further work will allow much of the "boilerplate" in each of
the build scripts to be replaced by a central implementation.

Environment variables used to control the build are now read via two
functions (get_config_var_string and get_config_var_bool).  There are
several reasons for this indirection:

- It allows us to have a single location to check which environment
  variables are being used to control the build.

- (For the alternate frontend) the options can be changed via
  e.g. command-line options instead of environment variables.

- The _bool function emits the string 'true' or 'false'. This allows
  the use of:

    if $Result; then ... fi

  instead of

    if [ "$Result" -eq 1 ]; then ... fi

  which is shorter/somewhat easier to read.

"Actions" (the emulated "nocmake", etc. options, and the new "list",
etc. options) are implemented as functions named "do_{foo}", invoked by
the new "command_dispatcher" function.

There are a few other actions, e.g.:

$ ./build_offload.sh show_build_dir perf
/home/julbrown/git/aomp21.0/build/home/julbrown/git/aomp21.0/offload_perf

These will be more useful once more scripts are "taskified".
@jtb20
Copy link
Contributor Author

jtb20 commented Jul 20, 2025

This version adds the (originally intended, but lost during development!) ability to run individual tasks:

$ ./build_offload.sh task_build debug+asan
 -----Running make for /home/julbrown/git/aomp21.0/build/offload_debug/asan ----
[  0%] Built target intrinsics_gen
[ 10%] Built target offload-tblgen
[ 46%] Built target libomptarget-amdgpu
[ 49%] Built target PluginErrcodes
[ 50%] Built target omptarget.amdgpu
[ 66%] Built target PluginCommon
[ 70%] Built target omptarget.rtl.cuda
[ 73%] Built target omptarget.rtl.host
[ 76%] Built target omptarget.rtl.amdgpu
[100%] Built target omptarget

That should make these changes a bit more immediately useful already.

I also realised the quite heavy use of command substitutions isn't really safe unless the shell "errexit" option (set -e) is turned on. I've added this (to the build_offload.sh script only), but there are several places where it might cause inadvertent early exit in non-error cases, so all that needs checking and fixing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants