On 1/2/25 12:08, Richard Sandiford wrote:
+ # This set in order to give libitm.c++/c++.exp a nicely named flag to set
+ # when adding C++ options.
+ set TEST_ALWAYS_FLAGS ""
This looked odd at first glance. By unconditionally writing "" to the
variable, it seems to subvert the save and restore done in c++.exp.
Yeah -- I see your point, that's not good.
How about instead copying the behaviour of asan_init and asan_finish,
so that libitm_init and libitm_finish do the save and restore? Or perhaps
a slight variation: after saving, libitm_init can set TEST_ALWAYS_FLAGS
to "" if TEST_ALWAYS_FLAGS was previously unset.
c++.exp would then not need to save and restore the flags itself, and
could still assume that TEST_ALWAYS_FLAGS is always set.
Have made the suggested change -- mentioning the extra little bit of
complexity that this introduced ...
Since libitm is a "tool" in the DejaGNU sense (while asan is not),
libitm_finish gets called twice for each libitm_init call.
The `runtest` procedure in DejaGNU's `runtest.exp` calls `${tool}_init`,
executes the c.exp or c++.exp test runner and then calls
`${tool}_finish`, while in each of the test runners we also call
`dg-finish` (as required by the dg.exp API) which calls `${tool}_finish`
directly.
This means using `libitm_finish` needs an extra bit in global state to
check whether we have already reset things.
- Has been set in libitm_init and was unset at start
=> saved_TEST_ALWAYS_FLAGS is unset.
- Has been set in libitm_init and was set at start
=> saved_TEST_ALWAYS_FLAGS is set.
- Has already been reset => some other flag.
Have attached the adjusted patch to this email.
From fbce3b25e8ccad80697f1596f566b268fff71221 Mon Sep 17 00:00:00 2001
From: Matthew Malcomson <mmalcom...@nvidia.com>
Date: Wed, 11 Dec 2024 11:03:55 +0000
Subject: [PATCH] testsuite: libitm: Adjust how libitm.c++ passes link flags
For the `gcc` and `g++` tools we often pass -B/path/to/object/dir in via
`TEST_ALWAYS_FLAGS` (see e.g. asan.exp where this is set).
In libitm.c++/c++.exp we pass that -B flag via the `tool_flags` argument
to `dg-runtest`.
Passing as the `tool_flags` argument means that these flags get added to
the name of the test. This means that if one were to compare the
testsuite results between runs made in different build directories
libitm.c++ gives a reasonable amount of NA->PASS and PASS->NA
differences even though the same tests passed each time.
This patch follows the example set in other parts of the testsuite like
asan_init and passes the -B arguments to the compiler via a global
variable called `TEST_ALWAYS_FLAGS`. For this DejaGNU "tool" we had to
newly initialise that variable in libitm_init and add a check against
that variable in libitm_target_compile. I thought about adding the
relevant flags we need for C++ into `ALWAYS_CFLAGS` but decided against
it since the name didn't match what we would be using it for.
We save the global `TEST_ALWAYS_FLAGS` in `libitm_init` and ensure
it's initialised. We then reset this in `libitm_finish`. Since
`libitm_finish` gets called twice (once from `dg-finish` and once from
the `runtest` procedure) we have to manage state to tell whether
TEST_ALWAYS_FLAGS has already been reset.
Testing done to bootstrap & regtest on AArch64. Manually observed that
the testsuite diff between two different build directories no longer
exists.
N.b. since I pass the new flags in the `ldflags` option of the DejaGNU
options while the previous code always passed this -B flag, the compile
test throwdown.C no longer gets compiled with this -B flag. I believe
that is not a problem.
libitm/ChangeLog:
* testsuite/libitm.c++/c++.exp: Use TEST_ALWAYS_FLAGS instead of
passing arguments to dg-runtest.
* testsuite/lib/libitm.exp (libitm_init): Initialise
TEST_ALWAYS_FLAGS.
(libitm_finish): Reset TEST_ALWAYS_FLAGS.
(libitm_target_compile): Take flags from TEST_ALWAYS_FLAGS.
Signed-off-by: Matthew Malcomson <mmalcom...@nvidia.com>
---
libitm/testsuite/lib/libitm.exp | 42 +++++++++++++++++++++++++++++
libitm/testsuite/libitm.c++/c++.exp | 5 ++--
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/libitm/testsuite/lib/libitm.exp b/libitm/testsuite/lib/libitm.exp
index ac390d6d0dd..c5b9bb1127f 100644
--- a/libitm/testsuite/lib/libitm.exp
+++ b/libitm/testsuite/lib/libitm.exp
@@ -59,6 +59,7 @@ set dg-do-what-default run
#
set libitm_compile_options ""
+set libitm_initialised 0
#
# libitm_init
@@ -77,12 +78,15 @@ proc libitm_init { args } {
global blddir
global gluefile wrap_flags
global ALWAYS_CFLAGS
+ global TEST_ALWAYS_FLAGS
+ global saved_TEST_ALWAYS_FLAGS
global CFLAGS
global TOOL_EXECUTABLE TOOL_OPTIONS
global GCC_UNDER_TEST
global TESTING_IN_BUILD_TREE
global target_triplet
global always_ld_library_path
+ global libitm_initialised
set blddir [lookfor_file [get_multilibs] libitm]
@@ -145,6 +149,13 @@ proc libitm_init { args } {
}
}
+ # This set in order to give libitm.c++/c++.exp a nicely named flag to set
+ # when adding C++ options.
+ if [info exists TEST_ALWAYS_FLAGS] {
+ set saved_TEST_ALWAYS_FLAGS $TEST_ALWAYS_FLAGS
+ } else {
+ set TEST_ALWAYS_FLAGS ""
+ }
set ALWAYS_CFLAGS ""
if { $blddir != "" } {
lappend ALWAYS_CFLAGS "additional_flags=-B${blddir}/"
@@ -180,6 +191,33 @@ proc libitm_init { args } {
lappend ALWAYS_CFLAGS "additional_flags=-fgnu-tm"
lappend ALWAYS_CFLAGS "additional_flags=-fdiagnostics-color=never"
+
+ set libitm_initialised 1
+}
+
+#
+# libitm_finish
+#
+proc libitm_finish { args } {
+ global TEST_ALWAYS_FLAGS
+ global saved_TEST_ALWAYS_FLAGS
+ global libitm_initialised
+ # Need two bits of information to tell difference between:
+ # - libitm_init has set TEST_ALWAYS_FLAGS and it was originally unset.
+ # - libitm_init has set TEST_ALWAYS_FLAGS and it was originally set.
+ # - libitm_finish has already been run and cleaned up what libitm_init had
+ # set.
+ if !$libitm_initialised {
+ return
+ }
+
+ if [info exists saved_TEST_ALWAYS_FLAGS] {
+ set TEST_ALWAYS_FLAGS $saved_TEST_ALWAYS_FLAGS
+ unset saved_TEST_ALWAYS_FLAGS
+ } else {
+ unset TEST_ALWAYS_FLAGS
+ }
+ set libitm_initialised 0
}
#
@@ -191,6 +229,7 @@ proc libitm_target_compile { source dest type options } {
global libitm_compile_options
global gluefile wrap_flags
global ALWAYS_CFLAGS
+ global TEST_ALWAYS_FLAGS
global GCC_UNDER_TEST
global lang_test_file
global lang_library_path
@@ -217,6 +256,9 @@ proc libitm_target_compile { source dest type options } {
if [info exists ALWAYS_CFLAGS] {
set options [concat "$ALWAYS_CFLAGS" $options]
}
+ if [info exists TEST_ALWAYS_FLAGS] {
+ set options [concat "$TEST_ALWAYS_FLAGS" $options]
+ }
set options [dg-additional-files-options $options $source $dest $type]
diff --git a/libitm/testsuite/libitm.c++/c++.exp b/libitm/testsuite/libitm.c++/c++.exp
index fdcfc28e1e0..7dbe75903d4 100644
--- a/libitm/testsuite/libitm.c++/c++.exp
+++ b/libitm/testsuite/libitm.c++/c++.exp
@@ -56,10 +56,9 @@ if { $lang_test_file_found } {
# Gather a list of all tests.
set tests [lsort [glob -nocomplain $srcdir/$subdir/*.C]]
- set stdcxxadder ""
if { $blddir != "" } {
set ld_library_path "$always_ld_library_path:${blddir}/${lang_library_path}"
- set stdcxxadder "-B ${blddir}/${lang_library_path}"
+ set TEST_ALWAYS_FLAGS "$TEST_ALWAYS_FLAGS ldflags=-B${blddir}/${lang_library_path}"
} else {
set ld_library_path "$always_ld_library_path"
}
@@ -74,7 +73,7 @@ if { $lang_test_file_found } {
}
# Main loop.
- dg-runtest $tests $stdcxxadder $libstdcxx_includes
+ dg-runtest $tests "" $libstdcxx_includes
}
# All done.
--
2.43.0
