Re: libgomp: In OpenACC testing, cycle though $offload_targets, and by default only build for the offload target that we're actually going to test

[Thomas Schwinge <thomas@codesourcery.com>]

Hi!

Ping.

On Mon, 02 May 2016 11:54:27 +0200, I wrote:
> On Fri, 29 Apr 2016 09:43:41 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
> > On Thu, Apr 28, 2016 at 12:43:43PM +0200, Thomas Schwinge wrote:
> > > commit 3b521f3e35fdb4b320e95b5f6a82b8d89399481a
> > > Author: Thomas Schwinge <thomas@codesourcery.com>
> > > Date:   Thu Apr 21 11:36:39 2016 +0200
> > > 
> > >     libgomp: Unconfuse offload plugins vs. offload targets
> > 
> > I don't like this patch at all, rather than unconfusing stuff it
> > makes stuff confusing.  Plugins are just a way to support various
> > offloading targets.
> 
> Huh; my patch exactly clarifies that the offload_targets variable does
> not actually list offload target names, but does list libgomp offload
> plugin names...
> 
> > Can you please post just a short patch without all those changes
> > that does what you want, rather than renaming everything at the same time?
> 
> I thought incremental, self-contained patches were easier to review.
> Anyway, here's the three patches merged into one:
> 
> commit 8060ae3474072eef685381d80f566d1c0942c603
> Author: Thomas Schwinge <thomas@codesourcery.com>
> Date:   Thu Apr 21 11:36:39 2016 +0200
> 
>     libgomp: In OpenACC testing, cycle though $offload_targets, and by default only build for the offload target that we're actually going to test
>     
>     	libgomp/
>     	* plugin/configfrag.ac (offload_targets): Actually enumerate
>     	offload targets, and add...
>     	(offload_plugins): ... this one to enumerate offload plugins.
>     	(OFFLOAD_PLUGINS): Renamed from OFFLOAD_TARGETS.
>     	* target.c (gomp_target_init): Adjust to that.
>     	* testsuite/lib/libgomp.exp: Likewise.
>     	(offload_targets_s, offload_targets_s_openacc): Remove variables.
>     	(offload_target_to_openacc_device_type): New proc.
>     	(check_effective_target_openacc_nvidia_accel_selected)
>     	(check_effective_target_openacc_host_selected): Examine
>     	$openacc_device_type instead of $offload_target_openacc.
>     	* Makefile.in: Regenerate.
>     	* config.h.in: Likewise.
>     	* configure: Likewise.
>     	* testsuite/Makefile.in: Likewise.
>     	* testsuite/libgomp.oacc-c++/c++.exp: Cycle through
>     	$offload_targets (plus "disable") instead of
>     	$offload_targets_s_openacc, and add "-foffload=$offload_target" to
>     	tagopt.
>     	* testsuite/libgomp.oacc-c/c.exp: Likewise.
>     	* testsuite/libgomp.oacc-fortran/fortran.exp: Likewise.
> ---
>  libgomp/Makefile.in                                |  1 +
>  libgomp/config.h.in                                |  4 +-
>  libgomp/configure                                  | 44 +++++++------
>  libgomp/plugin/configfrag.ac                       | 39 +++++++-----
>  libgomp/target.c                                   |  8 +--
>  libgomp/testsuite/Makefile.in                      |  1 +
>  libgomp/testsuite/lib/libgomp.exp                  | 72 ++++++++++------------
>  libgomp/testsuite/libgomp.oacc-c++/c++.exp         | 30 +++++----
>  libgomp/testsuite/libgomp.oacc-c/c.exp             | 30 +++++----
>  libgomp/testsuite/libgomp.oacc-fortran/fortran.exp | 22 ++++---
>  10 files changed, 142 insertions(+), 109 deletions(-)
> 
> diff --git libgomp/Makefile.in libgomp/Makefile.in
> [snipped]
> diff --git libgomp/config.h.in libgomp/config.h.in
> [snipped]
> diff --git libgomp/configure libgomp/configure
> [snipped]
> diff --git libgomp/plugin/configfrag.ac libgomp/plugin/configfrag.ac
> index 88b4156..de0a6f6 100644
> --- libgomp/plugin/configfrag.ac
> +++ libgomp/plugin/configfrag.ac
> @@ -26,8 +26,6 @@
>  # see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>  # <http://www.gnu.org/licenses/>.
>  
> -offload_targets=
> -AC_SUBST(offload_targets)
>  plugin_support=yes
>  AC_CHECK_LIB(dl, dlsym, , [plugin_support=no])
>  if test x"$plugin_support" = xyes; then
> @@ -142,7 +140,13 @@ AC_SUBST(PLUGIN_HSA_LIBS)
>  
>  
>  
> -# Get offload targets and path to install tree of offloading compiler.
> +# Parse offload targets, and figure out libgomp plugin, and configure the
> +# corresponding offload compiler.  offload_plugins and offload_targets will be
> +# populated in the same order.
> +offload_plugins=
> +offload_targets=
> +AC_SUBST(offload_plugins)
> +AC_SUBST(offload_targets)
>  offload_additional_options=
>  offload_additional_lib_paths=
>  AC_SUBST(offload_additional_options)
> @@ -151,13 +155,13 @@ if test x"$enable_offload_targets" != x; then
>    for tgt in `echo $enable_offload_targets | sed -e 's#,# #g'`; do
>      tgt_dir=`echo $tgt | grep '=' | sed 's/.*=//'`
>      tgt=`echo $tgt | sed 's/=.*//'`
> -    tgt_name=
> +    tgt_plugin=
>      case $tgt in
>        *-intelmic-* | *-intelmicemul-*)
> -	tgt_name=intelmic
> +	tgt_plugin=intelmic
>  	;;
>        nvptx*)
> -        tgt_name=nvptx
> +	tgt_plugin=nvptx
>  	PLUGIN_NVPTX=$tgt
>  	PLUGIN_NVPTX_CPPFLAGS=$CUDA_DRIVER_CPPFLAGS
>  	PLUGIN_NVPTX_LDFLAGS=$CUDA_DRIVER_LDFLAGS
> @@ -184,7 +188,7 @@ if test x"$enable_offload_targets" != x; then
>  	    ;;
>  	esac
>  	;;
> -      hsa*)
> +      hsa)
>  	case "${target}" in
>  	  x86_64-*-*)
>  	    case " ${CC} ${CFLAGS} " in
> @@ -192,7 +196,7 @@ if test x"$enable_offload_targets" != x; then
>  	        PLUGIN_HSA=0
>  		;;
>  	      *)
> -	        tgt_name=hsa
> +		tgt_plugin=hsa
>  	        PLUGIN_HSA=$tgt
>  	        PLUGIN_HSA_CPPFLAGS=$HSA_RUNTIME_CPPFLAGS
>  	        PLUGIN_HSA_LDFLAGS="$HSA_RUNTIME_LDFLAGS $HSA_KMT_LDFLAGS"
> @@ -214,7 +218,7 @@ if test x"$enable_offload_targets" != x; then
>  	        LDFLAGS=$PLUGIN_HSA_save_LDFLAGS
>  	        LIBS=$PLUGIN_HSA_save_LIBS
>  	        case $PLUGIN_HSA in
> -	          hsa*)
> +		  hsa)
>  	            HSA_PLUGIN=0
>  	            AC_MSG_ERROR([HSA run-time package required for HSA support])
>  	            ;;
> @@ -231,16 +235,19 @@ if test x"$enable_offload_targets" != x; then
>  	AC_MSG_ERROR([unknown offload target specified])
>  	;;
>      esac
> -    if test x"$tgt_name" = x; then
> +    if test x"$tgt_plugin" = x; then
>        # Don't configure libgomp for this offloading target if we don't build
>        # the corresponding plugin.
>        continue
> -    elif test x"$offload_targets" = x; then
> -      offload_targets=$tgt_name
> +    elif test x"$offload_plugins" = x; then
> +      offload_plugins=$tgt_plugin
> +      offload_targets=$tgt
>      else
> -      offload_targets=$offload_targets,$tgt_name
> +      offload_plugins=$offload_plugins,$tgt_plugin
> +      offload_targets=$offload_targets,$tgt
>      fi
> -    if test "$tgt_name" = hsa; then
> +    # Configure additional search paths.
> +    if test "$tgt_plugin" = hsa; then
>        # Offloading compilation is all handled by the target compiler.
>        :
>      elif test x"$tgt_dir" != x; then
> @@ -252,8 +259,8 @@ if test x"$enable_offload_targets" != x; then
>      fi
>    done
>  fi
> -AC_DEFINE_UNQUOTED(OFFLOAD_TARGETS, "$offload_targets",
> -  [Define to offload targets, separated by commas.])
> +AC_DEFINE_UNQUOTED(OFFLOAD_PLUGINS, "$offload_plugins",
> +  [Define to offload plugins, separated by commas.])
>  AM_CONDITIONAL([PLUGIN_NVPTX], [test $PLUGIN_NVPTX = 1])
>  AC_DEFINE_UNQUOTED([PLUGIN_NVPTX], [$PLUGIN_NVPTX],
>    [Define to 1 if the NVIDIA plugin is built, 0 if not.])
> diff --git libgomp/target.c libgomp/target.c
> index e2dd0e0..c12c085 100644
> --- libgomp/target.c
> +++ libgomp/target.c
> @@ -2467,9 +2467,9 @@ gomp_target_fini (void)
>      }
>  }
>  
> -/* This function initializes the runtime needed for offloading.
> -   It parses the list of offload targets and tries to load the plugins for
> -   these targets.  On return, the variables NUM_DEVICES and NUM_DEVICES_OPENMP
> +/* This function initializes the runtime for offloading.
> +   It parses the list of offload plugins, and tries to load these.
> +   On return, the variables NUM_DEVICES and NUM_DEVICES_OPENMP
>     will be set, and the array DEVICES initialized, containing descriptors for
>     corresponding devices, first the GOMP_OFFLOAD_CAP_OPENMP_400 ones, follows
>     by the others.  */
> @@ -2486,7 +2486,7 @@ gomp_target_init (void)
>    num_devices = 0;
>    devices = NULL;
>  
> -  cur = OFFLOAD_TARGETS;
> +  cur = OFFLOAD_PLUGINS;
>    if (*cur)
>      do
>        {
> diff --git libgomp/testsuite/Makefile.in libgomp/testsuite/Makefile.in
> [snipped]
> diff --git libgomp/testsuite/lib/libgomp.exp libgomp/testsuite/lib/libgomp.exp
> index 1cb4991..4a25421 100644
> --- libgomp/testsuite/lib/libgomp.exp
> +++ libgomp/testsuite/lib/libgomp.exp
> @@ -36,28 +36,6 @@ load_gcc_lib fortran-modules.exp
>  # Try to load a test support file, built during libgomp configuration.
>  load_file libgomp-test-support.exp
>  
> -# Populate offload_targets_s (offloading targets separated by a space), and
> -# offload_targets_s_openacc (the same, but with OpenACC names; OpenACC spells
> -# some of them a little differently).
> -set offload_targets_s [split $offload_targets ","]
> -set offload_targets_s_openacc {}
> -foreach offload_target_openacc $offload_targets_s {
> -    # Translate to OpenACC names, or skip if not yet supported.
> -    switch $offload_target_openacc {
> -	intelmic {
> -	    continue
> -	}
> -	nvptx {
> -	    set offload_target_openacc "nvidia"
> -	}
> -	hsa {
> -	    continue
> -	}
> -    }
> -    lappend offload_targets_s_openacc "$offload_target_openacc"
> -}
> -lappend offload_targets_s_openacc "host"
> -
>  set dg-do-what-default run
>  
>  #
> @@ -135,9 +113,9 @@ proc libgomp_init { args } {
>      set always_ld_library_path ".:${blddir}/.libs"
>  
>      # Add liboffloadmic build directory in LD_LIBRARY_PATH to support
> -    # non-fallback testing for Intel MIC targets
> +    # Intel MIC offloading testing.
>      global offload_targets
> -    if { [string match "*,intelmic,*" ",$offload_targets,"] } {
> +    if { [string match "*,*-intelmic*,*" ",$offload_targets,"] } {
>  	append always_ld_library_path ":${blddir}/../liboffloadmic/.libs"
>  	append always_ld_library_path ":${blddir}/../liboffloadmic/plugin/.libs"
>  	# libstdc++ is required by liboffloadmic
> @@ -245,8 +223,7 @@ proc libgomp_init { args } {
>      # Disable color diagnostics
>      lappend ALWAYS_CFLAGS "additional_flags=-fdiagnostics-color=never"
>  
> -    # Used for support non-fallback offloading.
> -    # Help GCC to find target mkoffload.
> +    # Help GCC to find offload compilers' mkoffload.
>      global offload_additional_options
>      if { $offload_additional_options != "" } {
>  	lappend ALWAYS_CFLAGS "additional_flags=${offload_additional_options}"
> @@ -322,6 +299,28 @@ proc libgomp_option_proc { option } {
>      }
>  }
>  
> +# Translate offload target to OpenACC device type.  Return the empty string if
> +# not suitable or not yet supported, and "host" for offload target "disable".
> +proc offload_target_to_openacc_device_type { offload_target } {
> +    switch -glob $offload_target {
> +	disable {
> +	    return "host"
> +	}
> +	hsa {
> +	    return ""
> +	}
> +	*-intelmic* {
> +	    return ""
> +	}
> +	nvptx* {
> +	    return "nvidia"
> +	}
> +	default {
> +	    error "Unknwon offload target: $offload_target"
> +	}
> +    }
> +}
> +
>  # Return 1 if offload device is available.
>  proc check_effective_target_offload_device { } {
>      return [check_runtime_nocache offload_device_available_ {
> @@ -373,29 +372,22 @@ proc check_effective_target_openacc_nvidia_accel_present { } {
>      } "" ]
>  }
>  
> -# Return 1 if at least one nvidia board is present, and the nvidia device type
> -# is selected by default by means of setting the environment variable
> -# ACC_DEVICE_TYPE.
> +# Return 1 if at least one nvidia board is present, and the OpenACC "nvidia"
> +# device type is selected.
>  
>  proc check_effective_target_openacc_nvidia_accel_selected { } {
>      if { ![check_effective_target_openacc_nvidia_accel_present] } {
>  	return 0;
>      }
> -    global offload_target_openacc
> -    if { $offload_target_openacc == "nvidia" } {
> -        return 1;
> -    }
> -    return 0;
> +    global openacc_device_type
> +    return [string match "nvidia" $openacc_device_type]
>  }
>  
> -# Return 1 if the host target is selected for offloaded
> +# Return 1 if the OpenACC "host" device type is selected.
>  
>  proc check_effective_target_openacc_host_selected { } {
> -    global offload_target_openacc
> -    if { $offload_target_openacc == "host" } {
> -        return 1;
> -    }
> -    return 0;
> +    global openacc_device_type
> +    return [string match "host" $openacc_device_type]
>  }
>  
>  # Return 1 if the selected OMP device is actually a HSA device
> diff --git libgomp/testsuite/libgomp.oacc-c++/c++.exp libgomp/testsuite/libgomp.oacc-c++/c++.exp
> index 608b298..befb921 100644
> --- libgomp/testsuite/libgomp.oacc-c++/c++.exp
> +++ libgomp/testsuite/libgomp.oacc-c++/c++.exp
> @@ -70,18 +70,23 @@ if { $lang_test_file_found } {
>  	set libstdcxx_includes ""
>      }
>  
> -    # Test OpenACC with available accelerators.
> -    foreach offload_target_openacc $offload_targets_s_openacc {
> -	set tagopt "-DACC_DEVICE_TYPE_$offload_target_openacc=1"
> +    # Test with all offload targets, and with offloading disabled.
> +    foreach offload_target [concat [split $offload_targets ","] "disable"] {
> +	set openacc_device_type [offload_target_to_openacc_device_type $offload_target]
> +	set tagopt "-DACC_DEVICE_TYPE_$openacc_device_type=1"
>  
> -	switch $offload_target_openacc {
> -	    host {
> +	switch $openacc_device_type {
> +	    "" {
> +		untested "$subdir $offload_target offloading"
> +		continue
> +	    }
> +	    "host" {
>  		set acc_mem_shared 1
>  	    }
> -	    nvidia {
> +	    "nvidia" {
>  		if { ![check_effective_target_openacc_nvidia_accel_present] } {
>  		    # Don't bother; execution testing is going to FAIL.
> -		    untested "$subdir $offload_target_openacc offloading"
> +		    untested "$subdir $offload_target offloading"
>  		    continue
>  		}
>  
> @@ -94,12 +99,15 @@ if { $lang_test_file_found } {
>  		set acc_mem_shared 0
>  	    }
>  	    default {
> -		set acc_mem_shared 0
> +		error "Unknwon OpenACC device type: $openacc_device_type (offload target: $offload_target)"
>  	    }
>  	}
>  	set tagopt "$tagopt -DACC_MEM_SHARED=$acc_mem_shared"
>  
> -	setenv ACC_DEVICE_TYPE $offload_target_openacc
> +	# To avoid overhead, by default only build for the offload target that
> +	# we're actually going to test.
> +	set tagopt "$tagopt -foffload=$offload_target"
> +	setenv ACC_DEVICE_TYPE $openacc_device_type
>  
>  	# To get better test coverage for device-specific code that is only
>  	# ever used in offloading configurations, we'd like more thorough
> @@ -108,8 +116,8 @@ if { $lang_test_file_found } {
>  	# -O0 and -O2 only, to avoid testing times exploding too much, under
>  	# the assumption that between -O0 and -O[something] there is the
>  	# biggest difference in the overall structure of the generated code.
> -	switch $offload_target_openacc {
> -	    host {
> +	switch $offload_target {
> +	    "disable" {
>  		set-torture-options [list \
>  					 { -O2 } ]
>  	    }
> diff --git libgomp/testsuite/libgomp.oacc-c/c.exp libgomp/testsuite/libgomp.oacc-c/c.exp
> index b509a10..5355b87 100644
> --- libgomp/testsuite/libgomp.oacc-c/c.exp
> +++ libgomp/testsuite/libgomp.oacc-c/c.exp
> @@ -31,20 +31,25 @@ set ld_library_path $always_ld_library_path
>  append ld_library_path [gcc-set-multilib-library-path $GCC_UNDER_TEST]
>  set_ld_library_path_env_vars
>  
> -# Test OpenACC with available accelerators.
> +# Test with all offload targets, and with offloading disabled.
>  set SAVE_ALWAYS_CFLAGS "$ALWAYS_CFLAGS"
> -foreach offload_target_openacc $offload_targets_s_openacc {
> +foreach offload_target [concat [split $offload_targets ","] "disable"] {
>      set ALWAYS_CFLAGS "$SAVE_ALWAYS_CFLAGS"
> -    set tagopt "-DACC_DEVICE_TYPE_$offload_target_openacc=1"
> +    set openacc_device_type [offload_target_to_openacc_device_type $offload_target]
> +    set tagopt "-DACC_DEVICE_TYPE_$openacc_device_type=1"
>  
> -    switch $offload_target_openacc {
> -	host {
> +    switch $openacc_device_type {
> +	"" {
> +	    untested "$subdir $offload_target offloading"
> +	    continue
> +	}
> +	"host" {
>  	    set acc_mem_shared 1
>  	}
> -	nvidia {
> +	"nvidia" {
>  	    if { ![check_effective_target_openacc_nvidia_accel_present] } {
>  		# Don't bother; execution testing is going to FAIL.
> -		untested "$subdir $offload_target_openacc offloading"
> +		untested "$subdir $offload_target offloading"
>  		continue
>  	    }
>  
> @@ -57,12 +62,15 @@ foreach offload_target_openacc $offload_targets_s_openacc {
>  	    set acc_mem_shared 0
>  	}
>  	default {
> -	    set acc_mem_shared 0
> +	    error "Unknwon OpenACC device type: $openacc_device_type (offload target: $offload_target)"
>  	}
>      }
>      set tagopt "$tagopt -DACC_MEM_SHARED=$acc_mem_shared"
>  
> -    setenv ACC_DEVICE_TYPE $offload_target_openacc
> +    # To avoid overhead, by default only build for the offload target that
> +    # we're actually going to test.
> +    set tagopt "$tagopt -foffload=$offload_target"
> +    setenv ACC_DEVICE_TYPE $openacc_device_type
>  
>      # To get better test coverage for device-specific code that is only
>      # ever used in offloading configurations, we'd like more thorough
> @@ -71,8 +79,8 @@ foreach offload_target_openacc $offload_targets_s_openacc {
>      # -O0 and -O2 only, to avoid testing times exploding too much, under
>      # the assumption that between -O0 and -O[something] there is the
>      # biggest difference in the overall structure of the generated code.
> -    switch $offload_target_openacc {
> -	host {
> +    switch $offload_target {
> +	"disable" {
>  	    set-torture-options [list \
>  				     { -O2 } ]
>  	}
> diff --git libgomp/testsuite/libgomp.oacc-fortran/fortran.exp libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
> index 2d6b647..8507b67 100644
> --- libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
> +++ libgomp/testsuite/libgomp.oacc-fortran/fortran.exp
> @@ -65,30 +65,38 @@ if { $lang_test_file_found } {
>      append ld_library_path [gcc-set-multilib-library-path $GCC_UNDER_TEST]
>      set_ld_library_path_env_vars
>  
> -    # Test OpenACC with available accelerators.
> -    foreach offload_target_openacc $offload_targets_s_openacc {
> -	set tagopt "-DACC_DEVICE_TYPE_$offload_target_openacc=1"
> +    # Test with all offload targets, and with offloading disabled.
> +    foreach offload_target [concat [split $offload_targets ","] "disable"] {
> +	set openacc_device_type [offload_target_to_openacc_device_type $offload_target]
> +	set tagopt "-DACC_DEVICE_TYPE_$openacc_device_type=1"
>  
> -	switch $offload_target_openacc {
> +	switch $openacc_device_type {
> +	    "" {
> +		untested "$subdir $offload_target offloading"
> +		continue
> +	    }
>  	    host {
>  		set acc_mem_shared 1
>  	    }
>  	    nvidia {
>  		if { ![check_effective_target_openacc_nvidia_accel_present] } {
>  		    # Don't bother; execution testing is going to FAIL.
> -		    untested "$subdir $offload_target_openacc offloading"
> +		    untested "$subdir $offload_target offloading"
>  		    continue
>  		}
>  
>  		set acc_mem_shared 0
>  	    }
>  	    default {
> -		set acc_mem_shared 0
> +		error "Unknwon OpenACC device type: $openacc_device_type (offload target: $offload_target)"
>  	    }
>  	}
>  	set tagopt "$tagopt -DACC_MEM_SHARED=$acc_mem_shared"
>  
> -	setenv ACC_DEVICE_TYPE $offload_target_openacc
> +	# To avoid overhead, by default only build for the offload target that
> +	# we're actually going to test.
> +	set tagopt "$tagopt -foffload=$offload_target"
> +	setenv ACC_DEVICE_TYPE $openacc_device_type
>  
>  	# For Fortran we're doing torture testing, as Fortran has far more tests
>  	# with arrays etc. that testing just -O0 or -O2 is insufficient, that is


Grüße
 Thomas