diff mbox

libgomp: Unconfuse offload plugins vs. offload targets

Message ID 87ziqvresv.fsf@hertz.schwinge.homeip.net
State New
Headers show

Commit Message

Thomas Schwinge June 8, 2016, 1:27 p.m. UTC
Hi!

This got me confused recently, so I took the effort to clean it up.  OK
to commit?

commit 5a1b2d8440a459fd2c623ba76fd6cab478ada54f
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Wed Jun 8 15:18:11 2016 +0200

    libgomp: Unconfuse offload plugins vs. offload targets
    
    	libgomp/
    	* plugin/configfrag.ac: Populate and AC_SUBST offload_plugins
    	instead of offload_targets, and AC_DEFINE_UNQUOTED OFFLOAD_PLUGINS
    	instead of OFFLOAD_TARGETS.
    	* target.c (gomp_target_init): Adjust to that.
    	* testsuite/lib/libgomp.exp: Likewise.
    	* testsuite/libgomp-test-support.exp.in: Likewise.
    	* Makefile.in: Regenerate.
    	* config.h.in: Regenerate.
    	* configure: Regenerate.
    	* testsuite/Makefile.in: Regenerate.
---
 libgomp/Makefile.in                           |  2 +-
 libgomp/config.h.in                           |  4 ++--
 libgomp/configure                             | 34 ++++++++++++++-------------
 libgomp/plugin/configfrag.ac                  | 34 ++++++++++++++-------------
 libgomp/target.c                              |  8 +++----
 libgomp/testsuite/Makefile.in                 |  2 +-
 libgomp/testsuite/lib/libgomp.exp             | 25 +++++++++-----------
 libgomp/testsuite/libgomp-test-support.exp.in |  2 +-
 8 files changed, 56 insertions(+), 55 deletions(-)



Grüße
 Thomas

Comments

Jakub Jelinek June 8, 2016, 2:08 p.m. UTC | #1
On Wed, Jun 08, 2016 at 03:27:44PM +0200, Thomas Schwinge wrote:
> Hi!
> 
> This got me confused recently, so I took the effort to clean it up.  OK
> to commit?

As I said earlier, I don't find anything confusing on what we have there
and would strongly prefer not to change it.
Can you submit the actual testsuite change which got hidden in all the
renaming separately?

Thanks.

	Jakub
Thomas Schwinge June 8, 2016, 3:45 p.m. UTC | #2
Hi!

On Wed, 8 Jun 2016 16:08:38 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Jun 08, 2016 at 03:27:44PM +0200, Thomas Schwinge wrote:
> > This got me confused recently, so I took the effort to clean it up.  OK
> > to commit?
> 
> As I said earlier, I don't find anything confusing on what we have there
> and would strongly prefer not to change it.

Please explain why you are rejecting clean-up patches that make the code
(variable names) actually match its semantics, make it easy for the
reader?

> Can you submit the actual testsuite change which got hidden in all the
> renaming separately?
> 
> Thanks.

I submitted that more than a month ago, and pinged it thrice,
<http://news.gmane.org/find-root.php?message_id=%3C871t4qu15e.fsf%40hertz.schwinge.homeip.net%3E>.
As you can see, there actually is a difference between offload_plugins
and offload_targets (for example, "intelmic"
vs. "x86_64-intelmicemul-linux-gnu"), and I'm using both variables -- to
avoid having to translate the more specific
"x86_64-intelmicemul-linux-gnu" (which we required in the test harness)
into the less specific "intelmic" (for plugin loading) in
libgomp/target.c.  I can do that, so that we can continue to use just a
single offload_targets variable, but I consider that a less elegant
solution.


Grüße
 Thomas
diff mbox

Patch

diff --git libgomp/Makefile.in libgomp/Makefile.in
[...]
diff --git libgomp/config.h.in libgomp/config.h.in
[...]
diff --git libgomp/configure libgomp/configure
[...]
diff --git libgomp/plugin/configfrag.ac libgomp/plugin/configfrag.ac
index 88b4156..93d3a71 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,10 @@  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=
+AC_SUBST(offload_plugins)
 offload_additional_options=
 offload_additional_lib_paths=
 AC_SUBST(offload_additional_options)
@@ -151,13 +152,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 +185,7 @@  if test x"$enable_offload_targets" != x; then
 	    ;;
 	esac
 	;;
-      hsa*)
+      hsa)
 	case "${target}" in
 	  x86_64-*-*)
 	    case " ${CC} ${CFLAGS} " in
@@ -192,7 +193,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 +215,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 +232,17 @@  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
     else
-      offload_targets=$offload_targets,$tgt_name
+      offload_plugins=$offload_plugins,$tgt_plugin
     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 +254,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 48b9ab8..2b7e627 100644
--- libgomp/target.c
+++ libgomp/target.c
@@ -2474,9 +2474,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.  */
@@ -2493,7 +2493,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
[...]
diff --git libgomp/testsuite/lib/libgomp.exp libgomp/testsuite/lib/libgomp.exp
index 1cb4991..ae65e68 100644
--- libgomp/testsuite/lib/libgomp.exp
+++ libgomp/testsuite/lib/libgomp.exp
@@ -36,12 +36,11 @@  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 ","]
+# Populate offload_plugins_s (offloading plugins separated by a space), and
+# offload_targets_s_openacc (offload targets with OpenACC device type names).
+set offload_plugins_s [split $offload_plugins ","]
 set offload_targets_s_openacc {}
-foreach offload_target_openacc $offload_targets_s {
+foreach offload_target_openacc $offload_plugins_s {
     # Translate to OpenACC names, or skip if not yet supported.
     switch $offload_target_openacc {
 	intelmic {
@@ -135,9 +134,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
-    global offload_targets
-    if { [string match "*,intelmic,*" ",$offload_targets,"] } {
+    # Intel MIC offloading testing.
+    global offload_plugins
+    if { [string match "*,intelmic,*" ",$offload_plugins,"] } {
 	append always_ld_library_path ":${blddir}/../liboffloadmic/.libs"
 	append always_ld_library_path ":${blddir}/../liboffloadmic/plugin/.libs"
 	# libstdc++ is required by liboffloadmic
@@ -245,8 +244,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}"
@@ -373,9 +371,8 @@  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] } {
@@ -388,7 +385,7 @@  proc check_effective_target_openacc_nvidia_accel_selected { } {
     return 0;
 }
 
-# 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
diff --git libgomp/testsuite/libgomp-test-support.exp.in libgomp/testsuite/libgomp-test-support.exp.in
index 5a724fb..1068483 100644
--- libgomp/testsuite/libgomp-test-support.exp.in
+++ libgomp/testsuite/libgomp-test-support.exp.in
@@ -3,4 +3,4 @@  set cuda_driver_lib "@CUDA_DRIVER_LIB@"
 set hsa_runtime_lib "@HSA_RUNTIME_LIB@"
 set hsa_kmt_lib "@HSA_KMT_LIB@"
 
-set offload_targets "@offload_targets@"
+set offload_plugins "@offload_plugins@"