From 54e2fa8508912d33a09252bac0b65e46412bba8b Mon Sep 17 00:00:00 2001 From: Brian Barrett Date: Fri, 9 Sep 2022 21:08:51 -0700 Subject: [PATCH 1/5] build: Refactor OPAL_VAR_SCOPE system Refactor the OPAL_VAR_SCOPE_PUSH and OPAL_VAR_SCOPE_POP macros to be wrappers around shell functions to reduce code duplication in the output configure. This patch reduces the size of the output configrue by approximately 30% on MacOS. Move the scope stack from shell to m4, as all the inputs are currently string literals, so we can have the same functionality with less work in the resulting configure script. With the scope stack in m4, also perform variable name conflicts during autogen time. This allows for us to detect more conflicts, as there were times that the parent macro pushed a variable name in the scope but didn't initialize it before calling a child macro with a conflict, which was a bit of a ticking timebomb. Note that we do still search the current shell environment during OPAL_VAR_SCOPE_PUSH, as macros may use a variable without pushing it into a scope. FInally, be more aggressive about cleaning up the environment after ourselves. Signed-off-by: Brian Barrett (cherry picked from commit a8c829c5899ff768878c9ab4e310bf23bb6c08e7) --- config/opal_functions.m4 | 65 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/config/opal_functions.m4 b/config/opal_functions.m4 index 50d1b553f63..080cef6e461 100644 --- a/config/opal_functions.m4 +++ b/config/opal_functions.m4 @@ -319,8 +319,69 @@ dnl ####################################################################### dnl ####################################################################### dnl ####################################################################### -m4_copy([OAC_VAR_SCOPE_PUSH], [OPAL_VAR_SCOPE_PUSH]) -m4_copy([OAC_VAR_SCOPE_POP], [OPAL_VAR_SCOPE_POP]) +AC_DEFUN([OPAL_VAR_SCOPE_INIT], +[opal_var_scope_push() +{ + opal_var_scope_push_lineno=$[]1 + shift + # First, check to see if any of these variables are already set. + # This is a simple sanity check to ensure we're not already + # overwriting pre-existing variables (that have a non-empty + # value). It's not a perfect check, but at least it's something. + for opal_var_scope_tmp_var in $[]@; do + AS_VAR_SET_IF([$opal_var_scope_tmp_var], + [AS_VAR_COPY([opal_var_scope_tmp_var_val], [$opal_var_scope_tmp_var]) + AC_MSG_WARN([Found configure shell variable clash at line $opal_var_scope_push_lineno!]) + AC_MSG_WARN([[OPAL_VAR_SCOPE_PUSH] called on "$opal_var_scope_tmp_var",]) + AC_MSG_WARN([but it is already defined with value "$opal_var_scope_tmp_var_val"]) + AC_MSG_WARN([This usually indicates an error in configure.]) + AC_MSG_ERROR([Cannot continue])]) + done + AS_UNSET([opal_var_scope_push_lineno]) + AS_UNSET([opal_var_scope_tmp_var]) + AS_UNSET([opal_var_scope_tmp_var_val]) +} + +opal_var_scope_pop() +{ + # Iterate over all the variables and unset them all + for opal_var_scope_tmp_var in $[]@; do + AS_UNSET([$opal_var_scope_tmp_var]) + done + AS_UNSET([opal_var_scope_tmp_var]) +}]) + +# OPAL_VAR_SCOPE_PUSH(vars list) +# ------------------------------ +# Scope-check that the vars in the space-separated vars list are not already +# in use. Generate a configure-time error if a conflict is found. Note that +# the in use check is defined as "defined", so even if a var in vars list is +# set outside of OPAL_VAR_SCOPE_PUSH, the check will still trip. +AC_DEFUN([OPAL_VAR_SCOPE_PUSH],[ + AC_REQUIRE([OPAL_VAR_SCOPE_INIT])dnl + m4_pushdef([opal_var_scope_stack], [$1])dnl + m4_foreach_w([opal_var_scope_var], [$1], + [m4_set_add([opal_var_scope_active_set], opal_var_scope_var, + [], [m4_fatal([OPAL_VAR_SCOPE_PUSH found the variable ]opal_var_scope_var[ +active in a previous scope.])])])dnl + opal_var_scope_push ${LINENO} $1 +])dnl + +# OPAL_VAR_SCOPE_POP() +# -------------------- +# Unset the last set of variables set in OPAL_VAR_SCOPE_POP. Every call to +# OPAL_VAR_SCOPE_PUSH should have a matched call to this macro. +AC_DEFUN([OPAL_VAR_SCOPE_POP],[ + AC_REQUIRE([OPAL_VAR_SCOPE_INIT])dnl + m4_ifdef([opal_var_scope_stack], [], + [m4_fatal([OPAL_VAR_SCOPE_POP was called without a defined +variable stack. This usually means that OPAL_VAR_SCOPE_POP was called more +times than OPAL_VAR_SCOPE_PUSH.])])dnl + m4_foreach_w([opal_var_scope_var], opal_var_scope_stack, + [m4_set_remove([opal_var_scope_active_set], opal_var_scope_var)])dnl + opal_var_scope_pop opal_var_scope_stack + m4_popdef([opal_var_scope_stack])dnl +])dnl dnl ####################################################################### dnl ####################################################################### From 069ca08d4b84d0bd44212c2632411fec2bb6b46b Mon Sep 17 00:00:00 2001 From: Sam James Date: Wed, 16 Nov 2022 20:58:24 +0000 Subject: [PATCH 2/5] build: fix bashisms in configure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit configure scripts need to be runnable with a POSIX-compliant /bin/sh. On many (but not all!) systems, /bin/sh is provided by Bash, so errors like this aren't spotted. Notably Debian defaults to /bin/sh provided by dash which doesn't tolerate such bashisms as '=='. This retains compatibility with bash. Signed-off-by: Sam James (cherry picked from commit 08a558d28213133105b6b1a1ca903a0da7807585) --- ompi/mca/op/avx/configure.m4 | 10 +++++----- opal/mca/memory/patcher/configure.m4 | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ompi/mca/op/avx/configure.m4 b/ompi/mca/op/avx/configure.m4 index 44e834301bb..73c80486039 100644 --- a/ompi/mca/op/avx/configure.m4 +++ b/ompi/mca/op/avx/configure.m4 @@ -315,15 +315,15 @@ AC_DEFUN([MCA_ompi_op_avx_CONFIG],[ [$op_sse3_support], [SSE3 supported in the current build]) AM_CONDITIONAL([MCA_BUILD_ompi_op_has_avx512_support], - [test "$op_avx512_support" == "1"]) + [test "$op_avx512_support" = "1"]) AM_CONDITIONAL([MCA_BUILD_ompi_op_has_avx2_support], - [test "$op_avx2_support" == "1"]) + [test "$op_avx2_support" = "1"]) AM_CONDITIONAL([MCA_BUILD_ompi_op_has_avx_support], - [test "$op_avx_support" == "1"]) + [test "$op_avx_support" = "1"]) AM_CONDITIONAL([MCA_BUILD_ompi_op_has_sse41_support], - [test "$op_sse41_support" == "1"]) + [test "$op_sse41_support" = "1"]) AM_CONDITIONAL([MCA_BUILD_ompi_op_has_sse3_support], - [test "$op_sse3_support" == "1"]) + [test "$op_sse3_support" = "1"]) AC_SUBST(MCA_BUILD_OP_AVX512_FLAGS) AC_SUBST(MCA_BUILD_OP_AVX2_FLAGS) AC_SUBST(MCA_BUILD_OP_AVX_FLAGS) diff --git a/opal/mca/memory/patcher/configure.m4 b/opal/mca/memory/patcher/configure.m4 index 579c480fb02..9dff16def65 100644 --- a/opal/mca/memory/patcher/configure.m4 +++ b/opal/mca/memory/patcher/configure.m4 @@ -51,7 +51,7 @@ AC_DEFUN([MCA_opal_memory_patcher_CONFIG],[ # Per the above logic, memory patcher no longer supports MacOS/Darwin, # so we no longer support Darwin-specific logic for intercept_mmap. # See issue #6853: mmap infinite recurse in opal/mca/memory/patcher - AS_IF([test "$opal_memory_patcher_happy" == "yes"], [ + AS_IF([test "$opal_memory_patcher_happy" = "yes"], [ AC_CHECK_FUNCS([__curbrk]) AC_CHECK_HEADERS([linux/mman.h sys/syscall.h]) AC_CHECK_DECLS([__syscall], [], [], [#include ]) From 47733d6d18f1cd0d7b1051e715e4f1df6f7b29b8 Mon Sep 17 00:00:00 2001 From: Brian Barrett Date: Mon, 5 Sep 2022 20:47:37 -0700 Subject: [PATCH 3/5] build: Drop support for config-time components Drop support for both adding components at configure time (ie, after autogen has run) and components that have a stand-alone configure script. The first change greatly simplifies reasoning about variable names in the MCA configure code, since the component name will never be a shell variable with this change. Note that there was already one place where we would have mis-handled such a component, in setting LDFLAGS for the wrapper compilers. The second is dead code after the first, as no component currently built in the OMPI tree uses an external configure script directly (note that config macros that call a third-party configure script, like the io/romio component, are still fine). Organizations can still build tarballs with added components after this change, they just must run autogen to add the components to the list of available component. And developers are also free to add run-time components that are built outside of the OMPI build (generally by building OMPI with the --with-devel-headers configure option). Signed-off-by: Brian Barrett (cherry picked from commit 6d06552ea0cc3c7aa8c2ac7ffb2b1d8a895e278f) --- config/opal_mca.m4 | 120 ++++++--------------------------------------- 1 file changed, 16 insertions(+), 104 deletions(-) diff --git a/config/opal_mca.m4 b/config/opal_mca.m4 index f9d33b66ffa..935b8c65be2 100644 --- a/config/opal_mca.m4 +++ b/config/opal_mca.m4 @@ -455,18 +455,6 @@ AC_DEFUN([MCA_CONFIGURE_FRAMEWORK],[ [AS_IF([test $components_last_result -eq 1], [best_mca_component_priority=]OPAL_EVAL_ARG([MCA_$1_$2_]mca_component[_PRIORITY]))]) ])]) - # configure components that provide their own configure script. - # It would be really hard to run these for "find first that - # works", so we don't :) - m4_if(OPAL_EVAL_ARG([MCA_$1_]$2[_CONFIGURE_MODE]), [STOP_AT_FIRST], [], - [m4_if(OPAL_EVAL_ARG([MCA_$1_]$2[_CONFIGURE_MODE]), [STOP_AT_FIRST_PRIORITY], [], - [m4_if(OPAL_EVAL_ARG([MCA_$1_]$2[_CONFIGURE_MODE]), [PRIORITY], [], - [MCA_CHECK_IGNORED_PRIORITY($1, $2) - AS_IF([test "$3" != "0"], - [MCA_CONFIGURE_ALL_CONFIG_COMPONENTS($1, $2, [all_components], - [static_components], [dso_components], - [static_ltlibs])])])])]) - AS_VAR_SET_IF([OPAL_EVAL_ARG([DIRECT_$2])], [ AC_MSG_CHECKING([if direct-selection component exists for $2 framework]) direct_component_happy=no @@ -640,74 +628,6 @@ AC_DEFUN([MCA_CONFIGURE_M4_CONFIG_COMPONENT],[ unset compile_mode ]) - -###################################################################### -# -# MCA_CONFIGURE_ALL_CONFIG_COMPONENTS -# -# configure all components in the given framework that have configure -# scripts and should be configured according to the usual rules... -# -# USAGE: -# MCA_CONFIGURE_ALL_CONFIG_COMPONENTS(project_name, -# framework_name, -# all_components_variable, -# static_components_variable, -# dso_components_variable, -# static_ltlibs_variable) -# -###################################################################### -AC_DEFUN([MCA_CONFIGURE_ALL_CONFIG_COMPONENTS],[ - for component_path in $srcdir/$1/mca/$2/* ; do - component="`basename $component_path`" - if test -d $component_path && test -x $component_path/configure ; then - opal_show_subsubsubtitle "MCA component $2:$component (need to configure)" - - opal_show_verbose "OPAL_MCA_ALL_CONFIG_COMPONENTS: before, should_build=$8" - MCA_COMPONENT_BUILD_CHECK($1, $2, $component, - [should_build=1], [should_build=0]) - MCA_COMPONENT_COMPILE_MODE($1, $2, $component, compile_mode) - opal_show_verbose "OPAL_MCA_ALL_CONFIG_COMPONENTS: after, should_build=$should_build" - - if test "$should_build" = "1" ; then - OPAL_CONFIG_SUBDIR([$1/mca/$2/$component], - [$opal_subdir_args], - [should_build=1], [should_build=0]) - opal_show_verbose "OPAL_MCA_ALL_CONFIG_COMPONENTS: after subdir, should_build=$should_build" - fi - - if test "$should_build" = "1" ; then - # do some extra work to pass flags back from the - # top-level configure, the way a configure.m4 - # component would. - infile="$srcdir/$1/mca/$2/$3/post_configure.sh" - if test -f $infile; then - - # First check for the ABORT tag - line="`$GREP ABORT= $infile | cut -d= -f2-`" - if test -n "$line" && test "$line" != "no"; then - AC_MSG_WARN([MCA component configure script told me to abort]) - AC_MSG_ERROR([cannot continue]) - fi - - m4_foreach(flags, [LDFLAGS, LIBS], - [[line="`$GREP WRAPPER_EXTRA_]flags[= $infile | cut -d= -f2-`"] - eval "line=$line" - if test -n "$line"; then - $2[_]$3[_WRAPPER_EXTRA_]flags[="$line"] - fi - ])dnl - fi - - MCA_PROCESS_COMPONENT($1, $2, $component, $3, $4, $5, $6, $compile_mode) - else - MCA_PROCESS_DEAD_COMPONENT($1, $2, $component) - fi - fi - done -]) - - # MCA_COMPONENT_COMPILE_MODE(project_name (1), framework_name (2), # component_name (3), compile_mode_variable (4)) # ------------------------------------------------------------------------- @@ -716,17 +636,15 @@ AC_DEFUN([MCA_CONFIGURE_ALL_CONFIG_COMPONENTS],[ # the cached value for subsequent tests. The string is not stored in a cache # variable (ie .*_cv_.*) because cache variables would not be invalidated # based on changes to --enable-mca-dso or --enable-mca-static. -# -# NOTE: component_name may not be determined until runtime.... AC_DEFUN([MCA_COMPONENT_COMPILE_MODE],[ OAC_ASSERT_LITERAL([$1], [1])dnl OAC_ASSERT_LITERAL([$2], [2])dnl + OAC_ASSERT_LITERAL([$3], [3])dnl AS_VAR_PUSHDEF([compile_mode_cv], [$1_$2_$3_compile_mode])dnl AS_VAR_SET_IF([compile_mode_cv], [], - [AS_LITERAL_IF([$3], - [m4_ifdef([MCA_$1_$2_$3_COMPILE_MODE], + [m4_ifdef([MCA_$1_$2_$3_COMPILE_MODE], [dnl We introduced caching of this check after setting the compile dnl mode by the substitute macro was common, and there was not a dnl polymorphic variable assumption in all those macros, so we use @@ -735,8 +653,7 @@ AC_DEFUN([MCA_COMPONENT_COMPILE_MODE],[ MCA_$1_$2_$3_COMPILE_MODE([$1], [$2], [$3], [component_compile_mode_tmp]) AS_VAR_COPY([compile_mode_cv], [$component_compile_mode_tmp]) OPAL_VAR_SCOPE_POP], - [MCA_COMPONENT_COMPILE_MODE_INTERNAL([$1], [$2], [$3], [compile_mode_cv])])], - [MCA_COMPONENT_COMPILE_MODE_INTERNAL([$1], [$2], [$3], [compile_mode_cv])])]) + [MCA_COMPONENT_COMPILE_MODE_INTERNAL([$1], [$2], [$3], [compile_mode_cv])])]) AS_VAR_COPY([$4], [compile_mode_cv]) AS_VAR_POPDEF([compile_mode_cv])dnl ]) @@ -758,10 +675,10 @@ AC_DEFUN([MCA_COMPONENT_COMPILE_MODE_INTERNAL], [ OPAL_VAR_SCOPE_PUSH([compile_mode_internal_tmp SHARED_FRAMEWORK SHARED_COMPONENT STATIC_FRAMEWORK STATIC_COMPONENT]) SHARED_FRAMEWORK="$DSO_$2" - AS_VAR_COPY([SHARED_COMPONENT], [DSO_$2_$3]) + SHARED_COMPONENT="$DSO_$2_$3" STATIC_FRAMEWORK="$STATIC_$2" - AS_VAR_COPY([STATIC_COMPONENT], [STATIC_$2_$3]) + STATIC_COMPONENT="$STATIC_$2_$3" # Look for the most specific specifier between static/dso. If # there is a tie (either neither or both specified), prefer @@ -819,8 +736,6 @@ AC_DEFUN([OPAL_MCA_STRIP_LAFILES], [ #--------------------------------------------------------------------- # Final setup work for a given component. It should be known before # calling that this component can build properly (and exists) -# -# NOTE: component_name may not be determined until runtime.... AC_DEFUN([MCA_PROCESS_COMPONENT],[ AC_REQUIRE([AC_PROG_GREP]) @@ -893,13 +808,13 @@ AC_MSG_ERROR([*** $2 component $3 was supposed to be direct-called, but # wishes all LDFLAGS and LIBS to be provided as wrapper flags. AS_IF([test "$8" = "static"], [AS_VAR_SET_IF([$2_$3_WRAPPER_EXTRA_LDFLAGS], - [AS_VAR_COPY([tmp_flags], [$2_$3_WRAPPER_EXTRA_LDFLAGS])], - [AS_VAR_COPY([tmp_flags], [$2_$3_LDFLAGS])]) + [tmp_flags=${$2_$3_WRAPPER_EXTRA_LDFLAGS}], + [tmp_flags=${$2_$3_LDFLAGS}]) OPAL_FLAGS_APPEND_UNIQ([mca_wrapper_extra_ldflags], [$tmp_flags]) AS_VAR_SET_IF([$2_$3_WRAPPER_EXTRA_LIBS], - [AS_VAR_COPY([tmp_flags], [$2_$3_WRAPPER_EXTRA_LIBS])], - [AS_VAR_COPY([tmp_all_flags], [$2_$3_LIBS]) + [tmp_flags=${$2_$3_WRAPPER_EXTRA_LIBS}], + [tmp_all_flags=${$2_$3_LIBS} OPAL_MCA_STRIP_LAFILES([tmp_flags], [$tmp_all_flags])]) OPAL_FLAGS_APPEND_MOVE([mca_wrapper_extra_libs], [$tmp_flags])]) @@ -910,13 +825,12 @@ AC_MSG_ERROR([*** $2 component $3 was supposed to be direct-called, but # Since a configure script component can never be used in a # STOP_AT_FIRST framework, we don't have to implement the else # clause in the literal check. - AS_LITERAL_IF([$3], - [AS_IF([test "$$2_$3_WRAPPER_EXTRA_CPPFLAGS" != ""], - [m4_if(OPAL_EVAL_ARG([MCA_$1_$2_CONFIGURE_MODE]), [STOP_AT_FIRST], [stop_at_first=1], [stop_at_first=0]) - AS_IF([test "$8" = "static" && test "$stop_at_first" = "1"], - [AS_IF([test "$with_devel_headers" = "yes"], - [OPAL_FLAGS_APPEND_UNIQ([mca_wrapper_extra_cppflags], [$$2_$3_WRAPPER_EXTRA_CPPFLAGS])])], - [AC_MSG_WARN([ignoring $2_$3_WRAPPER_EXTRA_CPPFLAGS ($$2_$3_WRAPPER_EXTRA_CPPFLAGS): component conditions not met])])])]) + AS_IF([test "$$2_$3_WRAPPER_EXTRA_CPPFLAGS" != ""], + [m4_if(OPAL_EVAL_ARG([MCA_$1_$2_CONFIGURE_MODE]), [STOP_AT_FIRST], [stop_at_first=1], [stop_at_first=0]) + AS_IF([test "$8" = "static" && test "$stop_at_first" = "1"], + [AS_IF([test "$with_devel_headers" = "yes"], + [OPAL_FLAGS_APPEND_UNIQ([mca_wrapper_extra_cppflags], [$$2_$3_WRAPPER_EXTRA_CPPFLAGS])])], + [AC_MSG_WARN([ignoring $2_$3_WRAPPER_EXTRA_CPPFLAGS ($$2_$3_WRAPPER_EXTRA_CPPFLAGS): component conditions not met])])]) ]) @@ -926,8 +840,6 @@ AC_MSG_ERROR([*** $2 component $3 was supposed to be direct-called, but # Final setup work for a component that can not be built. Do the # last minute checks to make sure the user isn't doing something # stupid. -# -# NOTE: component_name may not be determined until runtime.... AC_DEFUN([MCA_PROCESS_DEAD_COMPONENT],[ AC_MSG_CHECKING([if MCA component $2:$3 can compile]) AC_MSG_RESULT([no]) @@ -1004,7 +916,7 @@ AC_DEFUN([MCA_COMPONENT_BUILD_CHECK],[ # if we were explicitly disabled, don't build :) AS_IF([test "$DISABLE_$2" = "1"], [want_component=0]) - AS_VAR_IF([DISABLE_$2_$3], [1], [want_component=0]) + AS_IF([test "${DISABLE_$2_$3}" = "1"], [want_component=0]) AS_IF([test "$want_component" = "1"], [$4], [$5]) ]) From 550933065b7a50d60dedfe75f3dd02e03fad5c43 Mon Sep 17 00:00:00 2001 From: Brian Barrett Date: Fri, 8 Apr 2022 15:42:24 +0000 Subject: [PATCH 4/5] build: Run missing package dist-hook earlier The hook that would fail "make dist" if packages weren't configured to be included in the tarball ran at the end of "make dist". This was fine when the packages missing didn't break running make dist. However, Sphinx not being available means that "make dist" won't work because of missing files, but the error was really not intuitive. Move the dist-hook for missing packages from the top level to the config directory Makefile, which has the practical outcome of running the missing package check right after generation of the AUTHORS file, well before any likely missing package make breakage, returning us to clear error messages. Signed-off-by: Brian Barrett (cherry picked from commit 0224e23fe37bbaa0eafab4741f8d27b9ed87d61a) --- Makefile.am | 13 ++++--------- config/Makefile.am | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Makefile.am b/Makefile.am index 8dc915b6cb2..bff3c79c64f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -12,7 +12,7 @@ # Copyright (c) 2006-2022 Cisco Systems, Inc. All rights reserved. # Copyright (c) 2012-2015 Los Alamos National Security, Inc. All rights reserved. # Copyright (c) 2014-2019 Intel, Inc. All rights reserved. -# Copyright (c) 2017-2021 Amazon.com, Inc. or its affiliates. +# Copyright (c) 2017-2022 Amazon.com, Inc. or its affiliates. All Rights reserved. # All Rights reserved. # Copyright (c) 2020 IBM Corporation. All rights reserved. # $COPYRIGHT$ @@ -22,6 +22,9 @@ # $HEADER$ # +# There is an assumption in config/Makefile.am that config is the first +# subdirectory in the DIST_SUBDIRS list. If that changes, work may +# be required. SUBDIRS = config contrib 3rd-party $(MCA_PROJECT_SUBDIRS) test docs DIST_SUBDIRS = config contrib 3rd-party $(MCA_PROJECT_DIST_SUBDIRS) test docs EXTRA_DIST = README.md VERSION Doxyfile LICENSE autogen.pl AUTHORS @@ -34,14 +37,6 @@ dist-hook: echo "AUTHORS file is empty; aborting distribution"; \ exit 1; \ fi - @if test -n "$(OPAL_MAKEDIST_DISABLE)" ; then \ - echo "#########################################################################"; \ - echo "#"; \ - echo "# make dist is disabled due to the following packages: $(OPAL_MAKEDIST_DISABLE)"; \ - echo "#"; \ - echo "#########################################################################"; \ - exit 1; \ - fi # Check for common symbols. Use a "-hook" to increase the odds that a # developer will see it at the end of their installation process. diff --git a/config/Makefile.am b/config/Makefile.am index 92aea16438f..4af3a33f87c 100644 --- a/config/Makefile.am +++ b/config/Makefile.am @@ -15,6 +15,7 @@ # Copyright (c) 2014-2015 Intel, Inc. All rights reserved. # Copyright (c) 2016 Research Organization for Information Science # and Technology (RIST). All rights reserved. +# Copyright (c) 2022 Amazon.com, Inc. or its affiliates. All Rights reserved. # $COPYRIGHT$ # # Additional copyrights may follow @@ -22,6 +23,19 @@ # $HEADER$ # +# This is a weird place for this, but config/ is the first directory that +# is entered, and the dist-hook runs at the end of the current directory, +# so this is the earliest we can run this test. +dist-hook: + @if test -n "$(OPAL_MAKEDIST_DISABLE)" ; then \ + echo "#########################################################################"; \ + echo "#"; \ + echo "# make dist is disabled due to the following packages: $(OPAL_MAKEDIST_DISABLE)"; \ + echo "#"; \ + echo "#########################################################################"; \ + exit 1; \ + fi + EXTRA_DIST = \ distscript.sh \ opal_get_version.m4sh \ From 897084380f9d789cbd7e29bc5f166f21975f4fd1 Mon Sep 17 00:00:00 2001 From: Howard Pritchard Date: Wed, 23 Nov 2022 10:15:54 -0700 Subject: [PATCH 5/5] ofi: support RHEL7 and libfabric pre 1.9 Fix to allow Open MPI to build on RHEL7 systems with default distro libfabric. Related to #10954 Signed-off-by: Howard Pritchard (cherry picked from commit 4f404a8705f777d70e63bb02594d649e2cc5dab4) --- config/opal_check_ofi.m4 | 11 ++++++++++- ompi/mca/mtl/ofi/mtl_ofi.h | 4 ++++ ompi/mca/mtl/ofi/mtl_ofi_component.c | 14 ++++++++++++-- opal/mca/btl/ofi/btl_ofi_component.c | 14 +++++++++++++- opal/mca/btl/ofi/btl_ofi_module.c | 4 +++- 5 files changed, 42 insertions(+), 5 deletions(-) diff --git a/config/opal_check_ofi.m4 b/config/opal_check_ofi.m4 index e873a399fdc..401c8e8d2e9 100644 --- a/config/opal_check_ofi.m4 +++ b/config/opal_check_ofi.m4 @@ -137,7 +137,16 @@ AC_DEFUN([OPAL_CHECK_OFI],[ AC_CHECK_DECLS([PMIX_PACKAGE_RANK], [], [], - [#include ])]) + [#include ]) + + AC_CHECK_MEMBER([struct fi_mr_attr.iface], + [opal_check_fi_mr_attr_iface=1], + [opal_check_fi_mr_attr_iface=0], + [[#include ]]) + + AC_DEFINE_UNQUOTED([OPAL_OFI_HAVE_FI_MR_IFACE], + [${opal_check_fi_mr_attr_iface}], + [check if iface avaiable in fi_mr_attr])]) CPPFLAGS=${opal_check_ofi_save_CPPFLAGS} LDFLAGS=${opal_check_ofi_save_LDFLAGS} diff --git a/ompi/mca/mtl/ofi/mtl_ofi.h b/ompi/mca/mtl/ofi/mtl_ofi.h index bd78f5526f3..97a54c24b1b 100644 --- a/ompi/mca/mtl/ofi/mtl_ofi.h +++ b/ompi/mca/mtl/ofi/mtl_ofi.h @@ -306,6 +306,8 @@ int ompi_mtl_ofi_register_buffer(struct opal_convertor_t *convertor, return OMPI_SUCCESS; } +#if OPAL_OFI_HAVE_FI_MR_IFACE + if ((convertor->flags & CONVERTOR_ACCELERATOR) && ompi_mtl_ofi.hmem_needs_reg) { /* Register buffer */ int ret; @@ -343,6 +345,8 @@ int ompi_mtl_ofi_register_buffer(struct opal_convertor_t *convertor, } } +#endif + return OMPI_SUCCESS; } diff --git a/ompi/mca/mtl/ofi/mtl_ofi_component.c b/ompi/mca/mtl/ofi/mtl_ofi_component.c index 98d0537779e..bfd263db202 100644 --- a/ompi/mca/mtl/ofi/mtl_ofi_component.c +++ b/ompi/mca/mtl/ofi/mtl_ofi_component.c @@ -6,7 +6,7 @@ * Copyright (c) 2015-2016 Los Alamos National Security, LLC. All rights * reserved. * Copyright (c) 2018-2022 Amazon.com, Inc. or its affiliates. All Rights reserved. - * Copyright (c) 2020-2021 Triad National Security, LLC. All rights + * Copyright (c) 2020-2023 Triad National Security, LLC. All rights * reserved. * $COPYRIGHT$ * @@ -617,8 +617,10 @@ ompi_mtl_ofi_component_init(bool enable_progress_threads, } /* Request device transfer capabilities */ +#if defined(FI_HMEM) hints->caps |= FI_HMEM; hints->domain_attr->mr_mode |= FI_MR_HMEM | FI_MR_ALLOCATED; +#endif no_hmem: @@ -737,12 +739,14 @@ ompi_mtl_ofi_component_init(bool enable_progress_threads, __FILE__, __LINE__, fi_strerror(-ret)); if (FI_ENODATA == -ret) { +#if defined(FI_HMEM) /* Attempt selecting a provider without FI_HMEM hints */ if (hints->caps & FI_HMEM) { hints->caps &= ~FI_HMEM; hints->domain_attr->mr_mode &= ~FI_MR_HMEM; goto no_hmem; } +#endif /* It is not an error if no information is returned. */ goto error; } else if (0 != ret) { @@ -770,11 +774,12 @@ ompi_mtl_ofi_component_init(bool enable_progress_threads, opal_argv_free(exclude_list); exclude_list = NULL; + *accelerator_support = false; +#if defined(FI_HMEM) if (!(prov->caps & FI_HMEM)) { opal_output_verbose(50, opal_common_ofi.output, "%s:%d: Libfabric provider does not support device buffers. Continuing with device to host copies.\n", __FILE__, __LINE__); - *accelerator_support = false; } else { *accelerator_support = true; ompi_mtl_ofi.hmem_needs_reg = true; @@ -791,6 +796,11 @@ ompi_mtl_ofi_component_init(bool enable_progress_threads, } } +#else + opal_output_verbose(50, opal_common_ofi.output, + "%s:%d: Libfabric provider does not support device buffers. Continuing with device to host copies.\n", + __FILE__, __LINE__); +#endif /** * Select the format of the OFI tag diff --git a/opal/mca/btl/ofi/btl_ofi_component.c b/opal/mca/btl/ofi/btl_ofi_component.c index 579e5facc48..ebf6705be91 100644 --- a/opal/mca/btl/ofi/btl_ofi_component.c +++ b/opal/mca/btl/ofi/btl_ofi_component.c @@ -15,7 +15,7 @@ * Copyright (c) 2018-2019 Intel, Inc. All rights reserved. * * Copyright (c) 2018-2021 Amazon.com, Inc. or its affiliates. All Rights reserved. - * Copyright (c) 2020-2022 Triad National Security, LLC. All rights + * Copyright (c) 2020-2023 Triad National Security, LLC. All rights * reserved. * $COPYRIGHT$ * @@ -106,7 +106,11 @@ static int validate_info(struct fi_info *info, uint64_t required_caps, char **in mr_mode = info->domain_attr->mr_mode; if (!(mr_mode == FI_MR_BASIC || mr_mode == FI_MR_SCALABLE +#if defined(FI_MR_HMEM) || (mr_mode & ~(FI_MR_VIRT_ADDR | FI_MR_ALLOCATED | FI_MR_PROV_KEY | FI_MR_ENDPOINT | FI_MR_HMEM)) == 0)) { +#else + || (mr_mode & ~(FI_MR_VIRT_ADDR | FI_MR_ALLOCATED | FI_MR_PROV_KEY | FI_MR_ENDPOINT)) == 0)) { +#endif BTL_VERBOSE(("unsupported MR mode")); return OPAL_ERROR; } @@ -339,20 +343,24 @@ static mca_btl_base_module_t **mca_btl_ofi_component_init(int *num_btl_modules, mca_btl_ofi_component.module_count = 0; +#if defined(FI_HMEM) /* Request device transfer capabilities, separate from required_caps */ hints.caps |= FI_HMEM; hints.domain_attr->mr_mode |= FI_MR_HMEM; no_hmem: +#endif /* Do the query. The earliest version that supports FI_HMEM hints is 1.9 */ rc = fi_getinfo(FI_VERSION(1, 9), NULL, NULL, 0, &hints, &info_list); if (0 != rc) { +#if defined(FI_HMEM) if (hints.caps & FI_HMEM) { /* Try again without FI_HMEM hints */ hints.caps &= ~FI_HMEM; hints.domain_attr->mr_mode &= ~FI_MR_HMEM; goto no_hmem; } +#endif BTL_VERBOSE(("fi_getinfo failed with code %d: %s", rc, fi_strerror(-rc))); if (NULL != include_list) { opal_argv_free(include_list); @@ -360,6 +368,7 @@ static mca_btl_base_module_t **mca_btl_ofi_component_init(int *num_btl_modules, return NULL; } +#if defined(FI_HMEM) /* If we get to this point with FI_HMEM hint set, we want it to be a * required capability */ @@ -375,6 +384,7 @@ static mca_btl_base_module_t **mca_btl_ofi_component_init(int *num_btl_modules, } required_caps |= FI_HMEM; } +#endif /* count the number of resources/ */ info = info_list; @@ -604,9 +614,11 @@ static int mca_btl_ofi_init_device(struct fi_info *info) module->use_fi_mr_bind = false; module->bypass_cache = false; +#if defined(FI_HMEM) if (ofi_info->caps & FI_HMEM) { module->super.btl_flags |= MCA_BTL_FLAGS_ACCELERATOR_RDMA; } +#endif if (ofi_info->domain_attr->mr_mode == FI_MR_BASIC || ofi_info->domain_attr->mr_mode & FI_MR_VIRT_ADDR) { diff --git a/opal/mca/btl/ofi/btl_ofi_module.c b/opal/mca/btl/ofi/btl_ofi_module.c index 8497e559523..ae28dc123a5 100644 --- a/opal/mca/btl/ofi/btl_ofi_module.c +++ b/opal/mca/btl/ofi/btl_ofi_module.c @@ -16,7 +16,7 @@ * * Copyright (c) 2018 Amazon.com, Inc. or its affiliates. All Rights reserved. * Copyright (c) 2020 Google, LLC. All rights reserved. - * Copyright (c) 2022 Triad National Security, LLC. All rights + * Copyright (c) 2022-2023 Triad National Security, LLC. All rights * reserved. * $COPYRIGHT$ * @@ -254,6 +254,7 @@ int mca_btl_ofi_reg_mem(void *reg_data, void *base, size_t size, attr.context = NULL; attr.requested_key = (uint64_t) reg; +#if OPAL_OFI_HAVE_FI_MR_IFACE if (OPAL_LIKELY(NULL != base)) { rc = opal_accelerator.check_addr(base, &dev_id, &flags); if (rc < 0) { @@ -270,6 +271,7 @@ int mca_btl_ofi_reg_mem(void *reg_data, void *base, size_t size, } } } +#endif rc = fi_mr_regattr(btl->domain, &attr, 0, &ur->ur_mr); if (0 != rc) {