public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Thomas Schwinge <thomas@codesourcery.com>
To: Jakub Jelinek <jakub@redhat.com>,
	Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>,
	<gcc-patches@gcc.gnu.org>
Cc: Tobias Burnus <tobias@codesourcery.com>,
	Mike Stump <mikestump@comcast.net>,
	Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>,
	Jan Hubicka <hubicka@ucw.cz>,
	Segher Boessenkool <segher@kernel.crashing.org>
Subject: Support parallel testing in libgomp, part I [PR66005]
Date: Fri, 5 May 2023 10:55:41 +0200	[thread overview]
Message-ID: <87h6srb1iq.fsf@euler.schwinge.homeip.net> (raw)
In-Reply-To: <87mw1fmpi5.fsf@schwinge.name>

[-- Attachment #1: Type: text/plain, Size: 5410 bytes --]

Hi!

[Putting Bernhard, Honza, Segher in CC, as they are eager to test this,
based on recent comments on IRC.]  ;-P


First, establish the parallel testing infrastructure -- while still
hard-coding the number of parallel slots to one.

On 2015-05-08T10:40:02+0200, I wrote:
> On Thu, 7 May 2015 13:39:40 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
>> On Thu, May 07, 2015 at 01:26:57PM +0200, Rainer Orth wrote:
>> > As reported in the PR, with the addition of all those OpenACC tests,
>> > libgomp make check times have skyrocketed since the testsuite is still
>> > run sequentially.
>
> ACK.  And, thanks for looking into that!
>
>> > Fixing this proved trivial: I managed to almost literally copy the
>> > solution from libstdc++-v3/testsuite/Makefile.am

So I recently had re-created this patch independently, before remembering
that Rainer had -- just eight years ago... ;-) -- already submitted this.
Fortunately, we've done it in almost the exact same way, just that a few
libgomp testsuite infrastructure as well as libstdc++ parallel testing
infrastructure changes (which is where these changes have been copied
from) have since gone in, which I've adjusted for.  Additionally, use
'check_DEJAGNU_libgomp_targets' instead of 'check_DEJAGNU_normal_targets'
etc. (where "normal" is a libstdc++ detail), and regarding:

>> > with a minimal change
>> > to libgomp.exp so the generated libgomp-test-support.exp file is found
>> > in both the sequential and parallel cases.  This isn't an issue in
>> > libstdc++ since all necessary variables are stored in a single
>> > site.exp.

... in 'libgomp/testsuite/lib/libgomp.exp', I've changed:

    -load_file libgomp-test-support.exp
    +# Search in both .. and . to support parallel and sequential testing.
    +load_file -1 ../libgomp-test-support.exp libgomp-test-support.exp

... into the more explicit:

    -load_file libgomp-test-support.exp
    +# Search in '..' vs. '.' to support parallel vs. sequential testing.
    +if [info exists ::env(GCC_RUNTEST_PARALLELIZE_DIR)] {
    +    load_file ../libgomp-test-support.exp
    +} else {
    +    load_file libgomp-test-support.exp
    +}

And, for now, I hard-code the number of parallel slots to one.  This
means that libgomp for 'make -j' now does use the parallel testing code
paths, but is restricted to just one slot.  That is, no actual change in
behavior, other than that 'libgomp.sum' then is filtered through
'contrib/dg-extract-results.sh'.

OK to push the attached
"Support parallel testing in libgomp, part I [PR66005]"?


Grüße
 Thomas


>> It is far from trivial though.
>> The point is that most of the OpenMP tests are parallelized with the
>> default OMP_NUM_THREADS, so running the tests in parallel oversubscribes the
>> machine a lot, the higher number of hw threads the more.
>
> Do you agree that we have two classes of test cases in libgomp: 1) test
> cases that don't place a considerably higher load on the machine compared
> to "normal" (single-threaded) execution tests, because they're just
> testing some functionality that is not expected to actively depend
> on/interfere with parallelism.  If needed, and/or if not already done,
> such test cases can be parameterized (OMP_NUM_THREADS, OpenACC num_gangs,
> num_workers, vector_length clauses, and so on) for low parallelism
> levels.  And, 2) test cases that place a considerably higher load on the
> machine compared to "normal" (single-threaded) execution tests, because
> they're testing some functionality that actively depends on/interferes
> with some kind of parallelism.  What about marking such tests specially,
> such that DejaGnu will only ever schedule one of them for execution at
> the same time?  For example, a new dg-* directive to run them wrapped
> through »flock [libgomp/testsuite/serial.lock] [a.out]« or some such?
>
>> If we go forward with some parallelization of the tests, we at least should
>> try to export something like OMP_WAIT_POLICY=passive so that the
>> oversubscribed machine would at least not spend too much time in spinning.
>
> (Will again have the problem that DejaGnu doesn't provide infrastructure
> to communicate environment variables to boards in remote testing.)
>
>> And perhaps reconsider running all OpenACC threads 3 times, just allow
>> user to select which offloading target they want to test (host fallback,
>> the host nonshm hack, PTX, XeonPHI in the future?), and test just that
>> (that is pretty much how OpenMP offloading testing works).
>
> My rationale is: if you configure GCC to support a set of offloading
> devices (more than one), you'll also want to get the test coverage that
> indeed all these work as expected.  (It currently doesn't matter, but...)
> that's something I'd like to see improved in the libgomp OpenMP
> offloading testing (once it supports more than one architecture for
> offloading).
>
>> For tests that
>> always want to test host fallback, I hope OpenACC offers clauses to force
>> the host fallback.
>
> Yes.
>
>
> Grüße,
>  Thomas


-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-parallel-testing-in-libgomp-part-I-PR66005.patch --]
[-- Type: text/x-diff, Size: 10736 bytes --]

From ec0b945d2a24524b987e2e0324d3bd86d9d554bd Mon Sep 17 00:00:00 2001
From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
Date: Thu, 7 May 2015 13:26:57 +0200
Subject: [PATCH] Support parallel testing in libgomp, part I [PR66005]

..., while still hard-coding the number of parallel slots to one.

	PR libgomp/66005
	libgomp/
	* testsuite/Makefile.am (PWD_COMMAND): New variable.
	(%/site.exp): New target.
	(check_p_numbers0, check_p_numbers1, check_p_numbers2)
	(check_p_numbers3, check_p_numbers4, check_p_numbers5)
	(check_p_numbers6, check_p_numbers, gcc_test_parallel_slots)
	(check_p_subdirs)
	(check_DEJAGNU_libgomp_targets): New variables.
	($(check_DEJAGNU_libgomp_targets)): New target.
	($(check_DEJAGNU_libgomp_targets)): New dependency.
	(check-DEJAGNU $(check_DEJAGNU_libgomp_targets)): New targets.
	* testsuite/Makefile.in: Regenerate.
	* testsuite/lib/libgomp.exp: For parallel testing,
	'load_file ../libgomp-test-support.exp'.

Co-authored-by: Thomas Schwinge <thomas@codesourcery.com>
---
 libgomp/testsuite/Makefile.am     | 78 ++++++++++++++++++++++++++-----
 libgomp/testsuite/Makefile.in     | 76 +++++++++++++++++++++++++-----
 libgomp/testsuite/lib/libgomp.exp |  7 ++-
 3 files changed, 138 insertions(+), 23 deletions(-)

diff --git a/libgomp/testsuite/Makefile.am b/libgomp/testsuite/Makefile.am
index 655a413c160..eef02f4a8c2 100644
--- a/libgomp/testsuite/Makefile.am
+++ b/libgomp/testsuite/Makefile.am
@@ -12,6 +12,8 @@ _RUNTEST = $(shell if test -f $(top_srcdir)/../dejagnu/runtest; then \
 	     echo $(top_srcdir)/../dejagnu/runtest; else echo runtest; fi)
 RUNTESTDEFAULTFLAGS = --tool $$tool --srcdir $$srcdir
 
+PWD_COMMAND = $${PWDCMD-pwd}
+
 EXTRA_DEJAGNU_SITE_CONFIG = libgomp-site-extra.exp
 
 # Instead of directly in ../testsuite/libgomp-test-support.exp.in, the
@@ -25,17 +27,6 @@ libgomp-test-support.exp: libgomp-test-support.pt.exp Makefile
 	  'set offload_additional_lib_paths "$(offload_additional_lib_paths)"'
 	mv $@.tmp $@
 
-check-DEJAGNU: site.exp
-	srcdir='$(srcdir)'; export srcdir; \
-	EXPECT=$(EXPECT); export EXPECT; \
-	if $(SHELL) -c "$(_RUNTEST) --version" > /dev/null 2>&1; then \
-	  exit_status=0; l='$(PACKAGE)'; for tool in $$l; do \
-	    if $(_RUNTEST) $(AM_RUNTESTFLAGS) $(RUNTESTDEFAULTFLAGS) $(RUNTESTFLAGS); \
-	    then :; else exit_status=1; fi; \
-	  done; \
-	else echo "WARNING: could not find '$(_RUNTEST)'" 1>&2; :;\
-	fi; \
-	exit $$exit_status
 site.exp: Makefile $(EXTRA_DEJAGNU_SITE_CONFIG)
 	@echo 'Making a new site.exp file ...'
 	@echo '## these variables are automatically generated by make ##' >site.tmp
@@ -63,6 +54,71 @@ site.exp: Makefile $(EXTRA_DEJAGNU_SITE_CONFIG)
 	@test ! -f site.exp || mv site.exp site.bak
 	@mv site.tmp site.exp
 
+%/site.exp: site.exp
+	-@test -d $* || mkdir $*
+	@srcdir=`cd $(srcdir); ${PWD_COMMAND}`;
+	@objdir=`${PWD_COMMAND}`/$*; \
+	sed -e "s|^set srcdir .*$$|set srcdir $$srcdir|" \
+	    -e "s|^set objdir .*$$|set objdir $$objdir|" \
+	    site.exp > $*/site.exp.tmp
+	@-rm -f $*/site.bak
+	@test ! -f $*/site.exp || mv $*/site.exp $*/site.bak
+	@mv $*/site.exp.tmp $*/site.exp
+
+check_p_numbers0:=1 2 3 4 5 6 7 8 9
+check_p_numbers1:=0 $(check_p_numbers0)
+check_p_numbers2:=$(foreach i,$(check_p_numbers0),$(addprefix $(i),$(check_p_numbers1)))
+check_p_numbers3:=$(addprefix 0,$(check_p_numbers1)) $(check_p_numbers2)
+check_p_numbers4:=$(foreach i,$(check_p_numbers0),$(addprefix $(i),$(check_p_numbers3)))
+check_p_numbers5:=$(addprefix 0,$(check_p_numbers3)) $(check_p_numbers4)
+check_p_numbers6:=$(foreach i,$(check_p_numbers0),$(addprefix $(i),$(check_p_numbers5)))
+check_p_numbers:=$(check_p_numbers0) $(check_p_numbers2) $(check_p_numbers4) $(check_p_numbers6)
+gcc_test_parallel_slots:=1
+check_p_subdirs=$(wordlist 1,$(gcc_test_parallel_slots),$(check_p_numbers))
+check_DEJAGNU_libgomp_targets = $(addprefix check-DEJAGNUlibgomp,$(check_p_subdirs))
+$(check_DEJAGNU_libgomp_targets): check-DEJAGNUlibgomp%: libgomp%/site.exp
+
+check-DEJAGNU $(check_DEJAGNU_libgomp_targets): check-DEJAGNU%: site.exp
+	$(if $*,@)AR="$(AR)"; export AR; \
+	RANLIB="$(RANLIB)"; export RANLIB; \
+	if [ -z "$*" ] && [ -n "$(filter -j%, $(MFLAGS))" ]; then \
+	  rm -rf libgomp-parallel || true; \
+	  mkdir libgomp-parallel; \
+	  $(MAKE) $(AM_MAKEFLAGS) $(check_DEJAGNU_libgomp_targets); \
+	  rm -rf libgomp-parallel || true; \
+	  for idx in $(check_p_subdirs); do \
+	    if [ -d libgomp$$idx ]; then \
+	      mv -f libgomp$$idx/libgomp.sum libgomp$$idx/libgomp.sum.sep; \
+	      mv -f libgomp$$idx/libgomp.log libgomp$$idx/libgomp.log.sep; \
+	    fi; \
+	  done; \
+	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh \
+	    libgomp[0-9]*/libgomp.sum.sep > libgomp.sum; \
+	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L \
+	    libgomp[0-9]*/libgomp.log.sep > libgomp.log; \
+	  exit 0; \
+	fi; \
+	srcdir=`$(am__cd) $(srcdir) && pwd`; export srcdir; \
+	EXPECT=$(EXPECT); export EXPECT; \
+	runtest=$(_RUNTEST); \
+	if [ -z "$$runtest" ]; then runtest=runtest; fi; \
+	tool=libgomp; \
+	if [ -n "$*" ]; then \
+	  if [ -f libgomp-parallel/finished ]; then rm -rf "$*"; exit 0; fi; \
+	  GCC_RUNTEST_PARALLELIZE_DIR=`${PWD_COMMAND}`/libgomp-parallel; \
+	  export GCC_RUNTEST_PARALLELIZE_DIR; \
+	  cd "$*"; \
+	fi; \
+	if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \
+	  $$runtest $(AM_RUNTESTFLAGS) $(RUNTESTDEFAULTFLAGS) \
+		    $(RUNTESTFLAGS); \
+	  if [ -n "$*" ]; then \
+	    touch $$GCC_RUNTEST_PARALLELIZE_DIR/finished; \
+	  fi; \
+	else \
+	  echo "WARNING: could not find \`runtest'" 1>&2; :;\
+	fi
+
 distclean-DEJAGNU:
 	-rm -f site.exp site.bak
 	-l='$(PACKAGE)'; for tool in $$l; do \
diff --git a/libgomp/testsuite/Makefile.in b/libgomp/testsuite/Makefile.in
index 7a88f0fe5c6..3b5903daed9 100644
--- a/libgomp/testsuite/Makefile.in
+++ b/libgomp/testsuite/Makefile.in
@@ -299,7 +299,19 @@ _RUNTEST = $(shell if test -f $(top_srcdir)/../dejagnu/runtest; then \
 	     echo $(top_srcdir)/../dejagnu/runtest; else echo runtest; fi)
 
 RUNTESTDEFAULTFLAGS = --tool $$tool --srcdir $$srcdir
+PWD_COMMAND = $${PWDCMD-pwd}
 EXTRA_DEJAGNU_SITE_CONFIG = libgomp-site-extra.exp
+check_p_numbers0 := 1 2 3 4 5 6 7 8 9
+check_p_numbers1 := 0 $(check_p_numbers0)
+check_p_numbers2 := $(foreach i,$(check_p_numbers0),$(addprefix $(i),$(check_p_numbers1)))
+check_p_numbers3 := $(addprefix 0,$(check_p_numbers1)) $(check_p_numbers2)
+check_p_numbers4 := $(foreach i,$(check_p_numbers0),$(addprefix $(i),$(check_p_numbers3)))
+check_p_numbers5 := $(addprefix 0,$(check_p_numbers3)) $(check_p_numbers4)
+check_p_numbers6 := $(foreach i,$(check_p_numbers0),$(addprefix $(i),$(check_p_numbers5)))
+check_p_numbers := $(check_p_numbers0) $(check_p_numbers2) $(check_p_numbers4) $(check_p_numbers6)
+gcc_test_parallel_slots := 1
+check_p_subdirs = $(wordlist 1,$(gcc_test_parallel_slots),$(check_p_numbers))
+check_DEJAGNU_libgomp_targets = $(addprefix check-DEJAGNUlibgomp,$(check_p_subdirs))
 all: all-am
 
 .SUFFIXES:
@@ -474,17 +486,6 @@ libgomp-test-support.exp: libgomp-test-support.pt.exp Makefile
 	  'set offload_additional_lib_paths "$(offload_additional_lib_paths)"'
 	mv $@.tmp $@
 
-check-DEJAGNU: site.exp
-	srcdir='$(srcdir)'; export srcdir; \
-	EXPECT=$(EXPECT); export EXPECT; \
-	if $(SHELL) -c "$(_RUNTEST) --version" > /dev/null 2>&1; then \
-	  exit_status=0; l='$(PACKAGE)'; for tool in $$l; do \
-	    if $(_RUNTEST) $(AM_RUNTESTFLAGS) $(RUNTESTDEFAULTFLAGS) $(RUNTESTFLAGS); \
-	    then :; else exit_status=1; fi; \
-	  done; \
-	else echo "WARNING: could not find '$(_RUNTEST)'" 1>&2; :;\
-	fi; \
-	exit $$exit_status
 site.exp: Makefile $(EXTRA_DEJAGNU_SITE_CONFIG)
 	@echo 'Making a new site.exp file ...'
 	@echo '## these variables are automatically generated by make ##' >site.tmp
@@ -512,6 +513,59 @@ site.exp: Makefile $(EXTRA_DEJAGNU_SITE_CONFIG)
 	@test ! -f site.exp || mv site.exp site.bak
 	@mv site.tmp site.exp
 
+%/site.exp: site.exp
+	-@test -d $* || mkdir $*
+	@srcdir=`cd $(srcdir); ${PWD_COMMAND}`;
+	@objdir=`${PWD_COMMAND}`/$*; \
+	sed -e "s|^set srcdir .*$$|set srcdir $$srcdir|" \
+	    -e "s|^set objdir .*$$|set objdir $$objdir|" \
+	    site.exp > $*/site.exp.tmp
+	@-rm -f $*/site.bak
+	@test ! -f $*/site.exp || mv $*/site.exp $*/site.bak
+	@mv $*/site.exp.tmp $*/site.exp
+$(check_DEJAGNU_libgomp_targets): check-DEJAGNUlibgomp%: libgomp%/site.exp
+
+check-DEJAGNU $(check_DEJAGNU_libgomp_targets): check-DEJAGNU%: site.exp
+	$(if $*,@)AR="$(AR)"; export AR; \
+	RANLIB="$(RANLIB)"; export RANLIB; \
+	if [ -z "$*" ] && [ -n "$(filter -j%, $(MFLAGS))" ]; then \
+	  rm -rf libgomp-parallel || true; \
+	  mkdir libgomp-parallel; \
+	  $(MAKE) $(AM_MAKEFLAGS) $(check_DEJAGNU_libgomp_targets); \
+	  rm -rf libgomp-parallel || true; \
+	  for idx in $(check_p_subdirs); do \
+	    if [ -d libgomp$$idx ]; then \
+	      mv -f libgomp$$idx/libgomp.sum libgomp$$idx/libgomp.sum.sep; \
+	      mv -f libgomp$$idx/libgomp.log libgomp$$idx/libgomp.log.sep; \
+	    fi; \
+	  done; \
+	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh \
+	    libgomp[0-9]*/libgomp.sum.sep > libgomp.sum; \
+	  $(SHELL) $(srcdir)/../../contrib/dg-extract-results.sh -L \
+	    libgomp[0-9]*/libgomp.log.sep > libgomp.log; \
+	  exit 0; \
+	fi; \
+	srcdir=`$(am__cd) $(srcdir) && pwd`; export srcdir; \
+	EXPECT=$(EXPECT); export EXPECT; \
+	runtest=$(_RUNTEST); \
+	if [ -z "$$runtest" ]; then runtest=runtest; fi; \
+	tool=libgomp; \
+	if [ -n "$*" ]; then \
+	  if [ -f libgomp-parallel/finished ]; then rm -rf "$*"; exit 0; fi; \
+	  GCC_RUNTEST_PARALLELIZE_DIR=`${PWD_COMMAND}`/libgomp-parallel; \
+	  export GCC_RUNTEST_PARALLELIZE_DIR; \
+	  cd "$*"; \
+	fi; \
+	if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \
+	  $$runtest $(AM_RUNTESTFLAGS) $(RUNTESTDEFAULTFLAGS) \
+		    $(RUNTESTFLAGS); \
+	  if [ -n "$*" ]; then \
+	    touch $$GCC_RUNTEST_PARALLELIZE_DIR/finished; \
+	  fi; \
+	else \
+	  echo "WARNING: could not find \`runtest'" 1>&2; :;\
+	fi
+
 distclean-DEJAGNU:
 	-rm -f site.exp site.bak
 	-l='$(PACKAGE)'; for tool in $$l; do \
diff --git a/libgomp/testsuite/lib/libgomp.exp b/libgomp/testsuite/lib/libgomp.exp
index e12236e9083..4ea6f26c674 100644
--- a/libgomp/testsuite/lib/libgomp.exp
+++ b/libgomp/testsuite/lib/libgomp.exp
@@ -42,7 +42,12 @@ load_gcc_lib torture-options.exp
 load_gcc_lib fortran-modules.exp
 
 # Try to load a test support file, built during libgomp configuration.
-load_file libgomp-test-support.exp
+# Search in '..' vs. '.' to support parallel vs. sequential testing.
+if [info exists ::env(GCC_RUNTEST_PARALLELIZE_DIR)] {
+    load_file ../libgomp-test-support.exp
+} else {
+    load_file libgomp-test-support.exp
+}
 
 set dg-do-what-default run
 
-- 
2.34.1


  parent reply	other threads:[~2023-05-05  8:55 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-07 11:27 [libgomp, testsuite] Support parallel testing in libgomp (PR libgomp/66005) Rainer Orth
2015-05-07 11:39 ` Jakub Jelinek
2015-05-07 18:07   ` Mike Stump
2015-05-08  8:40   ` Thomas Schwinge
2018-08-14  8:37     ` Martin Liška
2023-05-05  8:55     ` Thomas Schwinge [this message]
2023-05-05  8:59       ` Support parallel testing in libgomp, part II [PR66005] Thomas Schwinge
2023-05-09 17:27         ` Bernhard Reutner-Fischer
2023-05-16 14:32         ` Support parallel testing in libgomp: fallback Perl 'flock' [PR66005] Thomas Schwinge
2023-05-06 14:15       ` Support parallel testing in libgomp, part I [PR66005] Bernhard Reutner-Fischer
2023-05-08 10:42         ` Thomas Schwinge
2023-05-08 17:52           ` Bernhard Reutner-Fischer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h6srb1iq.fsf@euler.schwinge.homeip.net \
    --to=thomas@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hubicka@ucw.cz \
    --cc=jakub@redhat.com \
    --cc=mikestump@comcast.net \
    --cc=rep.dot.nop@gmail.com \
    --cc=ro@CeBiTec.Uni-Bielefeld.DE \
    --cc=segher@kernel.crashing.org \
    --cc=tobias@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).