* [PATCH] testsuite: gluefile file need to be prefixed
@ 2022-09-09 13:03 Torbjörn SVENSSON
2022-09-12 11:56 ` Richard Sandiford
0 siblings, 1 reply; 5+ messages in thread
From: Torbjörn SVENSSON @ 2022-09-09 13:03 UTC (permalink / raw)
To: gcc-patches; +Cc: yvan.roux, aoliva, ro, mikestump, Torbjörn SVENSSON
PR/95720
When the status wrapper is used, the gluefile need to be prefixed with
-Wl, in order for the test cases to have the dump files with the
expected names.
gcc/testsuite/ChangeLog:
* gcc/testsuite/lib/g++.exp: Moved gluefile block to after
flags have been prefixed for the target_compile call.
* gcc/testsuite/lib/gcc.exp: Likewise.
* gcc/testsuite/lib/wrapper.exp: Reset adjusted state flag.
Co-Authored-By: Yvan ROUX <yvan.roux@foss.st.com>
Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
gcc/testsuite/lib/g++.exp | 10 +++++-----
gcc/testsuite/lib/gcc.exp | 21 +++++++++++----------
gcc/testsuite/lib/wrapper.exp | 7 ++++++-
3 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/gcc/testsuite/lib/g++.exp b/gcc/testsuite/lib/g++.exp
index 24ef068b239..16e61fb4ad4 100644
--- a/gcc/testsuite/lib/g++.exp
+++ b/gcc/testsuite/lib/g++.exp
@@ -303,11 +303,6 @@ proc g++_target_compile { source dest type options } {
global flags_to_postpone
global board_info
- if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } {
- lappend options "libs=${gluefile}"
- lappend options "ldflags=${wrap_flags}"
- }
-
global TEST_EXTRA_LIBS
if [info exists TEST_EXTRA_LIBS] {
lappend options "ldflags=$TEST_EXTRA_LIBS"
@@ -333,6 +328,11 @@ proc g++_target_compile { source dest type options } {
set options [dg-additional-files-options $options $source]
+ if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } {
+ lappend options "libs=${gluefile}"
+ lappend options "ldflags=${wrap_flags}"
+ }
+
set result [target_compile $source $dest $type $options]
if {[board_info $tboard exists multilib_flags]} {
diff --git a/gcc/testsuite/lib/gcc.exp b/gcc/testsuite/lib/gcc.exp
index 1b25ebec4cf..2f145d0fdf4 100644
--- a/gcc/testsuite/lib/gcc.exp
+++ b/gcc/testsuite/lib/gcc.exp
@@ -129,16 +129,6 @@ proc gcc_target_compile { source dest type options } {
global flags_to_postpone
global board_info
- if {[target_info needs_status_wrapper] != "" && \
- [target_info needs_status_wrapper] != "0" && \
- [info exists gluefile] } {
- lappend options "libs=${gluefile}"
- lappend options "ldflags=$wrap_flags"
- if { $type == "executable" } {
- set options [concat "{additional_flags=-dumpbase \"\"}" $options]
- }
- }
-
global TEST_EXTRA_LIBS
if [info exists TEST_EXTRA_LIBS] {
lappend options "ldflags=$TEST_EXTRA_LIBS"
@@ -170,6 +160,17 @@ proc gcc_target_compile { source dest type options } {
lappend options "timeout=[timeout_value]"
lappend options "compiler=$GCC_UNDER_TEST"
set options [dg-additional-files-options $options $source]
+
+ if {[target_info needs_status_wrapper] != "" && \
+ [target_info needs_status_wrapper] != "0" && \
+ [info exists gluefile] } {
+ lappend options "libs=${gluefile}"
+ lappend options "ldflags=$wrap_flags"
+ if { $type == "executable" } {
+ set options [concat "{additional_flags=-dumpbase \"\"}" $options]
+ }
+ }
+
set return_val [target_compile $source $dest $type $options]
if {[board_info $tboard exists multilib_flags]} {
diff --git a/gcc/testsuite/lib/wrapper.exp b/gcc/testsuite/lib/wrapper.exp
index 5a601b269da..4a7d56941fc 100644
--- a/gcc/testsuite/lib/wrapper.exp
+++ b/gcc/testsuite/lib/wrapper.exp
@@ -22,7 +22,7 @@
# the compiler when compiling FILENAME.
proc ${tool}_maybe_build_wrapper { filename args } {
- global gluefile wrap_flags
+ global gluefile wrap_flags gcc_adjusted_linker_flags
if { [target_info needs_status_wrapper] != "" \
&& [target_info needs_status_wrapper] != "0" \
@@ -43,6 +43,11 @@ proc ${tool}_maybe_build_wrapper { filename args } {
if { $result != "" } {
set gluefile [lindex $result 0]
set wrap_flags [lindex $result 1]
+
+ # Reset the cached state of the adjusted flags
+ if { [info exists gcc_adjusted_linker_flags] } {
+ set gcc_adjusted_linker_flags 0
+ }
}
}
}
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] testsuite: gluefile file need to be prefixed 2022-09-09 13:03 [PATCH] testsuite: gluefile file need to be prefixed Torbjörn SVENSSON @ 2022-09-12 11:56 ` Richard Sandiford 2022-09-12 15:55 ` Yvan Roux 0 siblings, 1 reply; 5+ messages in thread From: Richard Sandiford @ 2022-09-12 11:56 UTC (permalink / raw) To: Torbjörn SVENSSON via Gcc-patches; +Cc: Torbjörn SVENSSON, aoliva Torbjörn SVENSSON via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > PR/95720 > When the status wrapper is used, the gluefile need to be prefixed with > -Wl, in order for the test cases to have the dump files with the > expected names. > > gcc/testsuite/ChangeLog: > > * gcc/testsuite/lib/g++.exp: Moved gluefile block to after > flags have been prefixed for the target_compile call. > * gcc/testsuite/lib/gcc.exp: Likewise. > * gcc/testsuite/lib/wrapper.exp: Reset adjusted state flag. I agree this looks right, but I'm curious how you ended up with the behaviour seen in comment 16. I thought having: if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { lappend options "libs=${gluefile}" lappend options "ldflags=${wrap_flags}" } before the dg-additional-files-options/gcc_adjusted_linker_flags would lead to the gluefile missing a "-Wl," first time round. How is it that the gluefile still gets the "-Wl," and the -T misses it instead? Thanks, Richard > Co-Authored-By: Yvan ROUX <yvan.roux@foss.st.com> > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com> > --- > gcc/testsuite/lib/g++.exp | 10 +++++----- > gcc/testsuite/lib/gcc.exp | 21 +++++++++++---------- > gcc/testsuite/lib/wrapper.exp | 7 ++++++- > 3 files changed, 22 insertions(+), 16 deletions(-) > > diff --git a/gcc/testsuite/lib/g++.exp b/gcc/testsuite/lib/g++.exp > index 24ef068b239..16e61fb4ad4 100644 > --- a/gcc/testsuite/lib/g++.exp > +++ b/gcc/testsuite/lib/g++.exp > @@ -303,11 +303,6 @@ proc g++_target_compile { source dest type options } { > global flags_to_postpone > global board_info > > - if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { > - lappend options "libs=${gluefile}" > - lappend options "ldflags=${wrap_flags}" > - } > - > global TEST_EXTRA_LIBS > if [info exists TEST_EXTRA_LIBS] { > lappend options "ldflags=$TEST_EXTRA_LIBS" > @@ -333,6 +328,11 @@ proc g++_target_compile { source dest type options } { > > set options [dg-additional-files-options $options $source] > > + if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { > + lappend options "libs=${gluefile}" > + lappend options "ldflags=${wrap_flags}" > + } > + > set result [target_compile $source $dest $type $options] > > if {[board_info $tboard exists multilib_flags]} { > diff --git a/gcc/testsuite/lib/gcc.exp b/gcc/testsuite/lib/gcc.exp > index 1b25ebec4cf..2f145d0fdf4 100644 > --- a/gcc/testsuite/lib/gcc.exp > +++ b/gcc/testsuite/lib/gcc.exp > @@ -129,16 +129,6 @@ proc gcc_target_compile { source dest type options } { > global flags_to_postpone > global board_info > > - if {[target_info needs_status_wrapper] != "" && \ > - [target_info needs_status_wrapper] != "0" && \ > - [info exists gluefile] } { > - lappend options "libs=${gluefile}" > - lappend options "ldflags=$wrap_flags" > - if { $type == "executable" } { > - set options [concat "{additional_flags=-dumpbase \"\"}" $options] > - } > - } > - > global TEST_EXTRA_LIBS > if [info exists TEST_EXTRA_LIBS] { > lappend options "ldflags=$TEST_EXTRA_LIBS" > @@ -170,6 +160,17 @@ proc gcc_target_compile { source dest type options } { > lappend options "timeout=[timeout_value]" > lappend options "compiler=$GCC_UNDER_TEST" > set options [dg-additional-files-options $options $source] > + > + if {[target_info needs_status_wrapper] != "" && \ > + [target_info needs_status_wrapper] != "0" && \ > + [info exists gluefile] } { > + lappend options "libs=${gluefile}" > + lappend options "ldflags=$wrap_flags" > + if { $type == "executable" } { > + set options [concat "{additional_flags=-dumpbase \"\"}" $options] > + } > + } > + > set return_val [target_compile $source $dest $type $options] > > if {[board_info $tboard exists multilib_flags]} { > diff --git a/gcc/testsuite/lib/wrapper.exp b/gcc/testsuite/lib/wrapper.exp > index 5a601b269da..4a7d56941fc 100644 > --- a/gcc/testsuite/lib/wrapper.exp > +++ b/gcc/testsuite/lib/wrapper.exp > @@ -22,7 +22,7 @@ > # the compiler when compiling FILENAME. > > proc ${tool}_maybe_build_wrapper { filename args } { > - global gluefile wrap_flags > + global gluefile wrap_flags gcc_adjusted_linker_flags > > if { [target_info needs_status_wrapper] != "" \ > && [target_info needs_status_wrapper] != "0" \ > @@ -43,6 +43,11 @@ proc ${tool}_maybe_build_wrapper { filename args } { > if { $result != "" } { > set gluefile [lindex $result 0] > set wrap_flags [lindex $result 1] > + > + # Reset the cached state of the adjusted flags > + if { [info exists gcc_adjusted_linker_flags] } { > + set gcc_adjusted_linker_flags 0 > + } > } > } > } ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] testsuite: gluefile file need to be prefixed 2022-09-12 11:56 ` Richard Sandiford @ 2022-09-12 15:55 ` Yvan Roux 2022-09-12 16:09 ` Richard Sandiford 0 siblings, 1 reply; 5+ messages in thread From: Yvan Roux @ 2022-09-12 15:55 UTC (permalink / raw) To: gcc-patches, Torbjörn SVENSSON, aoliva, richard.sandiford Hi Richard, On Mon, Sep 12, 2022 at 12:56:52PM +0100, Richard Sandiford via Gcc-patches wrote: > Torbjörn SVENSSON via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > > PR/95720 > > When the status wrapper is used, the gluefile need to be prefixed with > > -Wl, in order for the test cases to have the dump files with the > > expected names. > > > > gcc/testsuite/ChangeLog: > > > > * gcc/testsuite/lib/g++.exp: Moved gluefile block to after > > flags have been prefixed for the target_compile call. > > * gcc/testsuite/lib/gcc.exp: Likewise. > > * gcc/testsuite/lib/wrapper.exp: Reset adjusted state flag. > > I agree this looks right, but I'm curious how you ended up with the > behaviour seen in comment 16. I thought having: > > if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { > lappend options "libs=${gluefile}" > lappend options "ldflags=${wrap_flags}" > } > > before the dg-additional-files-options/gcc_adjusted_linker_flags would > lead to the gluefile missing a "-Wl," first time round. How is it that > the gluefile still gets the "-Wl," and the -T misses it instead? The issue is that dg-additional-files-options/gcc_adjusted_linker_flags add the "-Wl," prefix to the content of the global variable <gluefile> and not to the one of <option>, thus if this chunk of code is placed beforei, <options> is already populated with "libs=$gluefile" and lacks the prefix. Notice that this is the case only for the first test compiled in a tested .exp file, the next ones will have the adjusted gluefile content. For -T, this is another issue not covered by this patch, it is not handled by <gcc_adjust_linker_flags_list> like "-Xlinker" thus a "-Wl," prefix is placed between -T and the linker file, we'll send another patch to fix that issue. Thanks, Yvan > Thanks, > Richard > > > Co-Authored-By: Yvan ROUX <yvan.roux@foss.st.com> > > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com> > > --- > > gcc/testsuite/lib/g++.exp | 10 +++++----- > > gcc/testsuite/lib/gcc.exp | 21 +++++++++++---------- > > gcc/testsuite/lib/wrapper.exp | 7 ++++++- > > 3 files changed, 22 insertions(+), 16 deletions(-) > > > > diff --git a/gcc/testsuite/lib/g++.exp b/gcc/testsuite/lib/g++.exp > > index 24ef068b239..16e61fb4ad4 100644 > > --- a/gcc/testsuite/lib/g++.exp > > +++ b/gcc/testsuite/lib/g++.exp > > @@ -303,11 +303,6 @@ proc g++_target_compile { source dest type options } { > > global flags_to_postpone > > global board_info > > > > - if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { > > - lappend options "libs=${gluefile}" > > - lappend options "ldflags=${wrap_flags}" > > - } > > - > > global TEST_EXTRA_LIBS > > if [info exists TEST_EXTRA_LIBS] { > > lappend options "ldflags=$TEST_EXTRA_LIBS" > > @@ -333,6 +328,11 @@ proc g++_target_compile { source dest type options } { > > > > set options [dg-additional-files-options $options $source] > > > > + if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { > > + lappend options "libs=${gluefile}" > > + lappend options "ldflags=${wrap_flags}" > > + } > > + > > set result [target_compile $source $dest $type $options] > > > > if {[board_info $tboard exists multilib_flags]} { > > diff --git a/gcc/testsuite/lib/gcc.exp b/gcc/testsuite/lib/gcc.exp > > index 1b25ebec4cf..2f145d0fdf4 100644 > > --- a/gcc/testsuite/lib/gcc.exp > > +++ b/gcc/testsuite/lib/gcc.exp > > @@ -129,16 +129,6 @@ proc gcc_target_compile { source dest type options } { > > global flags_to_postpone > > global board_info > > > > - if {[target_info needs_status_wrapper] != "" && \ > > - [target_info needs_status_wrapper] != "0" && \ > > - [info exists gluefile] } { > > - lappend options "libs=${gluefile}" > > - lappend options "ldflags=$wrap_flags" > > - if { $type == "executable" } { > > - set options [concat "{additional_flags=-dumpbase \"\"}" $options] > > - } > > - } > > - > > global TEST_EXTRA_LIBS > > if [info exists TEST_EXTRA_LIBS] { > > lappend options "ldflags=$TEST_EXTRA_LIBS" > > @@ -170,6 +160,17 @@ proc gcc_target_compile { source dest type options } { > > lappend options "timeout=[timeout_value]" > > lappend options "compiler=$GCC_UNDER_TEST" > > set options [dg-additional-files-options $options $source] > > + > > + if {[target_info needs_status_wrapper] != "" && \ > > + [target_info needs_status_wrapper] != "0" && \ > > + [info exists gluefile] } { > > + lappend options "libs=${gluefile}" > > + lappend options "ldflags=$wrap_flags" > > + if { $type == "executable" } { > > + set options [concat "{additional_flags=-dumpbase \"\"}" $options] > > + } > > + } > > + > > set return_val [target_compile $source $dest $type $options] > > > > if {[board_info $tboard exists multilib_flags]} { > > diff --git a/gcc/testsuite/lib/wrapper.exp b/gcc/testsuite/lib/wrapper.exp > > index 5a601b269da..4a7d56941fc 100644 > > --- a/gcc/testsuite/lib/wrapper.exp > > +++ b/gcc/testsuite/lib/wrapper.exp > > @@ -22,7 +22,7 @@ > > # the compiler when compiling FILENAME. > > > > proc ${tool}_maybe_build_wrapper { filename args } { > > - global gluefile wrap_flags > > + global gluefile wrap_flags gcc_adjusted_linker_flags > > > > if { [target_info needs_status_wrapper] != "" \ > > && [target_info needs_status_wrapper] != "0" \ > > @@ -43,6 +43,11 @@ proc ${tool}_maybe_build_wrapper { filename args } { > > if { $result != "" } { > > set gluefile [lindex $result 0] > > set wrap_flags [lindex $result 1] > > + > > + # Reset the cached state of the adjusted flags > > + if { [info exists gcc_adjusted_linker_flags] } { > > + set gcc_adjusted_linker_flags 0 > > + } > > } > > } > > } -- Y. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] testsuite: gluefile file need to be prefixed 2022-09-12 15:55 ` Yvan Roux @ 2022-09-12 16:09 ` Richard Sandiford 2022-09-12 16:29 ` Yvan Roux 0 siblings, 1 reply; 5+ messages in thread From: Richard Sandiford @ 2022-09-12 16:09 UTC (permalink / raw) To: Yvan Roux; +Cc: gcc-patches, Torbjörn SVENSSON, aoliva Yvan Roux <yvan.roux@foss.st.com> writes: > Hi Richard, > On Mon, Sep 12, 2022 at 12:56:52PM +0100, Richard Sandiford via Gcc-patches wrote: >> Torbjörn SVENSSON via Gcc-patches <gcc-patches@gcc.gnu.org> writes: >> > PR/95720 >> > When the status wrapper is used, the gluefile need to be prefixed with >> > -Wl, in order for the test cases to have the dump files with the >> > expected names. >> > >> > gcc/testsuite/ChangeLog: >> > >> > * gcc/testsuite/lib/g++.exp: Moved gluefile block to after >> > flags have been prefixed for the target_compile call. >> > * gcc/testsuite/lib/gcc.exp: Likewise. >> > * gcc/testsuite/lib/wrapper.exp: Reset adjusted state flag. >> >> I agree this looks right, but I'm curious how you ended up with the >> behaviour seen in comment 16. I thought having: >> >> if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { >> lappend options "libs=${gluefile}" >> lappend options "ldflags=${wrap_flags}" >> } >> >> before the dg-additional-files-options/gcc_adjusted_linker_flags would >> lead to the gluefile missing a "-Wl," first time round. How is it that >> the gluefile still gets the "-Wl," and the -T misses it instead? > > The issue is that dg-additional-files-options/gcc_adjusted_linker_flags > add the "-Wl," prefix to the content of the global variable <gluefile> > and not to the one of <option>, thus if this chunk of code is placed > beforei, <options> is already populated with "libs=$gluefile" and lacks > the prefix. Notice that this is the case only for the first test > compiled in a tested .exp file, the next ones will have the adjusted > gluefile content. > > For -T, this is another issue not covered by this patch, it is not > handled by <gcc_adjust_linker_flags_list> like "-Xlinker" thus a "-Wl," > prefix is placed between -T and the linker file, we'll send another > patch to fix that issue. Ah, OK, I thought the patch was fixing the -T issue too. If it's just fixing the ordering problem then it all makes sense. :-) So yeah, the patch is OK for trunk, thanks. Please follow the instructions on https://gcc.gnu.org/gitwrite.html to get write access (listing me as sponsor), but I can commit it for you if you prefer. Richard > > Thanks, > Yvan > >> Thanks, >> Richard >> >> > Co-Authored-By: Yvan ROUX <yvan.roux@foss.st.com> >> > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com> >> > --- >> > gcc/testsuite/lib/g++.exp | 10 +++++----- >> > gcc/testsuite/lib/gcc.exp | 21 +++++++++++---------- >> > gcc/testsuite/lib/wrapper.exp | 7 ++++++- >> > 3 files changed, 22 insertions(+), 16 deletions(-) >> > >> > diff --git a/gcc/testsuite/lib/g++.exp b/gcc/testsuite/lib/g++.exp >> > index 24ef068b239..16e61fb4ad4 100644 >> > --- a/gcc/testsuite/lib/g++.exp >> > +++ b/gcc/testsuite/lib/g++.exp >> > @@ -303,11 +303,6 @@ proc g++_target_compile { source dest type options } { >> > global flags_to_postpone >> > global board_info >> > >> > - if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { >> > - lappend options "libs=${gluefile}" >> > - lappend options "ldflags=${wrap_flags}" >> > - } >> > - >> > global TEST_EXTRA_LIBS >> > if [info exists TEST_EXTRA_LIBS] { >> > lappend options "ldflags=$TEST_EXTRA_LIBS" >> > @@ -333,6 +328,11 @@ proc g++_target_compile { source dest type options } { >> > >> > set options [dg-additional-files-options $options $source] >> > >> > + if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { >> > + lappend options "libs=${gluefile}" >> > + lappend options "ldflags=${wrap_flags}" >> > + } >> > + >> > set result [target_compile $source $dest $type $options] >> > >> > if {[board_info $tboard exists multilib_flags]} { >> > diff --git a/gcc/testsuite/lib/gcc.exp b/gcc/testsuite/lib/gcc.exp >> > index 1b25ebec4cf..2f145d0fdf4 100644 >> > --- a/gcc/testsuite/lib/gcc.exp >> > +++ b/gcc/testsuite/lib/gcc.exp >> > @@ -129,16 +129,6 @@ proc gcc_target_compile { source dest type options } { >> > global flags_to_postpone >> > global board_info >> > >> > - if {[target_info needs_status_wrapper] != "" && \ >> > - [target_info needs_status_wrapper] != "0" && \ >> > - [info exists gluefile] } { >> > - lappend options "libs=${gluefile}" >> > - lappend options "ldflags=$wrap_flags" >> > - if { $type == "executable" } { >> > - set options [concat "{additional_flags=-dumpbase \"\"}" $options] >> > - } >> > - } >> > - >> > global TEST_EXTRA_LIBS >> > if [info exists TEST_EXTRA_LIBS] { >> > lappend options "ldflags=$TEST_EXTRA_LIBS" >> > @@ -170,6 +160,17 @@ proc gcc_target_compile { source dest type options } { >> > lappend options "timeout=[timeout_value]" >> > lappend options "compiler=$GCC_UNDER_TEST" >> > set options [dg-additional-files-options $options $source] >> > + >> > + if {[target_info needs_status_wrapper] != "" && \ >> > + [target_info needs_status_wrapper] != "0" && \ >> > + [info exists gluefile] } { >> > + lappend options "libs=${gluefile}" >> > + lappend options "ldflags=$wrap_flags" >> > + if { $type == "executable" } { >> > + set options [concat "{additional_flags=-dumpbase \"\"}" $options] >> > + } >> > + } >> > + >> > set return_val [target_compile $source $dest $type $options] >> > >> > if {[board_info $tboard exists multilib_flags]} { >> > diff --git a/gcc/testsuite/lib/wrapper.exp b/gcc/testsuite/lib/wrapper.exp >> > index 5a601b269da..4a7d56941fc 100644 >> > --- a/gcc/testsuite/lib/wrapper.exp >> > +++ b/gcc/testsuite/lib/wrapper.exp >> > @@ -22,7 +22,7 @@ >> > # the compiler when compiling FILENAME. >> > >> > proc ${tool}_maybe_build_wrapper { filename args } { >> > - global gluefile wrap_flags >> > + global gluefile wrap_flags gcc_adjusted_linker_flags >> > >> > if { [target_info needs_status_wrapper] != "" \ >> > && [target_info needs_status_wrapper] != "0" \ >> > @@ -43,6 +43,11 @@ proc ${tool}_maybe_build_wrapper { filename args } { >> > if { $result != "" } { >> > set gluefile [lindex $result 0] >> > set wrap_flags [lindex $result 1] >> > + >> > + # Reset the cached state of the adjusted flags >> > + if { [info exists gcc_adjusted_linker_flags] } { >> > + set gcc_adjusted_linker_flags 0 >> > + } >> > } >> > } >> > } ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] testsuite: gluefile file need to be prefixed 2022-09-12 16:09 ` Richard Sandiford @ 2022-09-12 16:29 ` Yvan Roux 0 siblings, 0 replies; 5+ messages in thread From: Yvan Roux @ 2022-09-12 16:29 UTC (permalink / raw) To: gcc-patches, Torbjörn SVENSSON, aoliva, richard.sandiford On Mon, Sep 12, 2022 at 05:09:57PM +0100, Richard Sandiford wrote: > Yvan Roux <yvan.roux@foss.st.com> writes: > > Hi Richard, > > On Mon, Sep 12, 2022 at 12:56:52PM +0100, Richard Sandiford via Gcc-patches wrote: > >> Torbjörn SVENSSON via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > >> > PR/95720 > >> > When the status wrapper is used, the gluefile need to be prefixed with > >> > -Wl, in order for the test cases to have the dump files with the > >> > expected names. > >> > > >> > gcc/testsuite/ChangeLog: > >> > > >> > * gcc/testsuite/lib/g++.exp: Moved gluefile block to after > >> > flags have been prefixed for the target_compile call. > >> > * gcc/testsuite/lib/gcc.exp: Likewise. > >> > * gcc/testsuite/lib/wrapper.exp: Reset adjusted state flag. > >> > >> I agree this looks right, but I'm curious how you ended up with the > >> behaviour seen in comment 16. I thought having: > >> > >> if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { > >> lappend options "libs=${gluefile}" > >> lappend options "ldflags=${wrap_flags}" > >> } > >> > >> before the dg-additional-files-options/gcc_adjusted_linker_flags would > >> lead to the gluefile missing a "-Wl," first time round. How is it that > >> the gluefile still gets the "-Wl," and the -T misses it instead? > > > > The issue is that dg-additional-files-options/gcc_adjusted_linker_flags > > add the "-Wl," prefix to the content of the global variable <gluefile> > > and not to the one of <option>, thus if this chunk of code is placed > > beforei, <options> is already populated with "libs=$gluefile" and lacks > > the prefix. Notice that this is the case only for the first test > > compiled in a tested .exp file, the next ones will have the adjusted > > gluefile content. > > > > For -T, this is another issue not covered by this patch, it is not > > handled by <gcc_adjust_linker_flags_list> like "-Xlinker" thus a "-Wl," > > prefix is placed between -T and the linker file, we'll send another > > patch to fix that issue. > > Ah, OK, I thought the patch was fixing the -T issue too. > If it's just fixing the ordering problem then it all makes sense. :-) > > So yeah, the patch is OK for trunk, thanks. Please follow the > instructions on https://gcc.gnu.org/gitwrite.html to get write access > (listing me as sponsor), but I can commit it for you if you prefer. > > Richard Thanks Richard, I've GCC write access, even if I didn't work on it for several years, I'll try to commit it myself tomorrow to check if it still works ;) but maybe Torbjorn would need a sponsorship for write access. Cheers, Yvan > > > > Thanks, > > Yvan > > > >> Thanks, > >> Richard > >> > >> > Co-Authored-By: Yvan ROUX <yvan.roux@foss.st.com> > >> > Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com> > >> > --- > >> > gcc/testsuite/lib/g++.exp | 10 +++++----- > >> > gcc/testsuite/lib/gcc.exp | 21 +++++++++++---------- > >> > gcc/testsuite/lib/wrapper.exp | 7 ++++++- > >> > 3 files changed, 22 insertions(+), 16 deletions(-) > >> > > >> > diff --git a/gcc/testsuite/lib/g++.exp b/gcc/testsuite/lib/g++.exp > >> > index 24ef068b239..16e61fb4ad4 100644 > >> > --- a/gcc/testsuite/lib/g++.exp > >> > +++ b/gcc/testsuite/lib/g++.exp > >> > @@ -303,11 +303,6 @@ proc g++_target_compile { source dest type options } { > >> > global flags_to_postpone > >> > global board_info > >> > > >> > - if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { > >> > - lappend options "libs=${gluefile}" > >> > - lappend options "ldflags=${wrap_flags}" > >> > - } > >> > - > >> > global TEST_EXTRA_LIBS > >> > if [info exists TEST_EXTRA_LIBS] { > >> > lappend options "ldflags=$TEST_EXTRA_LIBS" > >> > @@ -333,6 +328,11 @@ proc g++_target_compile { source dest type options } { > >> > > >> > set options [dg-additional-files-options $options $source] > >> > > >> > + if { [target_info needs_status_wrapper] != "" && [info exists gluefile] } { > >> > + lappend options "libs=${gluefile}" > >> > + lappend options "ldflags=${wrap_flags}" > >> > + } > >> > + > >> > set result [target_compile $source $dest $type $options] > >> > > >> > if {[board_info $tboard exists multilib_flags]} { > >> > diff --git a/gcc/testsuite/lib/gcc.exp b/gcc/testsuite/lib/gcc.exp > >> > index 1b25ebec4cf..2f145d0fdf4 100644 > >> > --- a/gcc/testsuite/lib/gcc.exp > >> > +++ b/gcc/testsuite/lib/gcc.exp > >> > @@ -129,16 +129,6 @@ proc gcc_target_compile { source dest type options } { > >> > global flags_to_postpone > >> > global board_info > >> > > >> > - if {[target_info needs_status_wrapper] != "" && \ > >> > - [target_info needs_status_wrapper] != "0" && \ > >> > - [info exists gluefile] } { > >> > - lappend options "libs=${gluefile}" > >> > - lappend options "ldflags=$wrap_flags" > >> > - if { $type == "executable" } { > >> > - set options [concat "{additional_flags=-dumpbase \"\"}" $options] > >> > - } > >> > - } > >> > - > >> > global TEST_EXTRA_LIBS > >> > if [info exists TEST_EXTRA_LIBS] { > >> > lappend options "ldflags=$TEST_EXTRA_LIBS" > >> > @@ -170,6 +160,17 @@ proc gcc_target_compile { source dest type options } { > >> > lappend options "timeout=[timeout_value]" > >> > lappend options "compiler=$GCC_UNDER_TEST" > >> > set options [dg-additional-files-options $options $source] > >> > + > >> > + if {[target_info needs_status_wrapper] != "" && \ > >> > + [target_info needs_status_wrapper] != "0" && \ > >> > + [info exists gluefile] } { > >> > + lappend options "libs=${gluefile}" > >> > + lappend options "ldflags=$wrap_flags" > >> > + if { $type == "executable" } { > >> > + set options [concat "{additional_flags=-dumpbase \"\"}" $options] > >> > + } > >> > + } > >> > + > >> > set return_val [target_compile $source $dest $type $options] > >> > > >> > if {[board_info $tboard exists multilib_flags]} { > >> > diff --git a/gcc/testsuite/lib/wrapper.exp b/gcc/testsuite/lib/wrapper.exp > >> > index 5a601b269da..4a7d56941fc 100644 > >> > --- a/gcc/testsuite/lib/wrapper.exp > >> > +++ b/gcc/testsuite/lib/wrapper.exp > >> > @@ -22,7 +22,7 @@ > >> > # the compiler when compiling FILENAME. > >> > > >> > proc ${tool}_maybe_build_wrapper { filename args } { > >> > - global gluefile wrap_flags > >> > + global gluefile wrap_flags gcc_adjusted_linker_flags > >> > > >> > if { [target_info needs_status_wrapper] != "" \ > >> > && [target_info needs_status_wrapper] != "0" \ > >> > @@ -43,6 +43,11 @@ proc ${tool}_maybe_build_wrapper { filename args } { > >> > if { $result != "" } { > >> > set gluefile [lindex $result 0] > >> > set wrap_flags [lindex $result 1] > >> > + > >> > + # Reset the cached state of the adjusted flags > >> > + if { [info exists gcc_adjusted_linker_flags] } { > >> > + set gcc_adjusted_linker_flags 0 > >> > + } > >> > } > >> > } > >> > } -- Y. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-09-12 16:29 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-09 13:03 [PATCH] testsuite: gluefile file need to be prefixed Torbjörn SVENSSON 2022-09-12 11:56 ` Richard Sandiford 2022-09-12 15:55 ` Yvan Roux 2022-09-12 16:09 ` Richard Sandiford 2022-09-12 16:29 ` Yvan Roux
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).