Re: [PATCH v3] Run tests only if the machine supports the instruction set.

2016-12-20 Thread Dominik Vogt
On Tue, Dec 20, 2016 at 11:32:58AM +0100, Jakub Jelinek wrote:
> On Tue, Dec 20, 2016 at 11:22:47AM +0100, Dominik Vogt wrote:
> > On Mon, Dec 19, 2016 at 06:00:21PM +0100, Jakub Jelinek wrote:
> > > On Mon, Dec 19, 2016 at 05:50:40PM +0100, Dominik Vogt wrote:
> > > > * config/s390/s390-c.c (s390_cpu_cpp_builtins_internal): Define
> > > > __S390_ARCH_LEVEL__.
> > > > gcc/testsuite/ChangeLog-setmem
> > > > 
> > > > * gcc.target/s390/md/setmem_long-1.c: Use "runnable".
> > > > * gcc.target/s390/md/rXsbg_mode_sXl.c: Likewise.
> > > > * gcc.target/s390/md/andc-splitter-1.c: Likewise.
> > > > * gcc.target/s390/md/andc-splitter-2.c: Likewise.
> > > > * lib/gcc-dg.exp (gcc-dg-runtest): Export torture_current_flags.
> > > > * gcc.target/s390/s390.exp: Import torture_current_flags.
> > > > (check_effective_target_runnable): New.
> > > 
> > > Unless you want to add support for all targets in the runnable
> > > effective target, I think it would be better to call it less generically,
> > > s390_runnable or similar.

Done.

> > What do you think about the change in gcc-dg.exp?
> > 
> > We couldn't
> > decide whether it's a valid way of retrieving the flags needed for
> > compiling s390_check_runnable or not.  It would be nice to get all
> > options that are relevant for the test case, including the ones
> > from "dg-options" (etc.), but that probably requires larger
> > changes to lib/*.exp.  (The target specific check functions could
> > be removed then.)
> 
> I'm not a testsuite maintainer nor very good in tcl, so I think you want a
> testsuite maintainer to ack it in any case.
> But, I'd say you want something that will not be terribly expensive.
> If you have an effective target that happens to get flags from the
> current test, then that is necessarily non-cacheable, which would mean
> in addition to compiling every test you also compile another proglet for it.
> I think your current patch does that too, there is no caching, so it would
> be desirable to cache the results; you should invalidate those caches when
> torture_current_flags change.  See e.g. et_cache uses in
> target-supports.exp.  You want to remember torture_current_flags for which
> you've computed the s390_runnable et and if it changes, reset the cache.
> The other *_runnable flags can be probably just normally cached (and you do,
> by using check_runtime rather than check_runtime_nocache).

Right, it gets called even more often than one would think, and
even with empty torture_current_options.  The attached new patch
(v3) removes -Ox options and superflous whitespace and caches that
between calls if it's not empty.  There's another, permanent cache
for calls without any flags.  With proper ordering of the torture
options, the test program is built only a couple of times.

v3:

  * Cache test results.
  * Reorder torture tests for better caching.
  * Add ".machinemode zarch" to assembly file because the $flags
are overridden by the board options.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany
gcc/ChangeLog-archlevel

* config/s390/s390-c.c (s390_cpu_cpp_builtins_internal): Define
__S390_ARCH_LEVEL__.
gcc/testsuite/ChangeLog-setmem

* gcc.target/s390/md/setmem_long-1.c: Use "s390_runnable".
* gcc.target/s390/md/rXsbg_mode_sXl.c: Likewise.
* gcc.target/s390/md/andc-splitter-1.c: Likewise.
* gcc.target/s390/md/andc-splitter-2.c: Likewise.
* lib/gcc-dg.exp (gcc-dg-runtest): Export torture_current_flags.
* gcc.target/s390/s390.exp: Import torture_current_flags.
(check_effective_target_s390_runnable): New.
(check_effective_target_z900_runnable): New.
(check_effective_target_z990_runnable): New.
(check_effective_target_z9_ec_runnable): New.
(check_effective_target_z10_runnable): New.
(check_effective_target_z196_runnable): New.
(check_effective_target_zEC12_runnable): New.
(check_effective_target_z13_runnable): New.
(check_effective_target_z10_instructions): Removed.
(MD_TEST_OPTS): Add optimization level without -march=.
>From 4bd55b91b0487590bc9c9bde60664e5d94d2dc08 Mon Sep 17 00:00:00 2001
From: Dominik Vogt 
Date: Tue, 13 Dec 2016 10:21:08 +0100
Subject: [PATCH] S/390: Run md tests only if the machine supports the
 instruction set.

---
 gcc/config/s390/s390-c.c   |  17 +++
 gcc/testsuite/gcc.target/s390/md/andc-splitter-1.c |  19 +--
 gcc/testsuite/gcc.target/s390/md/andc-splitter-2.c |  19 +--
 gcc/testsuite/gcc.target/s390/md/rXsbg_mode_sXl.c  |   4 +-
 gcc/testsuite/gcc.target/s390/md/setmem_long-1.c   |   7 +-
 gcc/testsuite/gcc.target/s390/s390.exp | 170 ++---
 gcc/testsuite/lib/gcc-dg.exp   |   2 +
 7 files changed, 198 insertions(+), 40 deletions(-)

diff --git a/gcc/config/s390/s390-c.c b/gcc/config/s390/s390-c.c
index fcf7477..e841365 100644
---

Re: [PATCH v3] Run tests only if the machine supports the instruction set.

2016-12-20 Thread Mike Stump
On Dec 20, 2016, at 6:10 AM, Dominik Vogt  wrote:
> Right, it gets called even more often than one would think, and
> even with empty torture_current_options.  The attached new patch
> (v3) removes -Ox options and superflous whitespace and caches that
> between calls if it's not empty.  There's another, permanent cache
> for calls without any flags.  With proper ordering of the torture
> options, the test program is built only a couple of times.

Seems fine to me, but most other cases use the postfix _hw.  Any reason not use 
use _hw (and not _runable) on these?  If not, could you please use _hw instead.