[PATCH 00/12] A little TLC for the simulators (in particular CRIS)

[PATCH 00/12] A little TLC for the simulators (in particular CRIS)

[Hans-Peter Nilsson via Gdb-patches]

It's been a while since my last simulator commit, and it seems
cris-sim has broken since then.  I started pursuing a memory
framework bug and a 2.5x performance regression (found largely
salvageable by --disable-sim-hardware).  This set of commit
takes it and the testsuite back to a state usable for my gcc
autotester and (in part at least) for use in autotesting itself.
Hopefully more to come soonish (no promises) as the "linux" part
of the test-suite is still broken, though the "newlib" parts are
fixed.  Committed; general parts as authorized committer.

  sim cris: Correct PRIu32 to PRIx32
  sim/testsuite/cris: Assembler testcase for PRIx32 usage bug
  sim/testsuite: Set global_cc_os also when no compiler is found
  sim/testsuite/cris/c: Use -sim3 but only for newlib targets
  sim/testsuite/cris/hw/rv-n-cris/irq1.ms: Disable due to randomness
  sim/testsuite: Support "requires: simoption <--name-of-option>"
  sim/testsuite/cris: As applicable, require simoption --cris-900000xx
  sim cris: Unbreak --disable-sim-hardware builds
  sim: Fix use of out-of-tree assembler and linker when testing
  sim: Add sim_dump_memory for debugging
  sim/testsuite/cris: Remove faulty use of basename in C tests
  sim/testsuite/cris: If failing compilation, mark C tests as errors

 sim/Makefile.in                         |   7 +-
 sim/common/sim-memopt.c                 |  10 +
 sim/configure                           | 356 +++++++++++++++++-------
 sim/cris/sim-if.c                       |  10 +-
 sim/m4/sim_ac_toolchain.m4              |  21 +-
 sim/testsuite/cris/asm/endmem1.ms       |  47 ++++
 sim/testsuite/cris/asm/io1.ms           |   1 +
 sim/testsuite/cris/asm/io2.ms           |   1 +
 sim/testsuite/cris/asm/io3.ms           |   1 +
 sim/testsuite/cris/asm/io6.ms           |   1 +
 sim/testsuite/cris/asm/io7.ms           |   1 +
 sim/testsuite/cris/c/c.exp              |  18 +-
 sim/testsuite/cris/c/openpf1.c          |   8 +-
 sim/testsuite/cris/c/stat3.c            |   3 +-
 sim/testsuite/cris/hw/rv-n-cris/irq1.ms |   1 +
 sim/testsuite/lib/sim-defs.exp          |  61 ++++
 16 files changed, 431 insertions(+), 116 deletions(-)
 create mode 100644 sim/testsuite/cris/asm/endmem1.ms

-- 
2.30.2

brgds, H-P

[PATCH 01/12] sim cris: Correct PRIu32 to PRIx32

[Hans-Peter Nilsson via Gdb-patches]

In 5ee0bc23a68f "sim: clean up bfd_vma printing" there was
an additional introduction of PRIx32 and PRIu32 but just in
sim/cris/sim-if.c.  One type of bug was fixed in commit
d16ce6e4d581 "sim: cris: fix memory setup typos" but one
remained; the PRIu32 usage is wrong, as hex output is
desired; note the 0x prefix.

Without this fix, you'll see output like:
 memory map 0:0x4000..0x5fff (8192 bytes) overlaps 0:0x0..0x16383 (91012 bytes)
 program stopped with signal 6 (Aborted).
for some C programs, like some of the ones in the sim/cris/c
testsuite from where the example is taken (freopen2.c).

The bug behavior was with memory allocation.  With an
attempt to allocate memory using the brk syscall such that
the room up to the next 8192-byte "page boundary" wasn't
sufficient, the simulator memory allocation machinery horked
on a consistency error when trying to allocate a memory
block to raise the "end of the data segment": there was
already memory allocated at that address.

Unfortunately, none of the programs in sim/cris/asm exposed
this bug at the time, but an assembler test-case is
committed after this fix.

sim/cris:
	* sim-if.c (sim_open): Correct PRIu32 to PRIx32.
---
 sim/cris/sim-if.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sim/cris/sim-if.c b/sim/cris/sim-if.c
index 602db9aebf4b..63deb467bc07 100644
--- a/sim/cris/sim-if.c
+++ b/sim/cris/sim-if.c
@@ -887,7 +887,7 @@ sim_open (SIM_OPEN_KIND kind, host_callback *callback, struct bfd *abfd,
 
   /* Allocate core managed memory if none specified by user.  */
   if (sim_core_read_buffer (sd, NULL, read_map, &c, startmem, 1) == 0)
-    sim_do_commandf (sd, "memory region 0x%" PRIx32 ",0x%" PRIu32,
+    sim_do_commandf (sd, "memory region 0x%" PRIx32 ",0x%" PRIx32,
 		     startmem, endmem - startmem);
 
   /* Allocate simulator I/O managed memory if none specified by user.  */
-- 
2.30.2

[PATCH 03/12] sim/testsuite: Set global_cc_os also when no compiler is found

[Hans-Peter Nilsson via Gdb-patches]

If we don't set this variable, it doesn't exist, and using "#progos:"
in an assembler-file will cause an error rather than just skipping the
test, viz:

Running /src/sim/testsuite/cris/hw/rv-n-cris/rvc.exp ...
ERROR: tcl error sourcing /src/sim/testsuite/cris/hw/rv-n-cris/rvc.exp.
ERROR: can't read "global_cc_os": no such variable
    while executing
"if { $opts(progos) != "" && $opts(progos) != $global_cc_os } {
	untested $subdir/$name
	return
    }"
    (procedure "run_sim_test" line 102)

Neither the commit introducing progos, nor the top comment
in run_sim_test, mentions progos as intended only for C
tests, or that its use must be gated on $global_cc_works !=
0, so (not) setting it in the no-working-compiler path seems
just overlooked.

Allowing it to be used for assembler tests makes it usable
for e.g. an always-false predicate and in expressions in
.exp files without gating on $global_cc_works != 0.

With this patch, global_cc_os is set to "", just as for "unknown OS".

    sim/testsuite:
	* lib/sim-defs.exp (sim_init_toolchain): Set global_cc_os also when
	no working target C compiler is found.
---
 sim/testsuite/lib/sim-defs.exp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sim/testsuite/lib/sim-defs.exp b/sim/testsuite/lib/sim-defs.exp
index 2cf739b3e32e..3586fa550765 100644
--- a/sim/testsuite/lib/sim-defs.exp
+++ b/sim/testsuite/lib/sim-defs.exp
@@ -142,6 +142,7 @@ proc sim_init_toolchain {} {
     } {
 	verbose -log "Can't execute C compiler"
 	set global_cc_works 0
+	set global_cc_os ""
     }
 
     file delete $objdir/compilercheck.x
-- 
2.30.2

[PATCH 04/12] sim/testsuite/cris/c: Use -sim3 but only for newlib targets

[Hans-Peter Nilsson via Gdb-patches]

Commit a39487c6685f "sim: cris: use -sim with C tests for cris-elf
targets" caused " -sim" to be appended to CFLAGS_FOR_TARGET for
cris*-*-elf, where testing had until then relied on
"RUNTESTFLAGS=--target_board=cris-sim" being passed when running "make
check-sim", adding the right options.  While "-sim" happens to work,
the baseboard-file cris-sim.exp uses "-sim3" so for consistency use
that instead.

Then commit b42f20d2ac72 "sim: testsuite: drop most specific istarget
checks" caused " -sim" to be appended for *all* targets, which just
doesn't work.  For example, for crisv32-linux-gnu, that's not a
recognized option and will cause a dejagnu error and further testing
in c.exp will be aborted.

While cris-sim.exp appends "-static" for *-linux-gnu, further changes
in the test-suite have caused "linux"-specific tests to break, so that
part will be tended to separately.

But, save and restore CFLAGS_FOR_TARGET around the modification and
use where needed, to not have the CRIS-specific modification affect a
continuing test-run (possibly for other targets).

sim/testsuite/cris:
	* c/c.exp (CFLAGS_FOR_TARGET): Replace appended option " -sim"
	with " -sim3", but do it conditionally for newlib targets.  Save
	and restore CFLAGS_FOR_TARGET in saved_CFLAGS_FOR_TARGET such
	that it doesn't affect the value of CFLAGS_FOR_TARGET outside
	c.exp.
---
 sim/testsuite/cris/c/c.exp | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/sim/testsuite/cris/c/c.exp b/sim/testsuite/cris/c/c.exp
index 5711fd2f0bcd..3e186e072d8e 100644
--- a/sim/testsuite/cris/c/c.exp
+++ b/sim/testsuite/cris/c/c.exp
@@ -17,6 +17,9 @@
 
 sim_init
 
+global global_cc_works
+global global_cc_os
+
 set CFLAGS_FOR_TARGET "-O2"
 if [istarget cris-*-*] {
     set mach "crisv10"
@@ -24,13 +27,16 @@ if [istarget cris-*-*] {
     set mach "crisv32"
 }
 
-if [istarget *] {
-    append CFLAGS_FOR_TARGET " -sim"
+# Make sure we're using the right runtime for simulator runs.  If the
+# cris-sim dejagnu baseboard is used, -sim3 will be duplicated, but
+# that's ok.  For e.g. cris*-linux-gnu, neither -sim not -sim3 are
+# supported options and likely not other targets too.
+set saved_CFLAGS_FOR_TARGET $CFLAGS_FOR_TARGET
+if { $global_cc_os == "newlib" } {
+    append CFLAGS_FOR_TARGET " -sim3"
 }
 
 # Using target_compile, since it is less noisy,
-global global_cc_works
-global global_cc_os
 if { $global_cc_works == 1 } {
     # Now check if we can link a program dynamically, and where
     # libc.so is located.  If it is, we provide a sym link to the
@@ -239,3 +245,5 @@ foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.c]] {
 	$status "$mach $testname"
     }
 }
+
+set CFLAGS_FOR_TARGET $saved_CFLAGS_FOR_TARGET
-- 
2.30.2

[PATCH 06/12] sim/testsuite: Support "requires: simoption <--name-of-option>"

[Hans-Peter Nilsson via Gdb-patches]

Simulator features can be present or not, typically
depending on different-valued configure options, like
--enable-sim-hardware[=off|=on].  To avoid failures in
test-suite-runs when testing such configurations, a new
predicate is needed, as neither "target", "progos" nor
"mach" fits cleanly.

The immediate need was to check for presence of a simulator
option, but rather than a specialized "requires-simoption:"
predicate I thought I'd handle the general (parametrized)
need, so here's a generic predicate machinery and a (first)
predicate to use together with it; checking whether a
particular option is supported, by looking at "run --help"
output.  This was inspired by the check_effective_target_
machinery in the gcc test-suite.

Multiple "requires: <requirement> <parameter>" form a list of
predicates (with parameters), to be used as a conjunction.

sim/testsuite:
	* lib/sim-defs.exp (sim_check_requires_simoption): New function.
	(run_sim_test): Support "requires: <requirement> <parameter>".
---
 sim/testsuite/lib/sim-defs.exp | 60 ++++++++++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/sim/testsuite/lib/sim-defs.exp b/sim/testsuite/lib/sim-defs.exp
index 3586fa550765..d2750e08b046 100644
--- a/sim/testsuite/lib/sim-defs.exp
+++ b/sim/testsuite/lib/sim-defs.exp
@@ -261,6 +261,41 @@ proc sim_run { prog sim_opts prog_opts redir options } {
     return [list $return_code $output]
 }
 
+# Support function for "#requires: simoption <xx>":
+# Looks in "run --help" output for <xx>, returns 1 iff <xx> is mentioned
+# there and looks like an option name, otherwise 0.
+
+proc sim_check_requires_simoption { optname } {
+    global sim_path
+    set testrun "$sim_path --help"
+    verbose -log "Checking for simoption `$optname'" 3
+    remote_spawn host $testrun
+    set result [remote_wait host 240]
+
+    set return_code [lindex $result 0]
+    if { $return_code != 0 } {
+	perror "Can't execute `$testrun' to check for `$optname'"
+	return 0
+    }
+
+    set output [lindex $result 1]
+    # Remove \r as for regular runs.
+    regsub -all -- "\r" $output "" output
+
+    # The option output format for --help for each line where an
+    # option name is mentioned, is assumed to be two space followed
+    # by the option name followed by a space or left square bracket,
+    # like in (optname=--foo): "  --foo " or "  --foo[this|that]".
+    # Beware not to match "  --foo-bar" nor "  --foobar".
+    if [string match "*\n  $optname\[\[ \]*" $output] {
+	verbose -log "Found `$optname'" 3
+	return 1
+    }
+    verbose -log "Did not find `$optname'" 3
+
+    return 0
+}
+
 # Run testcase NAME.
 # NAME is either a fully specified file name, or just the file name in which
 # case $srcdir/$subdir will be prepended.
@@ -326,6 +361,7 @@ proc run_sim_test { name requested_machs } {
     set opts(cc) ""
     set opts(progopts) ""
     set opts(progos) ""
+    set opts(requires) {}
     set opts(sim) ""
     set opts(status) "0"
     set opts(output) ""
@@ -375,6 +411,14 @@ proc run_sim_test { name requested_machs } {
 	    set opt_val "$opts($opt_name) $opt_val"
 	}
 
+	# Similar for "requires", except we append a pair to a list, and
+	# that doesn't match the processing in the rest of the loop, so we
+	# "continue" early.
+	if { $opt_name == "requires" } {
+	    lappend opts($opt_name) [split $opt_val " "]
+	    continue
+	}
+
 	foreach m $opt_machs {
 	    set opts($opt_name,$m) $opt_val
 	}
@@ -449,6 +493,22 @@ proc run_sim_test { name requested_machs } {
 	    set opts(cc,$mach) $opts(cc)
 	}
 
+	foreach req $opts(requires) {
+	    set what [lindex $req 0]
+	    set what_opt [lindex $req 1]
+	    verbose -log "requires: <$what> <$what_opt>"
+	    if { [info procs sim_check_requires_${what}] != [list] } {
+		if ![sim_check_requires_${what} $what_opt] {
+		    untested $subdir/$name
+		    return
+		}
+	    } {
+		perror "unknown requirement `requires: $what' in file $file"
+		unresolved $subdir/$name
+		return
+	    }
+	}
+
 	if [string match "*.c" $sourcefile] {
 	    # If we don't have a compiler available, skip tests :(.
 	    if { $global_cc_works == 0 } {
-- 
2.30.2

[PATCH 08/12] sim cris: Unbreak --disable-sim-hardware builds

[Hans-Peter Nilsson via Gdb-patches]

With --disable-sim-hardware (--enable-sim-hardware=no),
whose default was changed to --enable-sim-hardware(=yes) in
commit 34cf51120683, building for cris-elf fails as
sim_hw_parse then doesn't exist.

A cris-elf simulator configured for --enable-sim-hardware
(or the default after to the mentioned commit) runs about
2.5x slower than one configured --disable-sim-hardware.
A further 2-5% performance regression was not investigated.

When sim_hw_parse doesn't exist, --cris-900000xx can't be
supported.  The best action here is to remove it completely,
so its absence can be identified through --help, but
avoiding littering the code with "#if WITH_HW".

sim/cris:
	* sim-if.c (cris_options) [WITH_HW]: Conditionalize
	support of option --cris-900000xx.
	(sim_open) [WITH_HW]: Conditionalize sim_hw_parse
	call.
---
 sim/cris/sim-if.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sim/cris/sim-if.c b/sim/cris/sim-if.c
index 63deb467bc07..c72edc752b6f 100644
--- a/sim/cris/sim-if.c
+++ b/sim/cris/sim-if.c
@@ -100,9 +100,11 @@ static const OPTION cris_options[] =
   { {"cris-naked", no_argument, NULL, OPTION_CRIS_NAKED},
      '\0', NULL, "Don't set up stack and environment",
      cris_option_handler, NULL },
+#if WITH_HW
   { {"cris-900000xx", no_argument, NULL, OPTION_CRIS_900000XXIF},
      '\0', NULL, "Define addresses at 0x900000xx with simulator semantics",
      cris_option_handler, NULL },
+#endif
   { {"cris-unknown-syscall", required_argument, NULL,
      OPTION_CRIS_UNKNOWN_SYSCALL},
      '\0', "stop|enosys|enosys-quiet", "Action at an unknown system call",
@@ -891,8 +893,14 @@ sim_open (SIM_OPEN_KIND kind, host_callback *callback, struct bfd *abfd,
 		     startmem, endmem - startmem);
 
   /* Allocate simulator I/O managed memory if none specified by user.  */
+#if WITH_HW
   if (cris_have_900000xxif)
     sim_hw_parse (sd, "/core/%s/reg %#x %i", "cris_900000xx", 0x90000000, 0x100);
+#else
+  /* With the option disabled, nothing should be able to set this variable.
+     We should "use" it, though, and why not assert that it isn't set.  */
+  ASSERT (! cris_have_900000xxif);
+#endif
 
   /* Establish any remaining configuration options.  */
   if (sim_config (sd) != SIM_RC_OK)
-- 
2.30.2

[PATCH 09/12] sim: Fix use of out-of-tree assembler and linker when testing

[Hans-Peter Nilsson via Gdb-patches]

With commit 7a259895bb2d "sim: testsuite: expand arch specific
toolchain settings", trying to use out-of-tree ld and as at test-time
broke for the "primary target", like when testing a release-tarball.

Subsequent to that commit, all assembler tests without in-tree-built
tools FAIL, getting errors when trying to call
$(abs_builddir)/../gas/as-new.  But, that isn't the actual culprint;
it's actually it's its immediate predecessor, commit 8996c21067373
"sim: testsuite: setup per-port toolchain settings for multitarget
build", which hardcodes in-tree-paths to those tools instead of
considering e.g. $(<X>_FOR_TARGET), the preferred overridable variable
for single-target builds, as set up by the toplevel Makefile.

This commit calls GCC_TARGET_TOOL (a deceptive name; gcc-specific
features aren't used) from toplev/config/acx.m4, somewhat like calls
in toplev/configure.ac but without the NCN_STRICT_CHECK_TARGET_TOOLS
step, for each X to find a value for $(<X>_FOR_TARGET).  N.B.: in-tree
tools still override any ${target}-${tool} found in $PATH, i.e. only
previously broken builds are affected.

The variables $(<X>_FOR_TARGET) are usually overridden by the toplevel
Makefile to the same value or better, but has to be set here too, as
automake "wants" Makefiles to be self-contained (you get an error
pointing out that the variable may be empty).  If it hadn't been for
that, SIM_AC_CHECK_TOOLCHAIN_FOR_PRIMARY_TARGET would not be needed.
This detail should only (positively) affect users invoking "make
check" in sim/ instead of "make check-sim" (or "make check") at the
toplevel.  Now the output from "configure" matches the target tools
actually used by sim at test-time, for the "primary target".

Using $(CC) for "example-" targets CC_FOR_TARGET is not changed, as
that appears to be a deliberate special-case.

Note that all tools still have to be installed and present in
$PATH at configure-time to be properly used at test-time.

sim:
	* m4/sim_ac_toolchain.m4 (SIM_AC_CHECK_TOOLCHAIN_FOR_PRIMARY_TARGET):
	New defun.
	(SIM_TOOLCHAIN_VARS): Call it using AC_REQUIRE, and use variables
	AS_FOR_TARGET, LD_FOR_TARGET and CC_FOR_TARGET instead of hard-coded
	values.
	* Makefile.in, configure: Regenerate.
---
 sim/Makefile.in            |   7 +-
 sim/configure              | 356 +++++++++++++++++++++++++++----------
 sim/m4/sim_ac_toolchain.m4 |  21 ++-
 3 files changed, 278 insertions(+), 106 deletions(-)

diff --git a/sim/Makefile.in b/sim/Makefile.in
index 8b208e034430..49a3c0b80dbf 100644
--- a/sim/Makefile.in
+++ b/sim/Makefile.in
@@ -1,7 +1,7 @@
 # Makefile.in generated by automake 1.15.1 from Makefile.am.
 # @configure_input@
 
-# Copyright (C) 1994-2022 Free Software Foundation, Inc.
+# Copyright (C) 1994-2017 Free Software Foundation, Inc.
 
 # This Makefile.in is free software; the Free Software Foundation
 # gives unlimited permission to copy and/or distribute it,
@@ -14,7 +14,7 @@
 
 @SET_MAKE@
 
-#   Copyright (C) 1993-2021 Free Software Foundation, Inc.
+#   Copyright (C) 1993-2022 Free Software Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -652,6 +652,7 @@ AMTAR = @AMTAR@
 AM_DEFAULT_VERBOSITY = @AM_DEFAULT_VERBOSITY@
 AR = @AR@
 AR_FOR_BUILD = @AR_FOR_BUILD@
+AS_FOR_TARGET = @AS_FOR_TARGET@
 AS_FOR_TARGET_AARCH64 = @AS_FOR_TARGET_AARCH64@
 AS_FOR_TARGET_ARM = @AS_FOR_TARGET_ARM@
 AS_FOR_TARGET_AVR = @AS_FOR_TARGET_AVR@
@@ -693,6 +694,7 @@ CATOBJEXT = @CATOBJEXT@
 CC = @CC@
 CCDEPMODE = @CCDEPMODE@
 CC_FOR_BUILD = @CC_FOR_BUILD@
+CC_FOR_TARGET = @CC_FOR_TARGET@
 CC_FOR_TARGET_AARCH64 = @CC_FOR_TARGET_AARCH64@
 CC_FOR_TARGET_ARM = @CC_FOR_TARGET_ARM@
 CC_FOR_TARGET_AVR = @CC_FOR_TARGET_AVR@
@@ -757,6 +759,7 @@ INSTOBJEXT = @INSTOBJEXT@
 LD = @LD@
 LDFLAGS = @LDFLAGS@
 LDFLAGS_FOR_BUILD = @LDFLAGS_FOR_BUILD@
+LD_FOR_TARGET = @LD_FOR_TARGET@
 LD_FOR_TARGET_AARCH64 = @LD_FOR_TARGET_AARCH64@
 LD_FOR_TARGET_ARM = @LD_FOR_TARGET_ARM@
 LD_FOR_TARGET_AVR = @LD_FOR_TARGET_AVR@
diff --git a/sim/configure b/sim/configure
index 51ac74a4ce45..019c2f41cf42 100755
--- a/sim/configure
+++ b/sim/configure
@@ -813,6 +813,9 @@ SIM_ENABLE_ARCH_aarch64_TRUE
 CC_FOR_TARGET_AARCH64
 LD_FOR_TARGET_AARCH64
 AS_FOR_TARGET_AARCH64
+LD_FOR_TARGET
+AS_FOR_TARGET
+CC_FOR_TARGET
 SIM_COMMON_BUILD_FALSE
 SIM_COMMON_BUILD_TRUE
 SIM_SUBDIRS
@@ -12632,7 +12635,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 12635 "configure"
+#line 12638 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -12738,7 +12741,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 12741 "configure"
+#line 12744 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -14395,13 +14398,141 @@ if test "${enable_sim}" != no; then
 
       ;;
   esac
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking where to find the target cc" >&5
+$as_echo_n "checking where to find the target cc... " >&6; }
+if test "x${build}" != "x${host}" ; then
+  if expr "x$CC_FOR_TARGET" : "x/" > /dev/null; then
+    # We already found the complete path
+    ac_dir=`dirname $CC_FOR_TARGET`
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed in $ac_dir" >&5
+$as_echo "pre-installed in $ac_dir" >&6; }
+  else
+    # Canadian cross, just use what we found
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed" >&5
+$as_echo "pre-installed" >&6; }
+  fi
+else
+  ok=yes
+  case " ${configdirs} " in
+    *" ${target_alias}-gcc "*) ;;
+    *) ok=no ;;
+  esac
+
+  if test $ok = yes; then
+    # An in-tree tool is available and we can use it
+    CC_FOR_TARGET='$$r/$(HOST_SUBDIR)/${target_alias}-gcc'
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: just compiled" >&5
+$as_echo "just compiled" >&6; }
+  elif expr "x$CC_FOR_TARGET" : "x/" > /dev/null; then
+    # We already found the complete path
+    ac_dir=`dirname $CC_FOR_TARGET`
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed in $ac_dir" >&5
+$as_echo "pre-installed in $ac_dir" >&6; }
+  elif test "x$target" = "x$host"; then
+    # We can use an host tool
+    CC_FOR_TARGET='$(CC)'
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: host tool" >&5
+$as_echo "host tool" >&6; }
+  else
+    # We need a cross tool
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed" >&5
+$as_echo "pre-installed" >&6; }
+  fi
+fi
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking where to find the target as" >&5
+$as_echo_n "checking where to find the target as... " >&6; }
+if test "x${build}" != "x${host}" ; then
+  if expr "x$AS_FOR_TARGET" : "x/" > /dev/null; then
+    # We already found the complete path
+    ac_dir=`dirname $AS_FOR_TARGET`
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed in $ac_dir" >&5
+$as_echo "pre-installed in $ac_dir" >&6; }
+  else
+    # Canadian cross, just use what we found
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed" >&5
+$as_echo "pre-installed" >&6; }
+  fi
+else
+  ok=yes
+  case " ${configdirs} " in
+    *" \$(abs_builddir) "*) ;;
+    *) ok=no ;;
+  esac
+
+  if test $ok = yes; then
+    # An in-tree tool is available and we can use it
+    AS_FOR_TARGET='$$r/$(HOST_SUBDIR)/\$(abs_builddir)/../gas/as-new'
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: just compiled" >&5
+$as_echo "just compiled" >&6; }
+  elif expr "x$AS_FOR_TARGET" : "x/" > /dev/null; then
+    # We already found the complete path
+    ac_dir=`dirname $AS_FOR_TARGET`
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed in $ac_dir" >&5
+$as_echo "pre-installed in $ac_dir" >&6; }
+  elif test "x$target" = "x$host"; then
+    # We can use an host tool
+    AS_FOR_TARGET='$(AS)'
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: host tool" >&5
+$as_echo "host tool" >&6; }
+  else
+    # We need a cross tool
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed" >&5
+$as_echo "pre-installed" >&6; }
+  fi
+fi
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking where to find the target ld" >&5
+$as_echo_n "checking where to find the target ld... " >&6; }
+if test "x${build}" != "x${host}" ; then
+  if expr "x$LD_FOR_TARGET" : "x/" > /dev/null; then
+    # We already found the complete path
+    ac_dir=`dirname $LD_FOR_TARGET`
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed in $ac_dir" >&5
+$as_echo "pre-installed in $ac_dir" >&6; }
+  else
+    # Canadian cross, just use what we found
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed" >&5
+$as_echo "pre-installed" >&6; }
+  fi
+else
+  ok=yes
+  case " ${configdirs} " in
+    *" \$(abs_builddir) "*) ;;
+    *) ok=no ;;
+  esac
+
+  if test $ok = yes; then
+    # An in-tree tool is available and we can use it
+    LD_FOR_TARGET='$$r/$(HOST_SUBDIR)/\$(abs_builddir)/../ld/ld-new'
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: just compiled" >&5
+$as_echo "just compiled" >&6; }
+  elif expr "x$LD_FOR_TARGET" : "x/" > /dev/null; then
+    # We already found the complete path
+    ac_dir=`dirname $LD_FOR_TARGET`
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed in $ac_dir" >&5
+$as_echo "pre-installed in $ac_dir" >&6; }
+  elif test "x$target" = "x$host"; then
+    # We can use an host tool
+    LD_FOR_TARGET='$(LD)'
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: host tool" >&5
+$as_echo "host tool" >&6; }
+  else
+    # We need a cross tool
+    { $as_echo "$as_me:${as_lineno-$LINENO}: result: pre-installed" >&5
+$as_echo "pre-installed" >&6; }
+  fi
+fi
+
+
+
 
 
 
   if test "$SIM_PRIMARY_TARGET" = "aarch64"; then :
-      : "${AS_FOR_TARGET_AARCH64:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_AARCH64:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_AARCH64:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_AARCH64:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_AARCH64:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_AARCH64:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14438,10 +14569,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "arm"; then :
-      : "${AS_FOR_TARGET_ARM:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_ARM:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_ARM:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_ARM:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_ARM:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_ARM:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14478,10 +14610,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "avr"; then :
-      : "${AS_FOR_TARGET_AVR:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_AVR:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_AVR:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_AVR:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_AVR:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_AVR:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14518,10 +14651,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "bfin"; then :
-      : "${AS_FOR_TARGET_BFIN:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_BFIN:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_BFIN:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_BFIN:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_BFIN:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_BFIN:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14557,10 +14691,11 @@ subdirs="$subdirs bpf"
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "bpf"; then :
-      : "${AS_FOR_TARGET_BPF:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_BPF:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_BPF:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_BPF:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_BPF:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_BPF:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14597,10 +14732,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "cr16"; then :
-      : "${AS_FOR_TARGET_CR16:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_CR16:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_CR16:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_CR16:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_CR16:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_CR16:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14637,10 +14773,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "cris"; then :
-      : "${AS_FOR_TARGET_CRIS:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_CRIS:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_CRIS:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_CRIS:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_CRIS:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_CRIS:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14677,10 +14814,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "d10v"; then :
-      : "${AS_FOR_TARGET_D10V:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_D10V:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_D10V:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_D10V:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_D10V:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_D10V:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14717,10 +14855,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "frv"; then :
-      : "${AS_FOR_TARGET_FRV:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_FRV:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_FRV:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_FRV:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_FRV:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_FRV:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14757,10 +14896,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "ft32"; then :
-      : "${AS_FOR_TARGET_FT32:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_FT32:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_FT32:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_FT32:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_FT32:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_FT32:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14797,10 +14937,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "h8300"; then :
-      : "${AS_FOR_TARGET_H8300:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_H8300:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_H8300:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_H8300:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_H8300:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_H8300:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14837,10 +14978,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "iq2000"; then :
-      : "${AS_FOR_TARGET_IQ2000:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_IQ2000:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_IQ2000:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_IQ2000:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_IQ2000:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_IQ2000:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14877,10 +15019,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "lm32"; then :
-      : "${AS_FOR_TARGET_LM32:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_LM32:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_LM32:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_LM32:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_LM32:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_LM32:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14917,10 +15060,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "m32c"; then :
-      : "${AS_FOR_TARGET_M32C:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_M32C:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_M32C:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_M32C:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_M32C:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_M32C:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14957,10 +15101,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "m32r"; then :
-      : "${AS_FOR_TARGET_M32R:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_M32R:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_M32R:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_M32R:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_M32R:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_M32R:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -14997,10 +15142,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "m68hc11"; then :
-      : "${AS_FOR_TARGET_M68HC11:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_M68HC11:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_M68HC11:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_M68HC11:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_M68HC11:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_M68HC11:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15037,10 +15183,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "mcore"; then :
-      : "${AS_FOR_TARGET_MCORE:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_MCORE:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_MCORE:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_MCORE:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_MCORE:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_MCORE:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15077,10 +15224,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "microblaze"; then :
-      : "${AS_FOR_TARGET_MICROBLAZE:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_MICROBLAZE:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_MICROBLAZE:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_MICROBLAZE:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_MICROBLAZE:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_MICROBLAZE:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15114,10 +15262,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "mips"; then :
-      : "${AS_FOR_TARGET_MIPS:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_MIPS:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_MIPS:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_MIPS:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_MIPS:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_MIPS:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15151,10 +15300,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "mn10300"; then :
-      : "${AS_FOR_TARGET_MN10300:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_MN10300:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_MN10300:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_MN10300:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_MN10300:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_MN10300:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15191,10 +15341,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "moxie"; then :
-      : "${AS_FOR_TARGET_MOXIE:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_MOXIE:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_MOXIE:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_MOXIE:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_MOXIE:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_MOXIE:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15231,10 +15382,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "msp430"; then :
-      : "${AS_FOR_TARGET_MSP430:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_MSP430:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_MSP430:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_MSP430:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_MSP430:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_MSP430:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15268,10 +15420,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "or1k"; then :
-      : "${AS_FOR_TARGET_OR1K:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_OR1K:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_OR1K:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_OR1K:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_OR1K:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_OR1K:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15305,10 +15458,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "ppc"; then :
-      : "${AS_FOR_TARGET_PPC:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_PPC:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_PPC:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_PPC:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_PPC:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_PPC:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15345,10 +15499,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "pru"; then :
-      : "${AS_FOR_TARGET_PRU:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_PRU:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_PRU:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_PRU:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_PRU:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_PRU:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15382,10 +15537,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "riscv"; then :
-      : "${AS_FOR_TARGET_RISCV:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_RISCV:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_RISCV:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_RISCV:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_RISCV:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_RISCV:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15422,10 +15578,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "rl78"; then :
-      : "${AS_FOR_TARGET_RL78:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_RL78:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_RL78:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_RL78:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_RL78:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_RL78:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15462,10 +15619,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "rx"; then :
-      : "${AS_FOR_TARGET_RX:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_RX:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_RX:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_RX:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_RX:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_RX:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15502,10 +15660,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "sh"; then :
-      : "${AS_FOR_TARGET_SH:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_SH:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_SH:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_SH:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_SH:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_SH:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15542,10 +15701,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "erc32"; then :
-      : "${AS_FOR_TARGET_ERC32:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_ERC32:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_ERC32:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_ERC32:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_ERC32:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_ERC32:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15579,10 +15739,11 @@ fi
 
 
 
+
   if test "$SIM_PRIMARY_TARGET" = "v850"; then :
-      : "${AS_FOR_TARGET_V850:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_V850:=\$(abs_builddir)/../ld/ld-new}"
-            : "${CC_FOR_TARGET_V850:=${target_alias}-gcc}"
+      : "${AS_FOR_TARGET_V850:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_V850:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_V850:=\$(CC_FOR_TARGET)}"
 
 fi
 
@@ -15603,8 +15764,9 @@ fi
 
 
 
-  : "${AS_FOR_TARGET_EXAMPLE_SYNACOR:=\$(abs_builddir)/../gas/as-new}"
-  : "${LD_FOR_TARGET_EXAMPLE_SYNACOR:=\$(abs_builddir)/../ld/ld-new}"
+
+  : "${AS_FOR_TARGET_EXAMPLE_SYNACOR:=\$(AS_FOR_TARGET)}"
+  : "${LD_FOR_TARGET_EXAMPLE_SYNACOR:=\$(LD_FOR_TARGET)}"
   : "${CC_FOR_TARGET_EXAMPLE_SYNACOR:=\$(CC)}"
 
 as_fn_append SIM_TOOLCHAIN_VARS " AS_FOR_TARGET_EXAMPLE_SYNACOR LD_FOR_TARGET_EXAMPLE_SYNACOR CC_FOR_TARGET_EXAMPLE_SYNACOR"
diff --git a/sim/m4/sim_ac_toolchain.m4 b/sim/m4/sim_ac_toolchain.m4
index 09b8705c14b7..74532142929c 100644
--- a/sim/m4/sim_ac_toolchain.m4
+++ b/sim/m4/sim_ac_toolchain.m4
@@ -78,24 +78,31 @@ AC_COMPILE_IFELSE([AC_LANG_SOURCE([
 AC_SUBST(C_DIALECT)
 ])
 dnl
+
+AC_DEFUN([SIM_AC_CHECK_TOOLCHAIN_FOR_PRIMARY_TARGET],
+[dnl
+GCC_TARGET_TOOL([cc], [CC_FOR_TARGET], [CC], [${target_alias}-gcc])
+GCC_TARGET_TOOL([as], [AS_FOR_TARGET], [AS], [\$(abs_builddir)/../gas/as-new])
+GCC_TARGET_TOOL([ld], [LD_FOR_TARGET], [LD], [\$(abs_builddir)/../ld/ld-new])
+])
+
 SIM_TOOLCHAIN_VARS=
 AC_SUBST(SIM_TOOLCHAIN_VARS)
 AC_DEFUN([_SIM_AC_TOOLCHAIN_FOR_TARGET],
 [dnl
+AC_REQUIRE([SIM_AC_CHECK_TOOLCHAIN_FOR_PRIMARY_TARGET])
 AC_ARG_VAR(AS_FOR_TARGET_$2, [Assembler for $1 tests])
 AC_ARG_VAR(LD_FOR_TARGET_$2, [Linker for $1 tests])
 AC_ARG_VAR(CC_FOR_TARGET_$2, [C compiler for $1 tests])
 m4_bmatch($1, [example-], [dnl
-  : "${AS_FOR_TARGET_$2:=\$(abs_builddir)/../gas/as-new}"
-  : "${LD_FOR_TARGET_$2:=\$(abs_builddir)/../ld/ld-new}"
+  : "${AS_FOR_TARGET_$2:=\$(AS_FOR_TARGET)}"
+  : "${LD_FOR_TARGET_$2:=\$(LD_FOR_TARGET)}"
   : "${CC_FOR_TARGET_$2:=\$(CC)}"
 ], [dnl
   AS_IF([test "$SIM_PRIMARY_TARGET" = "$1"], [dnl
-    : "${AS_FOR_TARGET_$2:=\$(abs_builddir)/../gas/as-new}"
-    : "${LD_FOR_TARGET_$2:=\$(abs_builddir)/../ld/ld-new}"
-    dnl The default will be checked at test time.  If it's not available, then
-    dnl it is automatically skipped.  So hardcoding this is safe.
-    : "${CC_FOR_TARGET_$2:=${target_alias}-gcc}"
+    : "${AS_FOR_TARGET_$2:=\$(AS_FOR_TARGET)}"
+    : "${LD_FOR_TARGET_$2:=\$(LD_FOR_TARGET)}"
+    : "${CC_FOR_TARGET_$2:=\$(CC_FOR_TARGET)}"
   ])
 ])
 AS_VAR_APPEND([SIM_TOOLCHAIN_VARS], [" AS_FOR_TARGET_$2 LD_FOR_TARGET_$2 CC_FOR_TARGET_$2"])
-- 
2.30.2

[PATCH 10/12] sim: Add sim_dump_memory for debugging

[Hans-Peter Nilsson via Gdb-patches]

Intended to be called from the debugger tool.

sim/common:
	* sim-memopt.c (sim_dump_memory): New function.
---
 sim/common/sim-memopt.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sim/common/sim-memopt.c b/sim/common/sim-memopt.c
index 342188391d73..62423421cf9c 100644
--- a/sim/common/sim-memopt.c
+++ b/sim/common/sim-memopt.c
@@ -639,6 +639,16 @@ sim_memory_uninstall (SIM_DESC sd)
     }
 }
 
+void sim_dump_memory (SIM_DESC sd);
+
+/* Convenience function for use when debugging the simulator.  */
+
+void
+sim_dump_memory (SIM_DESC sd)
+{
+  memory_option_handler (sd, NULL, OPTION_MEMORY_INFO, NULL, 0);
+  memory_option_handler (sd, NULL, OPTION_MAP_INFO, NULL, 0);
+}
 
 static SIM_RC
 sim_memory_init (SIM_DESC sd)
-- 
2.30.2

[PATCH 11/12] sim/testsuite/cris: Remove faulty use of basename in C tests

[Hans-Peter Nilsson via Gdb-patches]

Calls to basename were added here as part of commit
e1e1ae6e9b5e "sim: testsuite: fix objdir handling", but that
commit missed adding "#include <libgen.h>" or the equivalent
GNU extension, see basename(3).  Fixing that shows a logical
error in the change to openpf1.c; the non-/-prefixed
code-path was changed instead of the "/"-prefixed code-path,
which is the one executed after that commit.

For "newlib" these tests failed linking after that commit.
Recent newlib has the (asm-renamed) GNU-extension-variant of
basename, but we're better off not using it at all.

Unfortunately, compilation failures for C tests run by the
machinery in c.exp are currently just marked "unresolved",
in contrast to C and assembler tests run by calling
run_sim_test.

The interaction of calling with the full program-path vs.
use of --sysroot exposes a consistency problem: when
--sysroot is used, argv[0] isn't the path by which the
program can find itself.  It's undecided whether argv[0] for
the program running in the simulator should be edited
(related to the naked argument to the simulator before
passing on to the simulated program) to remove a leading
--sysroot.  Either way, such a change would be out of scope
for this commit.

	* c/stat3.c (mybasename): New macro.  Use it instead of basename.
	* c/openpf1.c: Correct basename-related change and update related
	comment.
---
 sim/testsuite/cris/c/openpf1.c | 8 +++++---
 sim/testsuite/cris/c/stat3.c   | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/sim/testsuite/cris/c/openpf1.c b/sim/testsuite/cris/c/openpf1.c
index 92d12bfc4371..37940d709fbb 100644
--- a/sim/testsuite/cris/c/openpf1.c
+++ b/sim/testsuite/cris/c/openpf1.c
@@ -3,8 +3,8 @@
 
    We assume, with EXE being the name of the executable:
    - The simulator executes with cwd the same directory where the executable
-     is located (so argv[0] contains a plain filename without directory
-     components).
+     is located (also argv[0] contains a plain filename without directory
+     components -or- argv[0] contains the full non-sysroot path to EXE).
    - There's no /EXE on the host file system.  */
 
 #include <stdio.h>
@@ -21,8 +21,10 @@ int main (int argc, char *argv[])
       if (fnam == NULL)
 	abort ();
       strcpy (fnam, "/");
-      strcat (fnam, basename (argv[0]));
+      strcat (fnam, argv[0]);
     }
+  else
+    fnam = strrchr (argv[0], '/');
 
   f = fopen (fnam, "rb");
   if (f == NULL)
diff --git a/sim/testsuite/cris/c/stat3.c b/sim/testsuite/cris/c/stat3.c
index eac4da9ea8d9..c44143d353e6 100644
--- a/sim/testsuite/cris/c/stat3.c
+++ b/sim/testsuite/cris/c/stat3.c
@@ -7,13 +7,14 @@
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
+#define mybasename(x) ({ const char *x_ = (x), *y_ = strrchr (x_, '/'); y_ != NULL ? y_ + 1 : x_; })
 
 int main (int argc, char *argv[])
 {
   char path[1024] = "/";
   struct stat buf;
 
-  strcat (path, basename (argv[0]));
+  strcat (path, mybasename(argv[0]));
   if (stat (".", &buf) != 0
       || !S_ISDIR (buf.st_mode))
     abort ();
-- 
2.30.2

Re: [PATCH 04/12] sim/testsuite/cris/c: Use -sim3 but only for newlib targets

[Dimitar Dimitrov]

On Tue, Feb 15, 2022 at 12:02:55AM +0100, Hans-Peter Nilsson via Gdb-patches wrote:
> Commit a39487c6685f "sim: cris: use -sim with C tests for cris-elf
> targets" caused " -sim" to be appended to CFLAGS_FOR_TARGET for
> cris*-*-elf, where testing had until then relied on
> "RUNTESTFLAGS=--target_board=cris-sim" being passed when running "make
> check-sim", adding the right options.  While "-sim" happens to work,
> the baseboard-file cris-sim.exp uses "-sim3" so for consistency use
> that instead.
> 
> Then commit b42f20d2ac72 "sim: testsuite: drop most specific istarget
> checks" caused " -sim" to be appended for *all* targets, which just
> doesn't work.  For example, for crisv32-linux-gnu, that's not a
> recognized option and will cause a dejagnu error and further testing
> in c.exp will be aborted.
> 
> While cris-sim.exp appends "-static" for *-linux-gnu, further changes
> in the test-suite have caused "linux"-specific tests to break, so that
> part will be tended to separately.
> 
> But, save and restore CFLAGS_FOR_TARGET around the modification and
> use where needed, to not have the CRIS-specific modification affect a
> continuing test-run (possibly for other targets).
> 
> sim/testsuite/cris:
> 	* c/c.exp (CFLAGS_FOR_TARGET): Replace appended option " -sim"
> 	with " -sim3", but do it conditionally for newlib targets.  Save
> 	and restore CFLAGS_FOR_TARGET in saved_CFLAGS_FOR_TARGET such
> 	that it doesn't affect the value of CFLAGS_FOR_TARGET outside
> 	c.exp.
> ---
>  sim/testsuite/cris/c/c.exp | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/sim/testsuite/cris/c/c.exp b/sim/testsuite/cris/c/c.exp
> index 5711fd2f0bcd..3e186e072d8e 100644
> --- a/sim/testsuite/cris/c/c.exp
> +++ b/sim/testsuite/cris/c/c.exp
> @@ -17,6 +17,9 @@
>  
>  sim_init
>  
> +global global_cc_works
> +global global_cc_os

Hi,

FYI, this change introduces an error for "make check-sim" when configured
for pru-elf target. Error is gone if I revert this commit only. Note
that I don't have cross-CC in my PATH, which judging from sim_init_toolchain
function in sim-defs.exp is supported.

ERROR: -------------------------------------------
ERROR: in testcase /home/dinux/projects/pru/testbot-workspace/binutils/sim/testsuite/./cris/c/c.exp
ERROR:  can't read "global_cc_os": no such variable
ERROR:  tcl error code TCL LOOKUP VARNAME global_cc_os
ERROR:  tcl error info:
can't read "global_cc_os": no such variable
    while executing
"if { $global_cc_os == "newlib" } {
    append CFLAGS_FOR_TARGET " -sim3"
}"
    (file "/home/dinux/projects/pru/testbot-workspace/binutils/sim/testsuite/./cris/c/c.exp" line 35)


Regards,
Dimitar

> +
>  set CFLAGS_FOR_TARGET "-O2"
>  if [istarget cris-*-*] {
>      set mach "crisv10"
> @@ -24,13 +27,16 @@ if [istarget cris-*-*] {
>      set mach "crisv32"
>  }
>  
> -if [istarget *] {
> -    append CFLAGS_FOR_TARGET " -sim"
> +# Make sure we're using the right runtime for simulator runs.  If the
> +# cris-sim dejagnu baseboard is used, -sim3 will be duplicated, but
> +# that's ok.  For e.g. cris*-linux-gnu, neither -sim not -sim3 are
> +# supported options and likely not other targets too.
> +set saved_CFLAGS_FOR_TARGET $CFLAGS_FOR_TARGET
> +if { $global_cc_os == "newlib" } {
> +    append CFLAGS_FOR_TARGET " -sim3"
>  }
>  
>  # Using target_compile, since it is less noisy,
> -global global_cc_works
> -global global_cc_os
>  if { $global_cc_works == 1 } {
>      # Now check if we can link a program dynamically, and where
>      # libc.so is located.  If it is, we provide a sym link to the
> @@ -239,3 +245,5 @@ foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.c]] {
>  	$status "$mach $testname"
>      }
>  }
> +
> +set CFLAGS_FOR_TARGET $saved_CFLAGS_FOR_TARGET
> -- 
> 2.30.2
> 

Re: [PATCH 04/12] sim/testsuite/cris/c: Use -sim3 but only for newlib targets

[Hans-Peter Nilsson via Gdb-patches]

> From: Dimitar Dimitrov <dimitar@dinux.eu>
> Date: Tue, 15 Feb 2022 18:43:15 +0100

> On Tue, Feb 15, 2022 at 12:02:55AM +0100, Hans-Peter Nilsson via Gdb-patches wrote:
> > sim/testsuite/cris:
> > 	* c/c.exp (CFLAGS_FOR_TARGET): Replace appended option " -sim"
> > 	with " -sim3", but do it conditionally for newlib targets.  Save
> > 	and restore CFLAGS_FOR_TARGET in saved_CFLAGS_FOR_TARGET such
> > 	that it doesn't affect the value of CFLAGS_FOR_TARGET outside
> > 	c.exp.

> Hi,
> 
> FYI, this change introduces an error for "make check-sim" when configured
> for pru-elf target. Error is gone if I revert this commit only. Note
> that I don't have cross-CC in my PATH, which judging from sim_init_toolchain
> function in sim-defs.exp is supported.
> 
> ERROR: -------------------------------------------
> ERROR: in testcase /home/dinux/projects/pru/testbot-workspace/binutils/sim/testsuite/./cris/c/c.exp
> ERROR:  can't read "global_cc_os": no such variable
> ERROR:  tcl error code TCL LOOKUP VARNAME global_cc_os
> ERROR:  tcl error info:
> can't read "global_cc_os": no such variable
>     while executing
> "if { $global_cc_os == "newlib" } {
>     append CFLAGS_FOR_TARGET " -sim3"
> }"
>     (file "/home/dinux/projects/pru/testbot-workspace/binutils/sim/testsuite/./cris/c/c.exp" line 35)
> 
> 
> Regards,
> Dimitar

Please accept my apologies for the breakage and thanks for
the report!

It was actually the commit preceding the quoted one, that
was in error.  For that one, I was considering initializing
both global_cc_os and global_cc_works as in the patch below,
but went for the extant style of setting both in every
if-arm.  For some reason I missed the if-arm at the top,
which pru-dently (sorry) showed the flaw in that style.

The following solved the issue, tested by configuring for
pru-elf and doing "make check-sim" for a build (with in-tree
gas and ld for pru-elf) and observed the breakage in the
reported manner before the patch (and gone with it in
place).

Committed. 

---- 8< ----
sim/testsuite: Default global_cc_os and global_cc_works properly

There was an omission on 3e6dc39ed7a8 "sim/testsuite: Set
global_cc_os also when no compiler is found"; global_cc_os
wasn't set for other than the primary target, which means
that the "unguarded" use of global_cc_os in
testsuite/cris/c/c.exp caused the dreaded "ERROR: can't read
"global_cc_os": no such variable" when e.g. configuring for
pru-elf and doing "make check-sim".  Better initializing
both variables at the top to default values, rather than
adding another single 'set global_cc_os ""', to reduce the
risk of not setting them properly if or when that
if-statement-chain is made longer.

sim/testsuite:
	* lib/sim-defs.exp (sim_init_toolchain): Default
	global_cc_os and global_cc_works properly, before if-chain.
---
 sim/testsuite/lib/sim-defs.exp | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/sim/testsuite/lib/sim-defs.exp b/sim/testsuite/lib/sim-defs.exp
index d2750e08b046..5528d64684b3 100644
--- a/sim/testsuite/lib/sim-defs.exp
+++ b/sim/testsuite/lib/sim-defs.exp
@@ -121,9 +121,10 @@ proc sim_init_toolchain {} {
     set global_cpp_works [string equal "" "$result"]
 
     # See if we have a compiler available, and which environment it's targeting.
+    set global_cc_os ""
+    set global_cc_works 0
     if { $arch != $SIM_PRIMARY_TARGET && $CC_FOR_TARGET == "false" } {
 	verbose -log "Can't find a compatible C compiler"
-	set global_cc_works 0
     } elseif { [target_compile $srcdir/lib/newlibcheck.c \
 		$objdir/compilercheck.x "executable" $cc_options] == "" } {
 	verbose -log "Found newlib C compiler"
@@ -138,11 +139,8 @@ proc sim_init_toolchain {} {
 		$objdir/compilercheck.x "executable" $cc_options] == "" } {
 	verbose -log "Found C compiler, but unknown OS"
 	set global_cc_works 1
-	set global_cc_os ""
     } {
 	verbose -log "Can't execute C compiler"
-	set global_cc_works 0
-	set global_cc_os ""
     }
 
     file delete $objdir/compilercheck.x
-- 
2.30.2

Re: [PATCH 03/12] sim/testsuite: Set global_cc_os also when no compiler is found

[Mike Frysinger via Gdb-patches]

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

On 15 Feb 2022 00:02, Hans-Peter Nilsson via Gdb-patches wrote:
> Neither the commit introducing progos, nor the top comment
> in run_sim_test, mentions progos as intended only for C
> tests, or that its use must be gated on $global_cc_works !=
> 0, so (not) setting it in the no-working-compiler path seems
> just overlooked.

you're correct, thanks -- it is not meant to be language agnostic.  just
pratically speaking, C tests tend to be the only ones that can target a
different OS.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 01/12] sim cris: Correct PRIu32 to PRIx32

[Mike Frysinger via Gdb-patches]

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

On 14 Feb 2022 23:59, Hans-Peter Nilsson via Gdb-patches wrote:
> In 5ee0bc23a68f "sim: clean up bfd_vma printing" there was
> an additional introduction of PRIx32 and PRIu32 but just in
> sim/cris/sim-if.c.  One type of bug was fixed in commit
> d16ce6e4d581 "sim: cris: fix memory setup typos" but one
> remained; the PRIu32 usage is wrong, as hex output is
> desired; note the 0x prefix.
> 
> Without this fix, you'll see output like:
>  memory map 0:0x4000..0x5fff (8192 bytes) overlaps 0:0x0..0x16383 (91012 bytes)
>  program stopped with signal 6 (Aborted).
> for some C programs, like some of the ones in the sim/cris/c
> testsuite from where the example is taken (freopen2.c).
> 
> The bug behavior was with memory allocation.  With an
> attempt to allocate memory using the brk syscall such that
> the room up to the next 8192-byte "page boundary" wasn't
> sufficient, the simulator memory allocation machinery horked
> on a consistency error when trying to allocate a memory
> block to raise the "end of the data segment": there was
> already memory allocated at that address.
> 
> Unfortunately, none of the programs in sim/cris/asm exposed
> this bug at the time, but an assembler test-case is
> committed after this fix.

thanks, this had been plaguing me for a while.  i eventually managed to
get a cris-elf toolchain working to test against, but it was long after
the bug, so i wasn't able to trace back where it was coming from.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 06/12] sim/testsuite: Support "requires: simoption <--name-of-option>"

[Mike Frysinger via Gdb-patches]

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

On 15 Feb 2022 00:03, Hans-Peter Nilsson via Gdb-patches wrote:
> Simulator features can be present or not, typically
> depending on different-valued configure options, like
> --enable-sim-hardware[=off|=on].  To avoid failures in
> test-suite-runs when testing such configurations, a new
> predicate is needed, as neither "target", "progos" nor
> "mach" fits cleanly.
> 
> The immediate need was to check for presence of a simulator
> option, but rather than a specialized "requires-simoption:"
> predicate I thought I'd handle the general (parametrized)
> need, so here's a generic predicate machinery and a (first)
> predicate to use together with it; checking whether a
> particular option is supported, by looking at "run --help"
> output.  This was inspired by the check_effective_target_
> machinery in the gcc test-suite.

i really don't want --help to be an API surface like this.  it's the wrong
layer for the job.

we have a sim_config_print function which dumps configuration information.
i'd be fine making that the surface to build off of.  i don't think we
print hardware there atm, but should be trivial to introduce.

only other missing piece is that it's not obvious how to access it from
the CLI.  `run --version` doesn't include it.  `run --do-command version`
does though :x.  i'd be amenable to improving this interface, either by a
new option like --info-config</bikeshed> or some other route.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 08/12] sim cris: Unbreak --disable-sim-hardware builds

[Mike Frysinger via Gdb-patches]

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

On 15 Feb 2022 00:05, Hans-Peter Nilsson via Gdb-patches wrote:
> +#if WITH_HW
>    if (cris_have_900000xxif)
>      sim_hw_parse (sd, "/core/%s/reg %#x %i", "cris_900000xx", 0x90000000, 0x100);
> +#else
> +  /* With the option disabled, nothing should be able to set this variable.
> +     We should "use" it, though, and why not assert that it isn't set.  */
> +  ASSERT (! cris_have_900000xxif);
> +#endif

WITH_HW is always defined to either 1 or 0.  could you write:
  if (WITH_HW)
    ...
  else
    ...

this avoids bit rot in the uncommon configure paths
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 11/12] sim/testsuite/cris: Remove faulty use of basename in C tests

[Mike Frysinger via Gdb-patches]

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

On 15 Feb 2022 00:07, Hans-Peter Nilsson via Gdb-patches wrote:
> --- a/sim/testsuite/cris/c/stat3.c
> +++ b/sim/testsuite/cris/c/stat3.c
> @@ -7,13 +7,14 @@
>  #include <stdio.h>
>  #include <string.h>
>  #include <stdlib.h>
> +#define mybasename(x) ({ const char *x_ = (x), *y_ = strrchr (x_, '/'); y_ != NULL ? y_ + 1 : x_; })
>  
>  int main (int argc, char *argv[])
>  {
>    char path[1024] = "/";
>    struct stat buf;
>  
> -  strcat (path, basename (argv[0]));
> +  strcat (path, mybasename(argv[0]));

GNU style (and this file style) says to put space before the (
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 10/12] sim: Add sim_dump_memory for debugging

[Mike Frysinger via Gdb-patches]

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

On 15 Feb 2022 00:06, Hans-Peter Nilsson via Gdb-patches wrote:
> --- a/sim/common/sim-memopt.c
> +++ b/sim/common/sim-memopt.c
>
> +/* Convenience function for use when debugging the simulator.  */

can you clarify that it's meant for an attached debugger to call vs a
manual call from within the sim itself ?  i know your commit message
said that, but the code comment would help too.

i vaguely recall seeing another such hook point in the tree.  i wonder
if we should gather the prototypes in a single header and give them a
dedicated symbol prefix to make them easier to discover.  as it stands,
i think you're prob the only one who will use this for quite some time
as no one else will notice it :P.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 09/12] sim: Fix use of out-of-tree assembler and linker when testing

[Mike Frysinger via Gdb-patches]

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

On 15 Feb 2022 00:05, Hans-Peter Nilsson via Gdb-patches wrote:
> This commit calls GCC_TARGET_TOOL (a deceptive name; gcc-specific
> features aren't used) from toplev/config/acx.m4, somewhat like calls
> in toplev/configure.ac but without the NCN_STRICT_CHECK_TARGET_TOOLS
> step, for each X to find a value for $(<X>_FOR_TARGET).  N.B.: in-tree
> tools still override any ${target}-${tool} found in $PATH, i.e. only
> previously broken builds are affected.

thanks, i hadn't noticed this macro before
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 04/12] sim/testsuite/cris/c: Use -sim3 but only for newlib targets

[Mike Frysinger via Gdb-patches]

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

On 15 Feb 2022 00:02, Hans-Peter Nilsson via Gdb-patches wrote:
> Commit a39487c6685f "sim: cris: use -sim with C tests for cris-elf
> targets" caused " -sim" to be appended to CFLAGS_FOR_TARGET for
> cris*-*-elf, where testing had until then relied on
> "RUNTESTFLAGS=--target_board=cris-sim" being passed when running "make
> check-sim", adding the right options.  While "-sim" happens to work,
> the baseboard-file cris-sim.exp uses "-sim3" so for consistency use
> that instead.
> 
> Then commit b42f20d2ac72 "sim: testsuite: drop most specific istarget
> checks" caused " -sim" to be appended for *all* targets, which just
> doesn't work.  For example, for crisv32-linux-gnu, that's not a
> recognized option and will cause a dejagnu error and further testing
> in c.exp will be aborted.
> 
> While cris-sim.exp appends "-static" for *-linux-gnu, further changes
> in the test-suite have caused "linux"-specific tests to break, so that
> part will be tended to separately.
> 
> But, save and restore CFLAGS_FOR_TARGET around the modification and
> use where needed, to not have the CRIS-specific modification affect a
> continuing test-run (possibly for other targets).

i'm trying to get away from needing dejagnu boards at all.  it brings nothing
to the table when it comes to testing the sim itself.  ideally we should have
a single sim binary that supports all targets simultaneously, and it's only
runtime options (or dynamic probing) that selects between them.  that's why
#progos was introduced -- so tests could declare which env they're targeting
and the test framework can run the simulator with the right settings.

one can now do:
$ ./configure --enable-targets=all
$ make check-sim

and every architecture will be built & tested.  no need for multiple build
dirs for diff targets or sep runs with diff runtestflags.

at some point i also want to delete all the custom compile+run logic in the
testsuite/cris/ tree.  that's why i spent so much time pulling code out and
into the common one.

i even have a poc locally that deletes the dejagnu framework entirely and
switches to Automake test harness, but i haven't quite figured out how to
cleanly handle the all_machs multiplex logic in Automake.  i eventually
pulled out individual cleanups and merged them so at least `make check`
works in a multi-target build, and isn't nearly as slow as it was.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 04/12] sim/testsuite/cris/c: Use -sim3 but only for newlib targets

[Mike Frysinger via Gdb-patches]

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

On 15 Feb 2022 00:02, Hans-Peter Nilsson via Gdb-patches wrote:
> But, save and restore CFLAGS_FOR_TARGET around the modification and
> use where needed, to not have the CRIS-specific modification affect a
> continuing test-run (possibly for other targets).

this part isn't needed by design.  every .exp file calls `sim_init` which in
turn calls `sim_init_toolchain` which fully initializes the toolchain for the
current file.  so as soon as c.exp finishes running, whatever comes next is
going to reset the flags you saved.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 08/12] sim cris: Unbreak --disable-sim-hardware builds

[Hans-Peter Nilsson via Gdb-patches]

> Date: Tue, 15 Feb 2022 23:51:16 -0500
> From: Mike Frysinger <vapier@gentoo.org>

> On 15 Feb 2022 00:05, Hans-Peter Nilsson via Gdb-patches wrote:
> > +#if WITH_HW
> >    if (cris_have_900000xxif)
> >      sim_hw_parse (sd, "/core/%s/reg %#x %i", "cris_900000xx", 0x90000000, 0x100);
> > +#else
> > +  /* With the option disabled, nothing should be able to set this variable.
> > +     We should "use" it, though, and why not assert that it isn't set.  */
> > +  ASSERT (! cris_have_900000xxif);
> > +#endif
> 
> WITH_HW is always defined to either 1 or 0.  could you write:
>   if (WITH_HW)
>     ...
>   else
>     ...
> 
> this avoids bit rot in the uncommon configure paths

I don't think that's a good idea, because then we'd rely on
the sim_hw_parse reference go away at *all* optimization
levels.

brgds, H-P

Re: [PATCH 04/12] sim/testsuite/cris/c: Use -sim3 but only for newlib targets

[Hans-Peter Nilsson via Gdb-patches]

> Date: Wed, 16 Feb 2022 00:25:07 -0500
> From: Mike Frysinger <vapier@gentoo.org>

> On 15 Feb 2022 00:02, Hans-Peter Nilsson via Gdb-patches wrote:
> > Commit a39487c6685f "sim: cris: use -sim with C tests for cris-elf
> > targets" caused " -sim" to be appended to CFLAGS_FOR_TARGET for
> > cris*-*-elf, where testing had until then relied on
> > "RUNTESTFLAGS=--target_board=cris-sim" being passed when running "make
> > check-sim", adding the right options.  While "-sim" happens to work,
> > the baseboard-file cris-sim.exp uses "-sim3" so for consistency use
> > that instead.
> > 
> > Then commit b42f20d2ac72 "sim: testsuite: drop most specific istarget
> > checks" caused " -sim" to be appended for *all* targets, which just
> > doesn't work.  For example, for crisv32-linux-gnu, that's not a
> > recognized option and will cause a dejagnu error and further testing
> > in c.exp will be aborted.
> > 
> > While cris-sim.exp appends "-static" for *-linux-gnu, further changes
> > in the test-suite have caused "linux"-specific tests to break, so that
> > part will be tended to separately.
> > 
> > But, save and restore CFLAGS_FOR_TARGET around the modification and
> > use where needed, to not have the CRIS-specific modification affect a
> > continuing test-run (possibly for other targets).
> 
> i'm trying to get away from needing dejagnu boards at all.  it brings nothing
> to the table when it comes to testing the sim itself.

I know you're of that opinion, but I'll say this once again (it
was decades ago last time): you're wrong.  This leads you to
discarding half of dejagnu and working *against* it rather than
*with it*.

>  ideally we should have
> a single sim binary that supports all targets simultaneously, and it's only
> runtime options (or dynamic probing) that selects between them.  that's why
> #progos was introduced -- so tests could declare which env they're targeting
> and the test framework can run the simulator with the right settings.
> 
> one can now do:
> $ ./configure --enable-targets=all
> $ make check-sim
> 
> and every architecture will be built & tested.  no need for multiple build
> dirs for diff targets or sep runs with diff runtestflags.

Right, I figured that's your preferred setup.  You can certainly
make dejagnu baseboards work in this scenario: just call *each*
of the one one that fits when testing *each* simulator, and
clear the slate in-between.

> at some point i also want to delete all the custom compile+run logic in the
> testsuite/cris/ tree.  that's why i spent so much time pulling code out and
> into the common one.

The baby went with the bath-water here.  (Or IOW, if you pull
out *all* of the test-suites the testsuite will be really
simple!)

> i even have a poc locally that deletes the dejagnu framework entirely and
> switches to Automake test harness, but i haven't quite figured out how to
> cleanly handle the all_machs multiplex logic in Automake.  i eventually
> pulled out individual cleanups and merged them so at least `make check`
> works in a multi-target build, and isn't nearly as slow as it was.
> -mike

Ouch.  I haven't found the "automake test harness" to be of use
for serious testing.

brgds, H-P

Re: [PATCH 04/12] sim/testsuite/cris/c: Use -sim3 but only for newlib targets

[Hans-Peter Nilsson via Gdb-patches]

> Date: Wed, 16 Feb 2022 00:39:09 -0500
> From: Mike Frysinger <vapier@gentoo.org>

> On 15 Feb 2022 00:02, Hans-Peter Nilsson via Gdb-patches wrote:
> > But, save and restore CFLAGS_FOR_TARGET around the modification and
> > use where needed, to not have the CRIS-specific modification affect a
> > continuing test-run (possibly for other targets).
> 
> this part isn't needed by design.  every .exp file calls `sim_init` which in
> turn calls `sim_init_toolchain` which fully initializes the toolchain for the
> current file.  so as soon as c.exp finishes running, whatever comes next is
> going to reset the flags you saved.
> -mike

I know, I just didn't want to rely on that being the case.

brgds, H-P

Re: [PATCH 10/12] sim: Add sim_dump_memory for debugging

[Hans-Peter Nilsson via Gdb-patches]

> Date: Wed, 16 Feb 2022 00:02:23 -0500
> From: Mike Frysinger <vapier@gentoo.org>

> On 15 Feb 2022 00:06, Hans-Peter Nilsson via Gdb-patches wrote:
> > --- a/sim/common/sim-memopt.c
> > +++ b/sim/common/sim-memopt.c
> >
> > +/* Convenience function for use when debugging the simulator.  */
> 
> can you clarify that it's meant for an attached debugger to call vs a
> manual call from within the sim itself ?  i know your commit message
> said that, but the code comment would help too.
> 
> i vaguely recall seeing another such hook point in the tree.  i wonder
> if we should gather the prototypes in a single header and give them a
> dedicated symbol prefix to make them easier to discover.  as it stands,
> i think you're prob the only one who will use this for quite some time
> as no one else will notice it :P.
> -mike

A good idea!

brgds, H-P

Re: [PATCH 06/12] sim/testsuite: Support "requires: simoption <--name-of-option>"

[Hans-Peter Nilsson via Gdb-patches]

> Date: Tue, 15 Feb 2022 23:49:47 -0500
> From: Mike Frysinger <vapier@gentoo.org>

> On 15 Feb 2022 00:03, Hans-Peter Nilsson via Gdb-patches wrote:
> > Simulator features can be present or not, typically
> > depending on different-valued configure options, like
> > --enable-sim-hardware[=off|=on].  To avoid failures in
> > test-suite-runs when testing such configurations, a new
> > predicate is needed, as neither "target", "progos" nor
> > "mach" fits cleanly.
> > 
> > The immediate need was to check for presence of a simulator
> > option, but rather than a specialized "requires-simoption:"
> > predicate I thought I'd handle the general (parametrized)
> > need, so here's a generic predicate machinery and a (first)
> > predicate to use together with it; checking whether a
> > particular option is supported, by looking at "run --help"
> > output.  This was inspired by the check_effective_target_
> > machinery in the gcc test-suite.
> 
> i really don't want --help to be an API surface like this.  it's the wrong
> layer for the job.
> 
> we have a sim_config_print function which dumps configuration information.
> i'd be fine making that the surface to build off of.  i don't think we
> print hardware there atm, but should be trivial to introduce.
> 
> only other missing piece is that it's not obvious how to access it from
> the CLI.  `run --version` doesn't include it.  `run --do-command version`
> does though :x.  i'd be amenable to improving this interface, either by a
> new option like --info-config</bikeshed> or some other route.

But, "run --version" is a check for the *option* to exist,
which exactly meets the need.  You describe a probe for a
particular *configuration*, which is arguably useful, but
not for checking whether a particular option is supported.

brgds, H-P

Re: [PATCH 10/12] sim: Add sim_dump_memory for debugging

[Hans-Peter Nilsson via Gdb-patches]

> Date: Wed, 16 Feb 2022 00:02:23 -0500
> From: Mike Frysinger <vapier@gentoo.org>

> On 15 Feb 2022 00:06, Hans-Peter Nilsson via Gdb-patches wrote:
> > --- a/sim/common/sim-memopt.c
> > +++ b/sim/common/sim-memopt.c
> >
> > +/* Convenience function for use when debugging the simulator.  */
> 
> can you clarify that it's meant for an attached debugger to call vs a
> manual call from within the sim itself ?  i know your commit message
> said that, but the code comment would help too.

Committed.

---- 8< ----

sim/common: Improve sim_dump_memory head comment

As requested by Mike.

	* sim-memopt.c: Improve head comment.
---
 sim/common/sim-memopt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sim/common/sim-memopt.c b/sim/common/sim-memopt.c
index 62423421cf9c..1f7186c5e12d 100644
--- a/sim/common/sim-memopt.c
+++ b/sim/common/sim-memopt.c
@@ -641,7 +641,8 @@ sim_memory_uninstall (SIM_DESC sd)
 
 void sim_dump_memory (SIM_DESC sd);
 
-/* Convenience function for use when debugging the simulator.  */
+/* Convenience function for use when debugging the simulator, to be
+   called from within e.g. gdb.  */
 
 void
 sim_dump_memory (SIM_DESC sd)
-- 
2.30.2

Re: [PATCH 11/12] sim/testsuite/cris: Remove faulty use of basename in C tests

[Hans-Peter Nilsson via Gdb-patches]

> Date: Tue, 15 Feb 2022 23:57:00 -0500
> From: Mike Frysinger <vapier@gentoo.org>

> On 15 Feb 2022 00:07, Hans-Peter Nilsson via Gdb-patches wrote:
> > --- a/sim/testsuite/cris/c/stat3.c
> > +++ b/sim/testsuite/cris/c/stat3.c
> > @@ -7,13 +7,14 @@
> >  #include <stdio.h>
> >  #include <string.h>
> >  #include <stdlib.h>
> > +#define mybasename(x) ({ const char *x_ = (x), *y_ = strrchr (x_, '/'); y_ != NULL ? y_ + 1 : x_; })
> >  
> >  int main (int argc, char *argv[])
> >  {
> >    char path[1024] = "/";
> >    struct stat buf;
> >  
> > -  strcat (path, basename (argv[0]));
> > +  strcat (path, mybasename(argv[0]));
> 
> GNU style (and this file style) says to put space before the (
> -mike

Now that's a nit if ever I saw one; it's in the test-suite!

Committed.

--- 8< ---
sim/testsuite/cris/c/stat3.c: Fix formatting nit

	* c/stat3.c (main): Fix formatting nit.
---
 sim/testsuite/cris/c/stat3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sim/testsuite/cris/c/stat3.c b/sim/testsuite/cris/c/stat3.c
index f7c96045832c..321da1b2bd61 100644
--- a/sim/testsuite/cris/c/stat3.c
+++ b/sim/testsuite/cris/c/stat3.c
@@ -14,7 +14,7 @@ int main (int argc, char *argv[])
   char path[1024] = "/";
   struct stat buf;
 
-  strcat (path, mybasename(argv[0]));
+  strcat (path, mybasename (argv[0]));
   if (stat (".", &buf) != 0
       || !S_ISDIR (buf.st_mode))
     abort ();
-- 
2.30.2

Re: [PATCH 08/12] sim cris: Unbreak --disable-sim-hardware builds

[Hans-Peter Nilsson via Gdb-patches]

> From: Hans-Peter Nilsson <Hans-Peter.Nilsson@axis.com>
> Date: Wed, 16 Feb 2022 06:54:19 +0100

> > Date: Tue, 15 Feb 2022 23:51:16 -0500
> > From: Mike Frysinger <vapier@gentoo.org>
> 
> > On 15 Feb 2022 00:05, Hans-Peter Nilsson via Gdb-patches wrote:
> > > +#if WITH_HW
> > >    if (cris_have_900000xxif)
> > >      sim_hw_parse (sd, "/core/%s/reg %#x %i", "cris_900000xx", 0x90000000, 0x100);
> > > +#else
> > > +  /* With the option disabled, nothing should be able to set this variable.
> > > +     We should "use" it, though, and why not assert that it isn't set.  */
> > > +  ASSERT (! cris_have_900000xxif);
> > > +#endif
> > 
> > WITH_HW is always defined to either 1 or 0.  could you write:
> >   if (WITH_HW)
> >     ...
> >   else
> >     ...
> > 
> > this avoids bit rot in the uncommon configure paths
> 
> I don't think that's a good idea, because then we'd rely on
> the sim_hw_parse reference go away at *all* optimization
> levels.

...on the other hand, an aborting function stub for
sim_hw_parse could be introduced for that purpose.  It's not
obvious to me where to put it, though.

brgds, H-P

Re: [PATCH 06/12] sim/testsuite: Support "requires: simoption <--name-of-option>"

[Mike Frysinger via Gdb-patches]

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

On 16 Feb 2022 07:24, Hans-Peter Nilsson wrote:
> Date: Tue, 15 Feb 2022 23:49:47 -0500 From: Mike Frysinger <vapier@gentoo.org>
> > On 15 Feb 2022 00:03, Hans-Peter Nilsson via Gdb-patches wrote:
> > > Simulator features can be present or not, typically
> > > depending on different-valued configure options, like
> > > --enable-sim-hardware[=off|=on].  To avoid failures in
> > > test-suite-runs when testing such configurations, a new
> > > predicate is needed, as neither "target", "progos" nor
> > > "mach" fits cleanly.
> > > 
> > > The immediate need was to check for presence of a simulator
> > > option, but rather than a specialized "requires-simoption:"
> > > predicate I thought I'd handle the general (parametrized)
> > > need, so here's a generic predicate machinery and a (first)
> > > predicate to use together with it; checking whether a
> > > particular option is supported, by looking at "run --help"
> > > output.  This was inspired by the check_effective_target_
> > > machinery in the gcc test-suite.
> > 
> > i really don't want --help to be an API surface like this.  it's the wrong
> > layer for the job.
> > 
> > we have a sim_config_print function which dumps configuration information.
> > i'd be fine making that the surface to build off of.  i don't think we
> > print hardware there atm, but should be trivial to introduce.
> > 
> > only other missing piece is that it's not obvious how to access it from
> > the CLI.  `run --version` doesn't include it.  `run --do-command version`
> > does though :x.  i'd be amenable to improving this interface, either by a
> > new option like --info-config</bikeshed> or some other route.
> 
> But, "run --version" is a check for the *option* to exist,
> which exactly meets the need.  You describe a probe for a
> particular *configuration*, which is arguably useful, but
> not for checking whether a particular option is supported.

i think you misunderstand.  you're basically running:
  run --help | grep -e--option
where --option is some functionality you care about.

i'm saying --help is not an interface.  it should be free to change and
reformat things as makes sense and not worry about testsuites breaking.
in the case of a multitarget binary, we probably wouldn't display all the
options in a single page, but have arch-specific sections.

i'm proposing:
  run --do-command version | grep <feature>
where in this case you seem to care about hardware support being enabled.
so the test would look like:
  # requires: simoption WITH_HW
and then the code would look for that in the structured output produced
by the sim config output (which is included in the extended version).
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 08/12] sim cris: Unbreak --disable-sim-hardware builds

[Mike Frysinger via Gdb-patches]

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

On 16 Feb 2022 06:54, Hans-Peter Nilsson wrote:
> Date: Tue, 15 Feb 2022 23:51:16 -0500 Mike Frysinger <vapier@gentoo.org>
> > On 15 Feb 2022 00:05, Hans-Peter Nilsson via Gdb-patches wrote:
> > > +#if WITH_HW
> > >    if (cris_have_900000xxif)
> > >      sim_hw_parse (sd, "/core/%s/reg %#x %i", "cris_900000xx", 0x90000000, 0x100);
> > > +#else
> > > +  /* With the option disabled, nothing should be able to set this variable.
> > > +     We should "use" it, though, and why not assert that it isn't set.  */
> > > +  ASSERT (! cris_have_900000xxif);
> > > +#endif
> > 
> > WITH_HW is always defined to either 1 or 0.  could you write:
> >   if (WITH_HW)
> >     ...
> >   else
> >     ...
> > 
> > this avoids bit rot in the uncommon configure paths
> 
> I don't think that's a good idea, because then we'd rely on
> the sim_hw_parse reference go away at *all* optimization
> levels.

we prob should rework the sim-hw module a bit to handle this scenario.  other
modules (like sim-profile) handle this internally.  basically we should have
a stable API regardless of configure settings.  then we wouldn't rely on DCE
kicking in all the time.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 04/12] sim/testsuite/cris/c: Use -sim3 but only for newlib targets

[Mike Frysinger via Gdb-patches]

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

On 16 Feb 2022 07:09, Hans-Peter Nilsson wrote:
> Date: Wed, 16 Feb 2022 00:39:09 -0500 Mike Frysinger <vapier@gentoo.org>
> > On 15 Feb 2022 00:02, Hans-Peter Nilsson via Gdb-patches wrote:
> > > But, save and restore CFLAGS_FOR_TARGET around the modification and
> > > use where needed, to not have the CRIS-specific modification affect a
> > > continuing test-run (possibly for other targets).
> > 
> > this part isn't needed by design.  every .exp file calls `sim_init` which in
> > turn calls `sim_init_toolchain` which fully initializes the toolchain for the
> > current file.  so as soon as c.exp finishes running, whatever comes next is
> > going to reset the flags you saved.
> 
> I know, I just didn't want to rely on that being the case.

i'm saying that the behavior isn't an accident.  it's designed this way.
hence you aren't relying on incidental behavior, you're using the API as
intended.  i explicitly went through and stripped out the save/restore
logic from the tests because it was already inconsistently implemented.
this way it's guaranteed to be consistent, and we don't need boilerplate.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 04/12] sim/testsuite/cris/c: Use -sim3 but only for newlib targets

[Mike Frysinger via Gdb-patches]

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

On 16 Feb 2022 07:07, Hans-Peter Nilsson wrote:
> Date: Wed, 16 Feb 2022 00:25:07 -0500 Mike Frysinger <vapier@gentoo.org>
> > On 15 Feb 2022 00:02, Hans-Peter Nilsson via Gdb-patches wrote:
> > > Commit a39487c6685f "sim: cris: use -sim with C tests for cris-elf
> > > targets" caused " -sim" to be appended to CFLAGS_FOR_TARGET for
> > > cris*-*-elf, where testing had until then relied on
> > > "RUNTESTFLAGS=--target_board=cris-sim" being passed when running "make
> > > check-sim", adding the right options.  While "-sim" happens to work,
> > > the baseboard-file cris-sim.exp uses "-sim3" so for consistency use
> > > that instead.
> > > 
> > > Then commit b42f20d2ac72 "sim: testsuite: drop most specific istarget
> > > checks" caused " -sim" to be appended for *all* targets, which just
> > > doesn't work.  For example, for crisv32-linux-gnu, that's not a
> > > recognized option and will cause a dejagnu error and further testing
> > > in c.exp will be aborted.
> > > 
> > > While cris-sim.exp appends "-static" for *-linux-gnu, further changes
> > > in the test-suite have caused "linux"-specific tests to break, so that
> > > part will be tended to separately.
> > > 
> > > But, save and restore CFLAGS_FOR_TARGET around the modification and
> > > use where needed, to not have the CRIS-specific modification affect a
> > > continuing test-run (possibly for other targets).
> > 
> > i'm trying to get away from needing dejagnu boards at all.  it brings nothing
> > to the table when it comes to testing the sim itself.
> 
> I know you're of that opinion, but I'll say this once again (it
> was decades ago last time): you're wrong.  This leads you to
> discarding half of dejagnu and working *against* it rather than
> *with it*.

i think you're overstating what it has to offer to the sim.  it can be useful
for people to define targets to run programs against (e.g. defining different
sim or hardware or remote systems), but none of that matters here.  we have
full control over the sim runtime and do not care about any of the other
aspects like running against real hardware.

> >  ideally we should have
> > a single sim binary that supports all targets simultaneously, and it's only
> > runtime options (or dynamic probing) that selects between them.  that's why
> > #progos was introduced -- so tests could declare which env they're targeting
> > and the test framework can run the simulator with the right settings.
> > 
> > one can now do:
> > $ ./configure --enable-targets=all
> > $ make check-sim
> > 
> > and every architecture will be built & tested.  no need for multiple build
> > dirs for diff targets or sep runs with diff runtestflags.
> 
> Right, I figured that's your preferred setup.  You can certainly
> make dejagnu baseboards work in this scenario: just call *each*
> of the one one that fits when testing *each* simulator, and
> clear the slate in-between.

this requires people to get their baseboards into dejagnu, and for a dejagnu
release to be made, and for these settings to be kept in sync.  releases are
way too slow on the dejagnu front.

people also tend to write their baseboards for their own setup they're using
which doesn't generalize.

`make check` should work out of the box for all targets & sim functionality
for everyone.  if they have to fumble about with runtestflags or any other
dejagnu settings, then the testsuite is doing it wrong.

if gas & ld ever get multitarget support, we'd have an even easier time with
all the xxx_FOR_TARGET_xxx settings.

again, i'm speaking only about the sim here.  something like gcc going through
dejagnu baseboards for the sim or real hardware makes sense.

> > at some point i also want to delete all the custom compile+run logic in the
> > testsuite/cris/ tree.  that's why i spent so much time pulling code out and
> > into the common one.
> 
> The baby went with the bath-water here.  (Or IOW, if you pull
> out *all* of the test-suites the testsuite will be really
> simple!)

the "baby" is the tests.  i'm not talking about deleting the tests.  there is
no reason cris should be parsing or compiling or linking or running any tests
on its own.  the functionality it wants is by no means unique to cris.

> > i even have a poc locally that deletes the dejagnu framework entirely and
> > switches to Automake test harness, but i haven't quite figured out how to
> > cleanly handle the all_machs multiplex logic in Automake.  i eventually
> > pulled out individual cleanups and merged them so at least `make check`
> > works in a multi-target build, and isn't nearly as slow as it was.
> 
> Ouch.  I haven't found the "automake test harness" to be of use
> for serious testing.

i'm not a fan of a framework that can't fathom a system with more than 1 cpu.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 06/12] sim/testsuite: Support "requires: simoption <--name-of-option>"

[Hans-Peter Nilsson via Gdb-patches]

> Date: Wed, 16 Feb 2022 02:09:44 -0500
> From: Mike Frysinger <vapier@gentoo.org>

> On 16 Feb 2022 07:24, Hans-Peter Nilsson wrote:
> > Date: Tue, 15 Feb 2022 23:49:47 -0500 From: Mike Frysinger <vapier@gentoo.org>
> > > On 15 Feb 2022 00:03, Hans-Peter Nilsson via Gdb-patches wrote:
> > > > Simulator features can be present or not, typically
> > > > depending on different-valued configure options, like
> > > > --enable-sim-hardware[=off|=on].  To avoid failures in
> > > > test-suite-runs when testing such configurations, a new
> > > > predicate is needed, as neither "target", "progos" nor
> > > > "mach" fits cleanly.
> > > > 
> > > > The immediate need was to check for presence of a simulator
> > > > option, but rather than a specialized "requires-simoption:"
> > > > predicate I thought I'd handle the general (parametrized)
> > > > need, so here's a generic predicate machinery and a (first)
> > > > predicate to use together with it; checking whether a
> > > > particular option is supported, by looking at "run --help"
> > > > output.  This was inspired by the check_effective_target_
> > > > machinery in the gcc test-suite.
> > > 
> > > i really don't want --help to be an API surface like this.  it's the wrong
> > > layer for the job.
> > > 
> > > we have a sim_config_print function which dumps configuration information.
> > > i'd be fine making that the surface to build off of.  i don't think we
> > > print hardware there atm, but should be trivial to introduce.
> > > 
> > > only other missing piece is that it's not obvious how to access it from
> > > the CLI.  `run --version` doesn't include it.  `run --do-command version`
> > > does though :x.  i'd be amenable to improving this interface, either by a
> > > new option like --info-config</bikeshed> or some other route.
> > 
> > But, "run --version" is a check for the *option* to exist,
> > which exactly meets the need.  You describe a probe for a
> > particular *configuration*, which is arguably useful, but
> > not for checking whether a particular option is supported.
> 
> i think you misunderstand.

(Methinks it's the other way round.)

> you're basically running:
>   run --help | grep -e--option
> where --option is some functionality you care about.

Right, the option *needs to be supported*.  *How* it's
implemented is secondary.

> i'm saying --help is not an interface.  it should be free to change and
> reformat things as makes sense and not worry about
> testsuites breaking.

If that format happens, that'll be handled just like any
other change, with the test-suite adjusted.  It's not like
the --help output format changes very often.

The same argument can also be used for your proposed "run
--do-command version", which has actually changed in the
last 10 years, so I don't see this argument as for or
against anything.

> in the case of a multitarget binary, we probably wouldn't display all the
> options in a single page, but have arch-specific sections.

And sim_check_requires_simoption would then be altered to
filter-in the subtarget-specific section for the subtarget
being tested, no biggie.  I can also imagine that it may
happen without further action just by changing all "run" to
"run $subtarget" or $run_subtarget everywhere in the
test-suite if/when you go multitarget-binary.

> i'm proposing:
>   run --do-command version | grep <feature>
> where in this case you seem to care about hardware support being enabled.
> so the test would look like:
>   # requires: simoption WITH_HW

That would be fine (except for s/simoption/configoption/) if
I was actually testing something that was *necessarily*
linked to WITH_HW, and perhaps not even reflected in the
presence of a run-time simulator option.

Here, I'm testing the functionality of an option that just
*happens* to be linked to --enable/--disable-sim-hardware!
As you surely remember, it used to be a whole different
module.

To wit, my main point is that the test itself shouldn't have
to care about the implementation or dependent config
options; it should just mention the exact options that it
uses, when that presence may depend on something unknown
(perhaps something other than a configure-time option), just
as the current version does.

brgds, H-P

Re: [PATCH 04/12] sim/testsuite/cris/c: Use -sim3 but only for newlib targets

[Hans-Peter Nilsson via Gdb-patches]

> Date: Wed, 16 Feb 2022 02:17:43 -0500
> From: Mike Frysinger <vapier@gentoo.org>

> On 16 Feb 2022 07:09, Hans-Peter Nilsson wrote:
> > Date: Wed, 16 Feb 2022 00:39:09 -0500 Mike Frysinger <vapier@gentoo.org>
> > > On 15 Feb 2022 00:02, Hans-Peter Nilsson via Gdb-patches wrote:
> > > > But, save and restore CFLAGS_FOR_TARGET around the modification and
> > > > use where needed, to not have the CRIS-specific modification affect a
> > > > continuing test-run (possibly for other targets).
> > > 
> > > this part isn't needed by design.  every .exp file calls `sim_init` which in
> > > turn calls `sim_init_toolchain` which fully initializes the toolchain for the
> > > current file.  so as soon as c.exp finishes running, whatever comes next is
> > > going to reset the flags you saved.
> > 
> > I know, I just didn't want to rely on that being the case.
> 
> i'm saying that the behavior isn't an accident.  it's designed this way.
> hence you aren't relying on incidental behavior, you're using the API as
> intended.  i explicitly went through and stripped out the save/restore
> logic from the tests because it was already inconsistently implemented.
> this way it's guaranteed to be consistent, and we don't need boilerplate.
> -mike

Ok then, I'll remove those bits.

brgds, H-P

Re: [PATCH 10/12] sim: Add sim_dump_memory for debugging

[Hans-Peter Nilsson via Gdb-patches]

> On 15 Feb 2022 00:06, Hans-Peter Nilsson via Gdb-patches wrote:
> > --- a/sim/common/sim-memopt.c
> > +++ b/sim/common/sim-memopt.c
> >
> > +/* Convenience function for use when debugging the simulator.  */
> 
> can you clarify that it's meant for an attached debugger to call vs a
> manual call from within the sim itself ?  i know your commit message
> said that, but the code comment would help too.
> 
> i vaguely recall seeing another such hook point in the
> tree.

That *could* have been in gdbinit.in a.k.a. .gdbinit.

Speaking of that, I just noticed there's no .gdbinit in
sim/<any> anymore.  They aren't build from sim/common/gdbinit.in
apparently due to some mixup of @SIM_COMMON_BUILD_FALSE@ and/or
@SIM_COMMON_BUILD_TRUE@ which seems to have been introduced in
your 36bb57e40c506.  (No, not planning on going there.)

brgds, H-P

Re: [PATCH 10/12] sim: Add sim_dump_memory for debugging

[Mike Frysinger via Gdb-patches]

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

On 17 Feb 2022 03:05, Hans-Peter Nilsson wrote:
> > On 15 Feb 2022 00:06, Hans-Peter Nilsson via Gdb-patches wrote:
> > > --- a/sim/common/sim-memopt.c
> > > +++ b/sim/common/sim-memopt.c
> > >
> > > +/* Convenience function for use when debugging the simulator.  */
> > 
> > can you clarify that it's meant for an attached debugger to call vs a
> > manual call from within the sim itself ?  i know your commit message
> > said that, but the code comment would help too.
> > 
> > i vaguely recall seeing another such hook point in the
> > tree.
> 
> That *could* have been in gdbinit.in a.k.a. .gdbinit.

hrm, maybe.  i want to say it was real code.  maybe in one of the ports.
at any rate, prob not worth diving deeper for now.

> Speaking of that, I just noticed there's no .gdbinit in
> sim/<any> anymore.  They aren't build from sim/common/gdbinit.in
> apparently due to some mixup of @SIM_COMMON_BUILD_FALSE@ and/or
> @SIM_COMMON_BUILD_TRUE@ which seems to have been introduced in
> your 36bb57e40c506.  (No, not planning on going there.)

thanks for pointing this out.  it was left behind because it has a configure
dependency on cgen ports and i hadn't figured out how to unwind that.  but
from playing around a bit, i think we can just always set the breakpoint.
if the symbol doesn't exist, gdb will warn, but that's it.  if you know how
to get it to not complain, i'm all ears.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]