<mmalcom...@nvidia.com> writes:
> From: Matthew Malcomson <mmalcom...@nvidia.com>
>
> 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.
>
> 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_target_compile): Take flags from TEST_ALWAYS_FLAGS.
>
> Signed-off-by: Matthew Malcomson <mmalcom...@nvidia.com>
> ---
> libitm/testsuite/lib/libitm.exp | 8 ++++++++
> libitm/testsuite/libitm.c++/c++.exp | 7 ++++---
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/libitm/testsuite/lib/libitm.exp b/libitm/testsuite/lib/libitm.exp
> index ac390d6d0dd..42a5aac4b0b 100644
> --- a/libitm/testsuite/lib/libitm.exp
> +++ b/libitm/testsuite/lib/libitm.exp
> @@ -77,6 +77,7 @@ proc libitm_init { args } {
> global blddir
> global gluefile wrap_flags
> global ALWAYS_CFLAGS
> + global TEST_ALWAYS_FLAGS
> global CFLAGS
> global TOOL_EXECUTABLE TOOL_OPTIONS
> global GCC_UNDER_TEST
> @@ -145,6 +146,9 @@ proc libitm_init { args } {
> }
> }
>
> + # 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.
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.
Thanks,
Richard
> set ALWAYS_CFLAGS ""
> if { $blddir != "" } {
> lappend ALWAYS_CFLAGS "additional_flags=-B${blddir}/"
> @@ -191,6 +195,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 +222,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 ab278f2cb33..d501e7e8170 100644
> --- a/libitm/testsuite/libitm.c++/c++.exp
> +++ b/libitm/testsuite/libitm.c++/c++.exp
> @@ -56,10 +56,10 @@ if { $lang_test_file_found } {
> # Gather a list of all tests.
> set tests [lsort [glob -nocomplain $srcdir/$subdir/*.C]]
>
> - set stdcxxadder ""
> + set saved_TEST_ALWAYS_FLAGS $TEST_ALWAYS_FLAGS
> 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 +74,8 @@ if { $lang_test_file_found } {
> }
>
> # Main loop.
> - dg-runtest $tests $stdcxxadder $libstdcxx_includes
> + dg-runtest $tests "" $libstdcxx_includes
> + set TEST_ALWAYS_FLAGS $saved_TEST_ALWAYS_FLAGS
> }
>
> # All done.
