This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH, testsuite] Fix no_trampolines test in check_effective_target_trampolines


Hi,

Consider check_effective_target_trampolines:
...
# Return 1 if according to target_info struct and explicit target list # target is supposed to support trampolines.

proc check_effective_target_trampolines { } {
    if [target_info exists no_trampolines] {
      return 0
    }
    if { [istarget avr-*-*]
         || [istarget msp430-*-*]
         || [istarget hppa2.0w-hp-hpux11.23]
         || [istarget hppa64-hp-hpux11.23] } {
        return 0;
    }
    return 1
}
...

If I disable the nvptx target test in check_effective_target_trampolines and run tests for target nvptx, then the proc returns true instead of false, and I get 'UNSUPPORTED -> FAIL' changes for tests that require the effective target.

This is due to the fact that https://github.com/MentorEmbedded/nvptx-tools/blob/master/nvptx-none-run.exp defines 'gcc,no_trampolines':
...
set_board_info gcc,no_trampolines 1
...
but check_effective_target_trampolines tests no_trampolines (without the 'gcc,' in front):

The effective target trampolines was introduced in 2008, but we've been testing 'gcc,no_trampolines' in gcc_target_compile since at least 1997. [ To complicate matters objc_target_compile tests for 'objc,no_trampolines' to set -DNO_TRAMPOLINES, but AFAICT that macro is not used anywhere in the objc test suites, so I think that's dead code. ]

The original submission of the keyword is here ( https://gcc.gnu.org/ml/gcc-patches/2005-04/msg01925.html ) and uses 'target_info exists no_trampolines'. But in a follow-up comment ( https://gcc.gnu.org/ml/gcc-patches/2005-04/msg01978.html ) there is the suggestion to add 'set_board_info gcc,no_trampolines 1' to the board file to trigger the test in check_effective_target_trampolines. So that sounds like the missing 'gcc,' is accidental rather than intentional.

In a further follow-up comment ( https://gcc.gnu.org/ml/gcc-patches/2005-04/msg02063.html ) Mike suggest to test for target triplets, which indeed was added in the next version. This probably explains why the missing 'gcc,' wasn't found in any further testing.

As things are now, boards for targets that are not listed in check_effective_target_trampolines but still want the no_trampolines behavior need to do:
- 'set_board_info gcc,no_trampolines 1' to trigger the test in
  gcc_target_compile and add -DNO_TRAMPOLINES to the flags
- 'set_board_info no_trampolines 1' to trigger the test in
  check_effective_target_trampolines

Given that:
- it's better to have to define one variable than two
- it looks like an accident that the 'gcc,' was dropped
- the one with the 'gcc,' prefix has been around longer, and is
  mentioned in dejagnu docs
I propose to test for 'gcc,no_trampolines' instead of 'no_trampolines' in check_effective_target_trampolines.

I don't think a backward compatibility test for 'no_trampolines' is required, because the affected boardfiles most likely already define both.

Tested by checking that the patch fixes the 'UNSUPPORTED -> FAIL' regression mentioned above for a single testcase.

OK for trunk?

Thanks,
- Tom
Fix no_trampolines test in check_effective_target_trampolines

2017-06-08  Tom de Vries  <tom@codesourcery.com>

	* lib/target-supports.exp (check_effective_target_trampolines): Test for
	'gcc,no_trampolines' instead of 'no_trampolines'.

---
 gcc/testsuite/lib/target-supports.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 8b99f35..d0b35be 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -491,7 +491,7 @@ proc check_gc_sections_available { } {
 # target is supposed to support trampolines.
  
 proc check_effective_target_trampolines { } {
-    if [target_info exists no_trampolines] {
+    if [target_info exists gcc,no_trampolines] {
       return 0
     }
     if { [istarget avr-*-*]

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]