* [testsuite] Clean up effective_target cache
@ 2015-08-25 8:17 Christophe Lyon
2015-08-25 15:44 ` Mike Stump
2015-08-25 20:28 ` Jeff Law
0 siblings, 2 replies; 18+ messages in thread
From: Christophe Lyon @ 2015-08-25 8:17 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 994 bytes --]
Hi,
Some subsets of the tests override ALWAYS_CXXFLAGS or
TEST_ALWAYS_FLAGS and perform effective_target support tests using
these modified flags.
In case these flags conflict with the effective_target tests, it means
that subsequent tests will be UNSUPPORTED even though
ALWAYS_CXXFLAGS/TEST_ALWAYS_FLAGS have been reset and no longer
conflict.
In practice, we noticed this when running validation under 'ulimit -v
XXX', which can conflict with ASAN. We observed that sse2 and
stack_protector tests would randomly fail when tested from asan.exp,
making non-asan tests UNSUPPORTED.
This patch adds a new function 'clear_effective_target_cache', which
is called at the end of every .exp file which overrides
ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
I tested it works well for asan.exp on x86_64 but the changes in other
.exp files seem mechanical.
However, I noticed that lib/g++.exp changes ALWAYS_CXXFLAGS, but does
not appear to restore it. In doubt, I didn't change it.
OK?
Christophe.
[-- Attachment #2: et_cache.patch --]
[-- Type: text/x-patch, Size: 5400 bytes --]
2015-08-25 Christophe Lyon <christophe.lyon@linaro.org>
* lib/target-supports.exp (clear_effective_target_cache): New.
(check_cached_effective_target): Update et_prop_list.
* lib/asan-dg.exp (asan_finish): Call clear_effective_target_cache.
* g++.dg/compat/compat.exp: Likewise.
* g++.dg/compat/struct-layout-1.exp: Likewise.
* lib/asan-dg.exp: Likewise.
* lib/atomic-dg.exp: Likewise.
* lib/cilk-plus-dg.exp: Likewise.
* lib/clearcap.exp: Likewise.
* lib/mpx-dg.exp: Likewise.
* lib/target-supports.exp: Likewise.
* lib/tsan-dg.exp: Likewise.
* lib/ubsan-dg.exp: Likewise.
diff --git a/gcc/testsuite/g++.dg/compat/compat.exp b/gcc/testsuite/g++.dg/compat/compat.exp
index 1272289..4c4b25f 100644
--- a/gcc/testsuite/g++.dg/compat/compat.exp
+++ b/gcc/testsuite/g++.dg/compat/compat.exp
@@ -78,6 +78,7 @@ proc compat-use-tst-compiler { } {
set ALWAYS_CXXFLAGS $save_always_cxxflags
set ld_library_path $save_ld_library_path
set_ld_library_path_env_vars
+ clear_effective_target_cache
}
}
diff --git a/gcc/testsuite/g++.dg/compat/struct-layout-1.exp b/gcc/testsuite/g++.dg/compat/struct-layout-1.exp
index 7777d98..097a731 100644
--- a/gcc/testsuite/g++.dg/compat/struct-layout-1.exp
+++ b/gcc/testsuite/g++.dg/compat/struct-layout-1.exp
@@ -61,6 +61,7 @@ proc compat-use-alt-compiler { } {
set ld_library_path $alt_ld_library_path
set_ld_library_path_env_vars
restore_gcc_exec_prefix_env_var
+ clear_effective_target_cache
}
}
diff --git a/gcc/testsuite/lib/asan-dg.exp b/gcc/testsuite/lib/asan-dg.exp
index 141a479..3ce264e 100644
--- a/gcc/testsuite/lib/asan-dg.exp
+++ b/gcc/testsuite/lib/asan-dg.exp
@@ -138,6 +138,7 @@ proc asan_finish { args } {
}
set ld_library_path $asan_saved_library_path
set_ld_library_path_env_vars
+ clear_effective_target_cache
}
# Symbolize lines like
diff --git a/gcc/testsuite/lib/atomic-dg.exp b/gcc/testsuite/lib/atomic-dg.exp
index d9df227..fe24127 100644
--- a/gcc/testsuite/lib/atomic-dg.exp
+++ b/gcc/testsuite/lib/atomic-dg.exp
@@ -101,4 +101,5 @@ proc atomic_finish { args } {
} else {
unset TEST_ALWAYS_FLAGS
}
+ clear_effective_target_cache
}
diff --git a/gcc/testsuite/lib/cilk-plus-dg.exp b/gcc/testsuite/lib/cilk-plus-dg.exp
index 38e5400..7f38f37 100644
--- a/gcc/testsuite/lib/cilk-plus-dg.exp
+++ b/gcc/testsuite/lib/cilk-plus-dg.exp
@@ -101,4 +101,5 @@ proc cilkplus_finish { args } {
} else {
unset TEST_ALWAYS_FLAGS
}
+ clear_effective_target_cache
}
diff --git a/gcc/testsuite/lib/clearcap.exp b/gcc/testsuite/lib/clearcap.exp
index d41aa1e..3e2a88c 100644
--- a/gcc/testsuite/lib/clearcap.exp
+++ b/gcc/testsuite/lib/clearcap.exp
@@ -55,4 +55,5 @@ proc clearcap-finish { args } {
} else {
unset TEST_ALWAYS_FLAGS
}
+ clear_effective_target_cache
}
diff --git a/gcc/testsuite/lib/mpx-dg.exp b/gcc/testsuite/lib/mpx-dg.exp
index c8f64cd..b2bd40c 100644
--- a/gcc/testsuite/lib/mpx-dg.exp
+++ b/gcc/testsuite/lib/mpx-dg.exp
@@ -142,4 +142,5 @@ proc mpx_finish { args } {
}
set ld_library_path $mpx_saved_library_path
set_ld_library_path_env_vars
+ clear_effective_target_cache
}
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 1988301..e2084bb 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -117,6 +117,7 @@ proc current_target_name { } {
proc check_cached_effective_target { prop args } {
global et_cache
+ global et_prop_list
set target [current_target_name]
if {![info exists et_cache($prop,target)]
@@ -124,12 +125,30 @@ proc check_cached_effective_target { prop args } {
verbose "check_cached_effective_target $prop: checking $target" 2
set et_cache($prop,target) $target
set et_cache($prop,value) [uplevel eval $args]
+ lappend et_prop_list $prop
+ verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
}
set value $et_cache($prop,value)
verbose "check_cached_effective_target $prop: returning $value for $target" 2
return $value
}
+# Clear effective-target cache. This is useful after testing
+# effective-target features and overriding TEST_ALWAYS_FLAGS and/or
+# ALWAYS_CXXFLAGS.
+
+proc clear_effective_target_cache { } {
+ global et_cache
+ global et_prop_list
+
+ verbose "clear_effective_target_cache: $et_prop_list" 2
+ foreach prop $et_prop_list {
+ unset et_cache($prop,value)
+ unset et_cache($prop,target)
+ }
+ unset et_prop_list
+}
+
# Like check_compile, but delete the output file and return true if the
# compiler printed no messages.
proc check_no_compiler_messages_nocache {args} {
diff --git a/gcc/testsuite/lib/tsan-dg.exp b/gcc/testsuite/lib/tsan-dg.exp
index eb528f8..ff51fdf 100644
--- a/gcc/testsuite/lib/tsan-dg.exp
+++ b/gcc/testsuite/lib/tsan-dg.exp
@@ -149,4 +149,5 @@ proc tsan_finish { args } {
}
set ld_library_path $tsan_saved_library_path
set_ld_library_path_env_vars
+ clear_effective_target_cache
}
diff --git a/gcc/testsuite/lib/ubsan-dg.exp b/gcc/testsuite/lib/ubsan-dg.exp
index 81934bb..65799db 100644
--- a/gcc/testsuite/lib/ubsan-dg.exp
+++ b/gcc/testsuite/lib/ubsan-dg.exp
@@ -121,4 +121,5 @@ proc ubsan_finish { args } {
}
set ld_library_path $ubsan_saved_library_path
set_ld_library_path_env_vars
+ clear_effective_target_cache
}
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-08-25 8:17 [testsuite] Clean up effective_target cache Christophe Lyon
@ 2015-08-25 15:44 ` Mike Stump
2015-09-01 14:12 ` Christophe Lyon
2015-08-25 20:28 ` Jeff Law
1 sibling, 1 reply; 18+ messages in thread
From: Mike Stump @ 2015-08-25 15:44 UTC (permalink / raw)
To: Christophe Lyon; +Cc: gcc-patches
On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> Some subsets of the tests override ALWAYS_CXXFLAGS or
> TEST_ALWAYS_FLAGS and perform effective_target support tests using
> these modified flags.
> This patch adds a new function 'clear_effective_target_cache', which
> is called at the end of every .exp file which overrides
> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
Anyway, all looks good. Ok.
> However, I noticed that lib/g++.exp changes ALWAYS_CXXFLAGS, but does
> not appear to restore it. In doubt, I didn't change it.
Yeah, I examined it. It seems like it might not matter, as anyone setting and unsetting would come in cleared, and if they didn’t, it should be roughly the same exact state, meaning, no clearing necessary. I think it is safe to punt this until someone finds a bug or can see a way that it would matter. I also don’t think it would hurt to clear, if someone wanted to refactor the code a bit and make the clearing and the cleanup a little more automatic. I’m thinking of a RAII style code in which the dtor runs the clear. Not sure if that is even possible in tcl. [ checking ] Nope, maybe not. Oh well.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-08-25 8:17 [testsuite] Clean up effective_target cache Christophe Lyon
2015-08-25 15:44 ` Mike Stump
@ 2015-08-25 20:28 ` Jeff Law
1 sibling, 0 replies; 18+ messages in thread
From: Jeff Law @ 2015-08-25 20:28 UTC (permalink / raw)
To: Christophe Lyon, gcc-patches
On 08/25/2015 02:14 AM, Christophe Lyon wrote:
> Hi,
>
> Some subsets of the tests override ALWAYS_CXXFLAGS or
> TEST_ALWAYS_FLAGS and perform effective_target support tests using
> these modified flags.
>
> In case these flags conflict with the effective_target tests, it means
> that subsequent tests will be UNSUPPORTED even though
> ALWAYS_CXXFLAGS/TEST_ALWAYS_FLAGS have been reset and no longer
> conflict.
>
> In practice, we noticed this when running validation under 'ulimit -v
> XXX', which can conflict with ASAN. We observed that sse2 and
> stack_protector tests would randomly fail when tested from asan.exp,
> making non-asan tests UNSUPPORTED.
>
> This patch adds a new function 'clear_effective_target_cache', which
> is called at the end of every .exp file which overrides
> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>
> I tested it works well for asan.exp on x86_64 but the changes in other
> .exp files seem mechanical.
>
> However, I noticed that lib/g++.exp changes ALWAYS_CXXFLAGS, but does
> not appear to restore it. In doubt, I didn't change it.
>
> OK?
OK after a full regression test. While I agree the change is
mechanical, there may be interactions that are non-obvious.
Jeff
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-08-25 15:44 ` Mike Stump
@ 2015-09-01 14:12 ` Christophe Lyon
2015-09-02 14:02 ` Christophe Lyon
0 siblings, 1 reply; 18+ messages in thread
From: Christophe Lyon @ 2015-09-01 14:12 UTC (permalink / raw)
To: Mike Stump; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2117 bytes --]
On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>> these modified flags.
>
>> This patch adds a new function 'clear_effective_target_cache', which
>> is called at the end of every .exp file which overrides
>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>
> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>
> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>
> Anyway, all looks good. Ok.
>
Here is what I have committed (r227372).
I updated the comment before clear_effective_target_cache, and copied
the directive you suggested above.
I also added a test to check if $et_prop_list exists before clearing
(there were error messages otherwise).
Christophe.
>> However, I noticed that lib/g++.exp changes ALWAYS_CXXFLAGS, but does
>> not appear to restore it. In doubt, I didn't change it.
>
> Yeah, I examined it. It seems like it might not matter, as anyone setting and unsetting would come in cleared, and if they didn’t, it should be roughly the same exact state, meaning, no clearing necessary. I think it is safe to punt this until someone finds a bug or can see a way that it would matter. I also don’t think it would hurt to clear, if someone wanted to refactor the code a bit and make the clearing and the cleanup a little more automatic. I’m thinking of a RAII style code in which the dtor runs the clear. Not sure if that is even possible in tcl. [ checking ] Nope, maybe not. Oh well.
[-- Attachment #2: et_cache.patch --]
[-- Type: text/x-patch, Size: 5946 bytes --]
2015-09-01 Christophe Lyon <christophe.lyon@linaro.org>
* lib/target-supports.exp (clear_effective_target_cache): New.
(check_cached_effective_target): Update et_prop_list.
* lib/asan-dg.exp (asan_finish): Call clear_effective_target_cache.
* g++.dg/compat/compat.exp: Likewise.
* g++.dg/compat/struct-layout-1.exp: Likewise.
* lib/asan-dg.exp: Likewise.
* lib/atomic-dg.exp: Likewise.
* lib/cilk-plus-dg.exp: Likewise.
* lib/clearcap.exp: Likewise.
* lib/mpx-dg.exp: Likewise.
* lib/tsan-dg.exp: Likewise.
* lib/ubsan-dg.exp: Likewise.
Index: gcc/testsuite/g++.dg/compat/compat.exp
===================================================================
--- gcc/testsuite/g++.dg/compat/compat.exp (revision 227370)
+++ gcc/testsuite/g++.dg/compat/compat.exp (working copy)
@@ -78,6 +78,7 @@ proc compat-use-tst-compiler { } {
set ALWAYS_CXXFLAGS $save_always_cxxflags
set ld_library_path $save_ld_library_path
set_ld_library_path_env_vars
+ clear_effective_target_cache
}
}
Index: gcc/testsuite/g++.dg/compat/struct-layout-1.exp
===================================================================
--- gcc/testsuite/g++.dg/compat/struct-layout-1.exp (revision 227370)
+++ gcc/testsuite/g++.dg/compat/struct-layout-1.exp (working copy)
@@ -61,6 +61,7 @@ proc compat-use-alt-compiler { } {
set ld_library_path $alt_ld_library_path
set_ld_library_path_env_vars
restore_gcc_exec_prefix_env_var
+ clear_effective_target_cache
}
}
Index: gcc/testsuite/lib/asan-dg.exp
===================================================================
--- gcc/testsuite/lib/asan-dg.exp (revision 227370)
+++ gcc/testsuite/lib/asan-dg.exp (working copy)
@@ -138,6 +138,7 @@ proc asan_finish { args } {
}
set ld_library_path $asan_saved_library_path
set_ld_library_path_env_vars
+ clear_effective_target_cache
}
# Symbolize lines like
Index: gcc/testsuite/lib/atomic-dg.exp
===================================================================
--- gcc/testsuite/lib/atomic-dg.exp (revision 227370)
+++ gcc/testsuite/lib/atomic-dg.exp (working copy)
@@ -101,4 +101,5 @@ proc atomic_finish { args } {
} else {
unset TEST_ALWAYS_FLAGS
}
+ clear_effective_target_cache
}
Index: gcc/testsuite/lib/cilk-plus-dg.exp
===================================================================
--- gcc/testsuite/lib/cilk-plus-dg.exp (revision 227370)
+++ gcc/testsuite/lib/cilk-plus-dg.exp (working copy)
@@ -101,4 +101,5 @@ proc cilkplus_finish { args } {
} else {
unset TEST_ALWAYS_FLAGS
}
+ clear_effective_target_cache
}
Index: gcc/testsuite/lib/clearcap.exp
===================================================================
--- gcc/testsuite/lib/clearcap.exp (revision 227370)
+++ gcc/testsuite/lib/clearcap.exp (working copy)
@@ -55,4 +55,5 @@ proc clearcap-finish { args } {
} else {
unset TEST_ALWAYS_FLAGS
}
+ clear_effective_target_cache
}
Index: gcc/testsuite/lib/mpx-dg.exp
===================================================================
--- gcc/testsuite/lib/mpx-dg.exp (revision 227370)
+++ gcc/testsuite/lib/mpx-dg.exp (working copy)
@@ -142,4 +142,5 @@ proc mpx_finish { args } {
}
set ld_library_path $mpx_saved_library_path
set_ld_library_path_env_vars
+ clear_effective_target_cache
}
Index: gcc/testsuite/lib/target-supports.exp
===================================================================
--- gcc/testsuite/lib/target-supports.exp (revision 227370)
+++ gcc/testsuite/lib/target-supports.exp (working copy)
@@ -117,6 +117,7 @@ proc current_target_name { } {
proc check_cached_effective_target { prop args } {
global et_cache
+ global et_prop_list
set target [current_target_name]
if {![info exists et_cache($prop,target)]
@@ -124,12 +125,37 @@ proc check_cached_effective_target { pro
verbose "check_cached_effective_target $prop: checking $target" 2
set et_cache($prop,target) $target
set et_cache($prop,value) [uplevel eval $args]
+ lappend et_prop_list $prop
+ verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
}
set value $et_cache($prop,value)
verbose "check_cached_effective_target $prop: returning $value for $target" 2
return $value
}
+# Clear effective-target cache. This is useful after testing
+# effective-target features and overriding TEST_ALWAYS_FLAGS and/or
+# ALWAYS_CXXFLAGS.
+# If one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should
+# do a clear_effective_target_cache at the end as the target cache can
+# make decisions based upon the flags, and those decisions need to be
+# redone when the flags change. An example of this is the
+# asan_init/asan_finish pair.
+
+proc clear_effective_target_cache { } {
+ global et_cache
+ global et_prop_list
+
+ if {[info exists et_prop_list]} {
+ verbose "clear_effective_target_cache: $et_prop_list" 2
+ foreach prop $et_prop_list {
+ unset et_cache($prop,value)
+ unset et_cache($prop,target)
+ }
+ unset et_prop_list
+ }
+}
+
# Like check_compile, but delete the output file and return true if the
# compiler printed no messages.
proc check_no_compiler_messages_nocache {args} {
Index: gcc/testsuite/lib/tsan-dg.exp
===================================================================
--- gcc/testsuite/lib/tsan-dg.exp (revision 227370)
+++ gcc/testsuite/lib/tsan-dg.exp (working copy)
@@ -149,4 +149,5 @@ proc tsan_finish { args } {
}
set ld_library_path $tsan_saved_library_path
set_ld_library_path_env_vars
+ clear_effective_target_cache
}
Index: gcc/testsuite/lib/ubsan-dg.exp
===================================================================
--- gcc/testsuite/lib/ubsan-dg.exp (revision 227370)
+++ gcc/testsuite/lib/ubsan-dg.exp (working copy)
@@ -121,4 +121,5 @@ proc ubsan_finish { args } {
}
set ld_library_path $ubsan_saved_library_path
set_ld_library_path_env_vars
+ clear_effective_target_cache
}
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-01 14:12 ` Christophe Lyon
@ 2015-09-02 14:02 ` Christophe Lyon
2015-09-03 11:36 ` H.J. Lu
0 siblings, 1 reply; 18+ messages in thread
From: Christophe Lyon @ 2015-09-02 14:02 UTC (permalink / raw)
To: Mike Stump; +Cc: gcc-patches
On 1 September 2015 at 16:04, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>> these modified flags.
>>
>>> This patch adds a new function 'clear_effective_target_cache', which
>>> is called at the end of every .exp file which overrides
>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>
>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>
>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>
>> Anyway, all looks good. Ok.
>>
> Here is what I have committed (r227372).
Hmmm, in fact this was r227401.
>
> I updated the comment before clear_effective_target_cache, and copied
> the directive you suggested above.
> I also added a test to check if $et_prop_list exists before clearing
> (there were error messages otherwise).
>
> Christophe.
>
>>> However, I noticed that lib/g++.exp changes ALWAYS_CXXFLAGS, but does
>>> not appear to restore it. In doubt, I didn't change it.
>>
>> Yeah, I examined it. It seems like it might not matter, as anyone setting and unsetting would come in cleared, and if they didn’t, it should be roughly the same exact state, meaning, no clearing necessary. I think it is safe to punt this until someone finds a bug or can see a way that it would matter. I also don’t think it would hurt to clear, if someone wanted to refactor the code a bit and make the clearing and the cleanup a little more automatic. I’m thinking of a RAII style code in which the dtor runs the clear. Not sure if that is even possible in tcl. [ checking ] Nope, maybe not. Oh well.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-02 14:02 ` Christophe Lyon
@ 2015-09-03 11:36 ` H.J. Lu
2015-09-03 15:10 ` Christophe Lyon
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2015-09-03 11:36 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Mike Stump, gcc-patches
On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 1 September 2015 at 16:04, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>> these modified flags.
>>>
>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>> is called at the end of every .exp file which overrides
>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>
>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>
>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>
>>> Anyway, all looks good. Ok.
>>>
>> Here is what I have committed (r227372).
>
> Hmmm, in fact this was r227401.
>
It caused:
ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
ERROR: can't unset "et_cache(dfp,value)": no such element in array
ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
ERROR: can't unset "et_cache(ia32,value)": no such element in array
ERROR: can't unset "et_cache(ia32,value)": no such element in array
ERROR: can't unset "et_cache(ia32,value)": no such element in array
ERROR: can't unset "et_cache(ia32,value)": no such element in array
ERROR: can't unset "et_cache(ia32,value)": no such element in array
ERROR: can't unset "et_cache(ilp32,value)": no such element in array
ERROR: can't unset "et_cache(ilp32,value)": no such element in array
ERROR: can't unset "et_cache(ilp32,value)": no such element in array
ERROR: can't unset "et_cache(ilp32,value)": no such element in array
ERROR: can't unset "et_cache(label_values,value)": no such element in array
ERROR: can't unset "et_cache(lp64,value)": no such element in array
ERROR: can't unset "et_cache(lp64,value)": no such element in array
ERROR: can't unset "et_cache(lp64,value)": no such element in array
ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
...
on Linux/x86-64:
https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-03 11:36 ` H.J. Lu
@ 2015-09-03 15:10 ` Christophe Lyon
2015-09-04 11:19 ` H.J. Lu
0 siblings, 1 reply; 18+ messages in thread
From: Christophe Lyon @ 2015-09-03 15:10 UTC (permalink / raw)
To: H.J. Lu; +Cc: Mike Stump, gcc-patches
On 3 September 2015 at 13:31, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
>> On 1 September 2015 at 16:04, Christophe Lyon
>> <christophe.lyon@linaro.org> wrote:
>>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>>> these modified flags.
>>>>
>>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>>> is called at the end of every .exp file which overrides
>>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>>
>>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>>
>>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>>
>>>> Anyway, all looks good. Ok.
>>>>
>>> Here is what I have committed (r227372).
>>
>> Hmmm, in fact this was r227401.
>>
>
> It caused:
>
> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
> ERROR: can't unset "et_cache(dfp,value)": no such element in array
> ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
> ERROR: can't unset "et_cache(ia32,value)": no such element in array
> ERROR: can't unset "et_cache(ia32,value)": no such element in array
> ERROR: can't unset "et_cache(ia32,value)": no such element in array
> ERROR: can't unset "et_cache(ia32,value)": no such element in array
> ERROR: can't unset "et_cache(ia32,value)": no such element in array
> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
> ERROR: can't unset "et_cache(label_values,value)": no such element in array
> ERROR: can't unset "et_cache(lp64,value)": no such element in array
> ERROR: can't unset "et_cache(lp64,value)": no such element in array
> ERROR: can't unset "et_cache(lp64,value)": no such element in array
> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
> ...
>
> on Linux/x86-64:
>
> https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
>
I'll have a look.
That's the configuration I used to check before committing, but I am
going to re-check.
> --
> H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-03 15:10 ` Christophe Lyon
@ 2015-09-04 11:19 ` H.J. Lu
2015-09-04 11:28 ` H.J. Lu
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2015-09-04 11:19 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Mike Stump, gcc-patches
On Thu, Sep 3, 2015 at 8:03 AM, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 3 September 2015 at 13:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
>> <christophe.lyon@linaro.org> wrote:
>>> On 1 September 2015 at 16:04, Christophe Lyon
>>> <christophe.lyon@linaro.org> wrote:
>>>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>>>> these modified flags.
>>>>>
>>>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>>>> is called at the end of every .exp file which overrides
>>>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>>>
>>>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>>>
>>>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>>>
>>>>> Anyway, all looks good. Ok.
>>>>>
>>>> Here is what I have committed (r227372).
>>>
>>> Hmmm, in fact this was r227401.
>>>
>>
>> It caused:
>>
>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>> ERROR: can't unset "et_cache(dfp,value)": no such element in array
>> ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>> ERROR: can't unset "et_cache(label_values,value)": no such element in array
>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>> ...
>>
>> on Linux/x86-64:
>>
>> https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
>>
>
> I'll have a look.
> That's the configuration I used to check before committing, but I am
> going to re-check.
proc check_cached_effective_target { prop args } {
global et_cache
global et_prop_list
set target [current_target_name]
if {![info exists et_cache($prop,target)]
|| $et_cache($prop,target) != $target} {
verbose "check_cached_effective_target $prop: checking $target" 2
set et_cache($prop,target) $target
set et_cache($prop,value) [uplevel eval $args]
lappend et_prop_list $prop
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Aren't you appending $pop to et_prop_list even if it may be already
on the list?
verbose "check_cached_effective_target cached list is now:
$et_prop_list" 2
}
set value $et_cache($prop,value)
verbose "check_cached_effective_target $prop: returning $value for
$target" 2
return $value
}
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-04 11:19 ` H.J. Lu
@ 2015-09-04 11:28 ` H.J. Lu
2015-09-04 12:13 ` H.J. Lu
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2015-09-04 11:28 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Mike Stump, gcc-patches
On Fri, Sep 4, 2015 at 4:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Sep 3, 2015 at 8:03 AM, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
>> On 3 September 2015 at 13:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
>>> <christophe.lyon@linaro.org> wrote:
>>>> On 1 September 2015 at 16:04, Christophe Lyon
>>>> <christophe.lyon@linaro.org> wrote:
>>>>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>>>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>>>>> these modified flags.
>>>>>>
>>>>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>>>>> is called at the end of every .exp file which overrides
>>>>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>>>>
>>>>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>>>>
>>>>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>>>>
>>>>>> Anyway, all looks good. Ok.
>>>>>>
>>>>> Here is what I have committed (r227372).
>>>>
>>>> Hmmm, in fact this was r227401.
>>>>
>>>
>>> It caused:
>>>
>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>> ERROR: can't unset "et_cache(dfp,value)": no such element in array
>>> ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>> ERROR: can't unset "et_cache(label_values,value)": no such element in array
>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>> ...
>>>
>>> on Linux/x86-64:
>>>
>>> https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
>>>
>>
>> I'll have a look.
>> That's the configuration I used to check before committing, but I am
>> going to re-check.
>
> proc check_cached_effective_target { prop args } {
> global et_cache
> global et_prop_list
>
> set target [current_target_name]
> if {![info exists et_cache($prop,target)]
> || $et_cache($prop,target) != $target} {
> verbose "check_cached_effective_target $prop: checking $target" 2
> set et_cache($prop,target) $target
> set et_cache($prop,value) [uplevel eval $args]
> lappend et_prop_list $prop
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> Aren't you appending $pop to et_prop_list even if it may be already
> on the list?
>
> verbose "check_cached_effective_target cached list is now:
> $et_prop_list" 2
> }
> set value $et_cache($prop,value)
> verbose "check_cached_effective_target $prop: returning $value for
> $target" 2
> return $value
> }
>
Like this?
--
H.J.
---
diff --git a/gcc/testsuite/lib/target-supports.exp
b/gcc/testsuite/lib/target-supports.exp
index aad45f9..a6c16fe 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -125,7 +125,9 @@ proc check_cached_effective_target { prop args } {
verbose "check_cached_effective_target $prop: checking $target" 2
set et_cache($prop,target) $target
set et_cache($prop,value) [uplevel eval $args]
- lappend et_prop_list $prop
+ if {[lsearch $et_prop_list $prop] < 0} {
+ lappend et_prop_list $prop
+ }
verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
}
set value $et_cache($prop,value)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-04 11:28 ` H.J. Lu
@ 2015-09-04 12:13 ` H.J. Lu
2015-09-04 12:35 ` H.J. Lu
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2015-09-04 12:13 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Mike Stump, gcc-patches
On Fri, Sep 4, 2015 at 4:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Sep 4, 2015 at 4:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, Sep 3, 2015 at 8:03 AM, Christophe Lyon
>> <christophe.lyon@linaro.org> wrote:
>>> On 3 September 2015 at 13:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
>>>> <christophe.lyon@linaro.org> wrote:
>>>>> On 1 September 2015 at 16:04, Christophe Lyon
>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>>>>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>>>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>>>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>>>>>> these modified flags.
>>>>>>>
>>>>>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>>>>>> is called at the end of every .exp file which overrides
>>>>>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>>>>>
>>>>>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>>>>>
>>>>>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>>>>>
>>>>>>> Anyway, all looks good. Ok.
>>>>>>>
>>>>>> Here is what I have committed (r227372).
>>>>>
>>>>> Hmmm, in fact this was r227401.
>>>>>
>>>>
>>>> It caused:
>>>>
>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>> ERROR: can't unset "et_cache(dfp,value)": no such element in array
>>>> ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>> ERROR: can't unset "et_cache(label_values,value)": no such element in array
>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>> ...
>>>>
>>>> on Linux/x86-64:
>>>>
>>>> https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
>>>>
>>>
>>> I'll have a look.
>>> That's the configuration I used to check before committing, but I am
>>> going to re-check.
>>
>> proc check_cached_effective_target { prop args } {
>> global et_cache
>> global et_prop_list
>>
>> set target [current_target_name]
>> if {![info exists et_cache($prop,target)]
>> || $et_cache($prop,target) != $target} {
>> verbose "check_cached_effective_target $prop: checking $target" 2
>> set et_cache($prop,target) $target
>> set et_cache($prop,value) [uplevel eval $args]
>> lappend et_prop_list $prop
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> Aren't you appending $pop to et_prop_list even if it may be already
>> on the list?
>>
>> verbose "check_cached_effective_target cached list is now:
>> $et_prop_list" 2
>> }
>> set value $et_cache($prop,value)
>> verbose "check_cached_effective_target $prop: returning $value for
>> $target" 2
>> return $value
>> }
>>
>
> Like this?
>
> --
> H.J.
> ---
> diff --git a/gcc/testsuite/lib/target-supports.exp
> b/gcc/testsuite/lib/target-supports.exp
> index aad45f9..a6c16fe 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -125,7 +125,9 @@ proc check_cached_effective_target { prop args } {
> verbose "check_cached_effective_target $prop: checking $target" 2
> set et_cache($prop,target) $target
> set et_cache($prop,value) [uplevel eval $args]
> - lappend et_prop_list $prop
> + if {[lsearch $et_prop_list $prop] < 0} {
> + lappend et_prop_list $prop
> + }
> verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
> }
> set value $et_cache($prop,value)
It should be
if {![info exists et_prop_list]
|| [lsearch $et_prop_list $prop] < 0} {
lappend et_prop_list $prop
}
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-04 12:13 ` H.J. Lu
@ 2015-09-04 12:35 ` H.J. Lu
2015-09-04 13:21 ` Christophe Lyon
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2015-09-04 12:35 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Mike Stump, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 5651 bytes --]
On Fri, Sep 4, 2015 at 4:47 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Sep 4, 2015 at 4:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Sep 4, 2015 at 4:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Thu, Sep 3, 2015 at 8:03 AM, Christophe Lyon
>>> <christophe.lyon@linaro.org> wrote:
>>>> On 3 September 2015 at 13:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>> On 1 September 2015 at 16:04, Christophe Lyon
>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>>>>>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>>>>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>>>>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>>>>>>> these modified flags.
>>>>>>>>
>>>>>>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>>>>>>> is called at the end of every .exp file which overrides
>>>>>>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>>>>>>
>>>>>>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>>>>>>
>>>>>>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>>>>>>
>>>>>>>> Anyway, all looks good. Ok.
>>>>>>>>
>>>>>>> Here is what I have committed (r227372).
>>>>>>
>>>>>> Hmmm, in fact this was r227401.
>>>>>>
>>>>>
>>>>> It caused:
>>>>>
>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(dfp,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(label_values,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>> ...
>>>>>
>>>>> on Linux/x86-64:
>>>>>
>>>>> https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
>>>>>
>>>>
>>>> I'll have a look.
>>>> That's the configuration I used to check before committing, but I am
>>>> going to re-check.
>>>
>>> proc check_cached_effective_target { prop args } {
>>> global et_cache
>>> global et_prop_list
>>>
>>> set target [current_target_name]
>>> if {![info exists et_cache($prop,target)]
>>> || $et_cache($prop,target) != $target} {
>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>> set et_cache($prop,target) $target
>>> set et_cache($prop,value) [uplevel eval $args]
>>> lappend et_prop_list $prop
>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>
>>> Aren't you appending $pop to et_prop_list even if it may be already
>>> on the list?
>>>
>>> verbose "check_cached_effective_target cached list is now:
>>> $et_prop_list" 2
>>> }
>>> set value $et_cache($prop,value)
>>> verbose "check_cached_effective_target $prop: returning $value for
>>> $target" 2
>>> return $value
>>> }
>>>
>>
>> Like this?
>>
>> --
>> H.J.
>> ---
>> diff --git a/gcc/testsuite/lib/target-supports.exp
>> b/gcc/testsuite/lib/target-supports.exp
>> index aad45f9..a6c16fe 100644
>> --- a/gcc/testsuite/lib/target-supports.exp
>> +++ b/gcc/testsuite/lib/target-supports.exp
>> @@ -125,7 +125,9 @@ proc check_cached_effective_target { prop args } {
>> verbose "check_cached_effective_target $prop: checking $target" 2
>> set et_cache($prop,target) $target
>> set et_cache($prop,value) [uplevel eval $args]
>> - lappend et_prop_list $prop
>> + if {[lsearch $et_prop_list $prop] < 0} {
>> + lappend et_prop_list $prop
>> + }
>> verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
>> }
>> set value $et_cache($prop,value)
>
>
> It should be
>
> if {![info exists et_prop_list]
> || [lsearch $et_prop_list $prop] < 0} {
> lappend et_prop_list $prop
> }
>
Here is a patch. OK for trunk?
--
H.J.
[-- Attachment #2: pr67450.patch --]
[-- Type: text/x-patch, Size: 833 bytes --]
PR testsuite/67450
* lib/target-supports.exp (check_cached_effective_target): Apppend
$prop to et_prop_list only if needed.
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index aad45f9..5e17b26 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -125,7 +125,10 @@ proc check_cached_effective_target { prop args } {
verbose "check_cached_effective_target $prop: checking $target" 2
set et_cache($prop,target) $target
set et_cache($prop,value) [uplevel eval $args]
- lappend et_prop_list $prop
+ if {![info exists et_prop_list]
+ || [lsearch $et_prop_list $prop] < 0} {
+ lappend et_prop_list $prop
+ }
verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
}
set value $et_cache($prop,value)
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-04 12:35 ` H.J. Lu
@ 2015-09-04 13:21 ` Christophe Lyon
2015-09-04 14:21 ` H.J. Lu
0 siblings, 1 reply; 18+ messages in thread
From: Christophe Lyon @ 2015-09-04 13:21 UTC (permalink / raw)
To: H.J. Lu; +Cc: Mike Stump, gcc-patches
On 4 September 2015 at 14:13, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Sep 4, 2015 at 4:47 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Sep 4, 2015 at 4:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Fri, Sep 4, 2015 at 4:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Thu, Sep 3, 2015 at 8:03 AM, Christophe Lyon
>>>> <christophe.lyon@linaro.org> wrote:
>>>>> On 3 September 2015 at 13:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>> On 1 September 2015 at 16:04, Christophe Lyon
>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>>>>>>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>>>>>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>>>>>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>>>>>>>> these modified flags.
>>>>>>>>>
>>>>>>>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>>>>>>>> is called at the end of every .exp file which overrides
>>>>>>>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>>>>>>>
>>>>>>>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>>>>>>>
>>>>>>>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>>>>>>>
>>>>>>>>> Anyway, all looks good. Ok.
>>>>>>>>>
>>>>>>>> Here is what I have committed (r227372).
>>>>>>>
>>>>>>> Hmmm, in fact this was r227401.
>>>>>>>
>>>>>>
>>>>>> It caused:
>>>>>>
>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(dfp,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(label_values,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>> ...
>>>>>>
>>>>>> on Linux/x86-64:
>>>>>>
>>>>>> https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
>>>>>>
>>>>>
>>>>> I'll have a look.
>>>>> That's the configuration I used to check before committing, but I am
>>>>> going to re-check.
>>>>
>>>> proc check_cached_effective_target { prop args } {
>>>> global et_cache
>>>> global et_prop_list
>>>>
>>>> set target [current_target_name]
>>>> if {![info exists et_cache($prop,target)]
>>>> || $et_cache($prop,target) != $target} {
>>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>>> set et_cache($prop,target) $target
>>>> set et_cache($prop,value) [uplevel eval $args]
>>>> lappend et_prop_list $prop
>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>
>>>> Aren't you appending $pop to et_prop_list even if it may be already
>>>> on the list?
>>>>
>>>> verbose "check_cached_effective_target cached list is now:
>>>> $et_prop_list" 2
>>>> }
>>>> set value $et_cache($prop,value)
>>>> verbose "check_cached_effective_target $prop: returning $value for
>>>> $target" 2
>>>> return $value
>>>> }
>>>>
>>>
>>> Like this?
>>>
>>> --
>>> H.J.
>>> ---
>>> diff --git a/gcc/testsuite/lib/target-supports.exp
>>> b/gcc/testsuite/lib/target-supports.exp
>>> index aad45f9..a6c16fe 100644
>>> --- a/gcc/testsuite/lib/target-supports.exp
>>> +++ b/gcc/testsuite/lib/target-supports.exp
>>> @@ -125,7 +125,9 @@ proc check_cached_effective_target { prop args } {
>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>> set et_cache($prop,target) $target
>>> set et_cache($prop,value) [uplevel eval $args]
>>> - lappend et_prop_list $prop
>>> + if {[lsearch $et_prop_list $prop] < 0} {
>>> + lappend et_prop_list $prop
>>> + }
>>> verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
>>> }
>>> set value $et_cache($prop,value)
>>
>>
>> It should be
>>
>> if {![info exists et_prop_list]
>> || [lsearch $et_prop_list $prop] < 0} {
>> lappend et_prop_list $prop
>> }
>>
>
> Here is a patch. OK for trunk?
>
It makes sense, indeed, although I still haven't managed to reproduce
the issue you reported.
>
> --
> H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-04 13:21 ` Christophe Lyon
@ 2015-09-04 14:21 ` H.J. Lu
2015-09-04 14:54 ` Christophe Lyon
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2015-09-04 14:21 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Mike Stump, gcc-patches
On Fri, Sep 4, 2015 at 6:15 AM, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 4 September 2015 at 14:13, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Sep 4, 2015 at 4:47 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Fri, Sep 4, 2015 at 4:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Fri, Sep 4, 2015 at 4:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Thu, Sep 3, 2015 at 8:03 AM, Christophe Lyon
>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>> On 3 September 2015 at 13:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>> On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>> On 1 September 2015 at 16:04, Christophe Lyon
>>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>>>>>>>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>>>>>>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>>>>>>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>>>>>>>>> these modified flags.
>>>>>>>>>>
>>>>>>>>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>>>>>>>>> is called at the end of every .exp file which overrides
>>>>>>>>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>>>>>>>>
>>>>>>>>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>>>>>>>>
>>>>>>>>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>>>>>>>>
>>>>>>>>>> Anyway, all looks good. Ok.
>>>>>>>>>>
>>>>>>>>> Here is what I have committed (r227372).
>>>>>>>>
>>>>>>>> Hmmm, in fact this was r227401.
>>>>>>>>
>>>>>>>
>>>>>>> It caused:
>>>>>>>
>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(dfp,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(label_values,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>>> ...
>>>>>>>
>>>>>>> on Linux/x86-64:
>>>>>>>
>>>>>>> https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
>>>>>>>
>>>>>>
>>>>>> I'll have a look.
>>>>>> That's the configuration I used to check before committing, but I am
>>>>>> going to re-check.
>>>>>
>>>>> proc check_cached_effective_target { prop args } {
>>>>> global et_cache
>>>>> global et_prop_list
>>>>>
>>>>> set target [current_target_name]
>>>>> if {![info exists et_cache($prop,target)]
>>>>> || $et_cache($prop,target) != $target} {
>>>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>>>> set et_cache($prop,target) $target
>>>>> set et_cache($prop,value) [uplevel eval $args]
>>>>> lappend et_prop_list $prop
>>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>
>>>>> Aren't you appending $pop to et_prop_list even if it may be already
>>>>> on the list?
>>>>>
>>>>> verbose "check_cached_effective_target cached list is now:
>>>>> $et_prop_list" 2
>>>>> }
>>>>> set value $et_cache($prop,value)
>>>>> verbose "check_cached_effective_target $prop: returning $value for
>>>>> $target" 2
>>>>> return $value
>>>>> }
>>>>>
>>>>
>>>> Like this?
>>>>
>>>> --
>>>> H.J.
>>>> ---
>>>> diff --git a/gcc/testsuite/lib/target-supports.exp
>>>> b/gcc/testsuite/lib/target-supports.exp
>>>> index aad45f9..a6c16fe 100644
>>>> --- a/gcc/testsuite/lib/target-supports.exp
>>>> +++ b/gcc/testsuite/lib/target-supports.exp
>>>> @@ -125,7 +125,9 @@ proc check_cached_effective_target { prop args } {
>>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>>> set et_cache($prop,target) $target
>>>> set et_cache($prop,value) [uplevel eval $args]
>>>> - lappend et_prop_list $prop
>>>> + if {[lsearch $et_prop_list $prop] < 0} {
>>>> + lappend et_prop_list $prop
>>>> + }
>>>> verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
>>>> }
>>>> set value $et_cache($prop,value)
>>>
>>>
>>> It should be
>>>
>>> if {![info exists et_prop_list]
>>> || [lsearch $et_prop_list $prop] < 0} {
>>> lappend et_prop_list $prop
>>> }
>>>
>>
>> Here is a patch. OK for trunk?
>>
>
> It makes sense, indeed, although I still haven't managed to reproduce
> the issue you reported.
The failure is random with parallel check on machines with >= 8 cores.
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-04 14:21 ` H.J. Lu
@ 2015-09-04 14:54 ` Christophe Lyon
2015-09-04 14:59 ` H.J. Lu
0 siblings, 1 reply; 18+ messages in thread
From: Christophe Lyon @ 2015-09-04 14:54 UTC (permalink / raw)
To: H.J. Lu; +Cc: Mike Stump, gcc-patches
On 4 September 2015 at 15:58, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Sep 4, 2015 at 6:15 AM, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
>> On 4 September 2015 at 14:13, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Fri, Sep 4, 2015 at 4:47 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Fri, Sep 4, 2015 at 4:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Fri, Sep 4, 2015 at 4:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> On Thu, Sep 3, 2015 at 8:03 AM, Christophe Lyon
>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>> On 3 September 2015 at 13:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>> On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
>>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>>> On 1 September 2015 at 16:04, Christophe Lyon
>>>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>>>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>>>>>>>>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>>>>>>>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>>>>>>>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>>>>>>>>>> these modified flags.
>>>>>>>>>>>
>>>>>>>>>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>>>>>>>>>> is called at the end of every .exp file which overrides
>>>>>>>>>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>>>>>>>>>
>>>>>>>>>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>>>>>>>>>
>>>>>>>>>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>>>>>>>>>
>>>>>>>>>>> Anyway, all looks good. Ok.
>>>>>>>>>>>
>>>>>>>>>> Here is what I have committed (r227372).
>>>>>>>>>
>>>>>>>>> Hmmm, in fact this was r227401.
>>>>>>>>>
>>>>>>>>
>>>>>>>> It caused:
>>>>>>>>
>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(dfp,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(label_values,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>>>> ...
>>>>>>>>
>>>>>>>> on Linux/x86-64:
>>>>>>>>
>>>>>>>> https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
>>>>>>>>
>>>>>>>
>>>>>>> I'll have a look.
>>>>>>> That's the configuration I used to check before committing, but I am
>>>>>>> going to re-check.
>>>>>>
>>>>>> proc check_cached_effective_target { prop args } {
>>>>>> global et_cache
>>>>>> global et_prop_list
>>>>>>
>>>>>> set target [current_target_name]
>>>>>> if {![info exists et_cache($prop,target)]
>>>>>> || $et_cache($prop,target) != $target} {
>>>>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>>>>> set et_cache($prop,target) $target
>>>>>> set et_cache($prop,value) [uplevel eval $args]
>>>>>> lappend et_prop_list $prop
>>>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>>
>>>>>> Aren't you appending $pop to et_prop_list even if it may be already
>>>>>> on the list?
>>>>>>
>>>>>> verbose "check_cached_effective_target cached list is now:
>>>>>> $et_prop_list" 2
>>>>>> }
>>>>>> set value $et_cache($prop,value)
>>>>>> verbose "check_cached_effective_target $prop: returning $value for
>>>>>> $target" 2
>>>>>> return $value
>>>>>> }
>>>>>>
>>>>>
>>>>> Like this?
>>>>>
>>>>> --
>>>>> H.J.
>>>>> ---
>>>>> diff --git a/gcc/testsuite/lib/target-supports.exp
>>>>> b/gcc/testsuite/lib/target-supports.exp
>>>>> index aad45f9..a6c16fe 100644
>>>>> --- a/gcc/testsuite/lib/target-supports.exp
>>>>> +++ b/gcc/testsuite/lib/target-supports.exp
>>>>> @@ -125,7 +125,9 @@ proc check_cached_effective_target { prop args } {
>>>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>>>> set et_cache($prop,target) $target
>>>>> set et_cache($prop,value) [uplevel eval $args]
>>>>> - lappend et_prop_list $prop
>>>>> + if {[lsearch $et_prop_list $prop] < 0} {
>>>>> + lappend et_prop_list $prop
>>>>> + }
>>>>> verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
>>>>> }
>>>>> set value $et_cache($prop,value)
>>>>
>>>>
>>>> It should be
>>>>
>>>> if {![info exists et_prop_list]
>>>> || [lsearch $et_prop_list $prop] < 0} {
>>>> lappend et_prop_list $prop
>>>> }
>>>>
>>>
>>> Here is a patch. OK for trunk?
>>>
>>
>> It makes sense, indeed, although I still haven't managed to reproduce
>> the issue you reported.
>
> The failure is random with parallel check on machines with >= 8 cores.
>
In fact that's because you are running the testsuite with several
values for 'target' (unix and unix/-m32), which indeed result in
appending $prop twice.
Thanks
>
> --
> H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-04 14:54 ` Christophe Lyon
@ 2015-09-04 14:59 ` H.J. Lu
2015-09-04 15:02 ` Christophe Lyon
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2015-09-04 14:59 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Mike Stump, gcc-patches
On Fri, Sep 4, 2015 at 7:52 AM, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 4 September 2015 at 15:58, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Sep 4, 2015 at 6:15 AM, Christophe Lyon
>> <christophe.lyon@linaro.org> wrote:
>>> On 4 September 2015 at 14:13, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Fri, Sep 4, 2015 at 4:47 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Fri, Sep 4, 2015 at 4:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> On Fri, Sep 4, 2015 at 4:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>> On Thu, Sep 3, 2015 at 8:03 AM, Christophe Lyon
>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>> On 3 September 2015 at 13:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>>> On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
>>>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>>>> On 1 September 2015 at 16:04, Christophe Lyon
>>>>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>>>>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>>>>>>>>>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>>>>>>>>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>>>>>>>>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>>>>>>>>>>> these modified flags.
>>>>>>>>>>>>
>>>>>>>>>>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>>>>>>>>>>> is called at the end of every .exp file which overrides
>>>>>>>>>>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>>>>>>>>>>
>>>>>>>>>>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>>>>>>>>>>
>>>>>>>>>>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>>>>>>>>>>
>>>>>>>>>>>> Anyway, all looks good. Ok.
>>>>>>>>>>>>
>>>>>>>>>>> Here is what I have committed (r227372).
>>>>>>>>>>
>>>>>>>>>> Hmmm, in fact this was r227401.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> It caused:
>>>>>>>>>
>>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(dfp,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(label_values,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>>>>> ...
>>>>>>>>>
>>>>>>>>> on Linux/x86-64:
>>>>>>>>>
>>>>>>>>> https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
>>>>>>>>>
>>>>>>>>
>>>>>>>> I'll have a look.
>>>>>>>> That's the configuration I used to check before committing, but I am
>>>>>>>> going to re-check.
>>>>>>>
>>>>>>> proc check_cached_effective_target { prop args } {
>>>>>>> global et_cache
>>>>>>> global et_prop_list
>>>>>>>
>>>>>>> set target [current_target_name]
>>>>>>> if {![info exists et_cache($prop,target)]
>>>>>>> || $et_cache($prop,target) != $target} {
>>>>>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>>>>>> set et_cache($prop,target) $target
>>>>>>> set et_cache($prop,value) [uplevel eval $args]
>>>>>>> lappend et_prop_list $prop
>>>>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>>>
>>>>>>> Aren't you appending $pop to et_prop_list even if it may be already
>>>>>>> on the list?
>>>>>>>
>>>>>>> verbose "check_cached_effective_target cached list is now:
>>>>>>> $et_prop_list" 2
>>>>>>> }
>>>>>>> set value $et_cache($prop,value)
>>>>>>> verbose "check_cached_effective_target $prop: returning $value for
>>>>>>> $target" 2
>>>>>>> return $value
>>>>>>> }
>>>>>>>
>>>>>>
>>>>>> Like this?
>>>>>>
>>>>>> --
>>>>>> H.J.
>>>>>> ---
>>>>>> diff --git a/gcc/testsuite/lib/target-supports.exp
>>>>>> b/gcc/testsuite/lib/target-supports.exp
>>>>>> index aad45f9..a6c16fe 100644
>>>>>> --- a/gcc/testsuite/lib/target-supports.exp
>>>>>> +++ b/gcc/testsuite/lib/target-supports.exp
>>>>>> @@ -125,7 +125,9 @@ proc check_cached_effective_target { prop args } {
>>>>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>>>>> set et_cache($prop,target) $target
>>>>>> set et_cache($prop,value) [uplevel eval $args]
>>>>>> - lappend et_prop_list $prop
>>>>>> + if {[lsearch $et_prop_list $prop] < 0} {
>>>>>> + lappend et_prop_list $prop
>>>>>> + }
>>>>>> verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
>>>>>> }
>>>>>> set value $et_cache($prop,value)
>>>>>
>>>>>
>>>>> It should be
>>>>>
>>>>> if {![info exists et_prop_list]
>>>>> || [lsearch $et_prop_list $prop] < 0} {
>>>>> lappend et_prop_list $prop
>>>>> }
>>>>>
>>>>
>>>> Here is a patch. OK for trunk?
>>>>
>>>
>>> It makes sense, indeed, although I still haven't managed to reproduce
>>> the issue you reported.
>>
>> The failure is random with parallel check on machines with >= 8 cores.
>>
> In fact that's because you are running the testsuite with several
> values for 'target' (unix and unix/-m32), which indeed result in
> appending $prop twice.
Is my patch correct or you have a different fix?
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-04 14:59 ` H.J. Lu
@ 2015-09-04 15:02 ` Christophe Lyon
2015-09-04 15:16 ` H.J. Lu
0 siblings, 1 reply; 18+ messages in thread
From: Christophe Lyon @ 2015-09-04 15:02 UTC (permalink / raw)
To: H.J. Lu; +Cc: Mike Stump, gcc-patches
On 4 September 2015 at 16:54, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Sep 4, 2015 at 7:52 AM, Christophe Lyon
> <christophe.lyon@linaro.org> wrote:
>> On 4 September 2015 at 15:58, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Fri, Sep 4, 2015 at 6:15 AM, Christophe Lyon
>>> <christophe.lyon@linaro.org> wrote:
>>>> On 4 September 2015 at 14:13, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Fri, Sep 4, 2015 at 4:47 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> On Fri, Sep 4, 2015 at 4:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>> On Fri, Sep 4, 2015 at 4:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>> On Thu, Sep 3, 2015 at 8:03 AM, Christophe Lyon
>>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>>> On 3 September 2015 at 13:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>>>> On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
>>>>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>>>>> On 1 September 2015 at 16:04, Christophe Lyon
>>>>>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>>>>>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>>>>>>>>>>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>>>>>>>>>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>>>>>>>>>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>>>>>>>>>>>> these modified flags.
>>>>>>>>>>>>>
>>>>>>>>>>>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>>>>>>>>>>>> is called at the end of every .exp file which overrides
>>>>>>>>>>>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>>>>>>>>>>>
>>>>>>>>>>>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Anyway, all looks good. Ok.
>>>>>>>>>>>>>
>>>>>>>>>>>> Here is what I have committed (r227372).
>>>>>>>>>>>
>>>>>>>>>>> Hmmm, in fact this was r227401.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> It caused:
>>>>>>>>>>
>>>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(dfp,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(label_values,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>>>>>> ...
>>>>>>>>>>
>>>>>>>>>> on Linux/x86-64:
>>>>>>>>>>
>>>>>>>>>> https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I'll have a look.
>>>>>>>>> That's the configuration I used to check before committing, but I am
>>>>>>>>> going to re-check.
>>>>>>>>
>>>>>>>> proc check_cached_effective_target { prop args } {
>>>>>>>> global et_cache
>>>>>>>> global et_prop_list
>>>>>>>>
>>>>>>>> set target [current_target_name]
>>>>>>>> if {![info exists et_cache($prop,target)]
>>>>>>>> || $et_cache($prop,target) != $target} {
>>>>>>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>>>>>>> set et_cache($prop,target) $target
>>>>>>>> set et_cache($prop,value) [uplevel eval $args]
>>>>>>>> lappend et_prop_list $prop
>>>>>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>>>>
>>>>>>>> Aren't you appending $pop to et_prop_list even if it may be already
>>>>>>>> on the list?
>>>>>>>>
>>>>>>>> verbose "check_cached_effective_target cached list is now:
>>>>>>>> $et_prop_list" 2
>>>>>>>> }
>>>>>>>> set value $et_cache($prop,value)
>>>>>>>> verbose "check_cached_effective_target $prop: returning $value for
>>>>>>>> $target" 2
>>>>>>>> return $value
>>>>>>>> }
>>>>>>>>
>>>>>>>
>>>>>>> Like this?
>>>>>>>
>>>>>>> --
>>>>>>> H.J.
>>>>>>> ---
>>>>>>> diff --git a/gcc/testsuite/lib/target-supports.exp
>>>>>>> b/gcc/testsuite/lib/target-supports.exp
>>>>>>> index aad45f9..a6c16fe 100644
>>>>>>> --- a/gcc/testsuite/lib/target-supports.exp
>>>>>>> +++ b/gcc/testsuite/lib/target-supports.exp
>>>>>>> @@ -125,7 +125,9 @@ proc check_cached_effective_target { prop args } {
>>>>>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>>>>>> set et_cache($prop,target) $target
>>>>>>> set et_cache($prop,value) [uplevel eval $args]
>>>>>>> - lappend et_prop_list $prop
>>>>>>> + if {[lsearch $et_prop_list $prop] < 0} {
>>>>>>> + lappend et_prop_list $prop
>>>>>>> + }
>>>>>>> verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
>>>>>>> }
>>>>>>> set value $et_cache($prop,value)
>>>>>>
>>>>>>
>>>>>> It should be
>>>>>>
>>>>>> if {![info exists et_prop_list]
>>>>>> || [lsearch $et_prop_list $prop] < 0} {
>>>>>> lappend et_prop_list $prop
>>>>>> }
>>>>>>
>>>>>
>>>>> Here is a patch. OK for trunk?
>>>>>
>>>>
>>>> It makes sense, indeed, although I still haven't managed to reproduce
>>>> the issue you reported.
>>>
>>> The failure is random with parallel check on machines with >= 8 cores.
>>>
>> In fact that's because you are running the testsuite with several
>> values for 'target' (unix and unix/-m32), which indeed result in
>> appending $prop twice.
>
> Is my patch correct or you have a different fix?
>
It's OK for me, but I can't approve it.
> --
> H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-04 15:02 ` Christophe Lyon
@ 2015-09-04 15:16 ` H.J. Lu
2015-09-04 19:29 ` Mike Stump
0 siblings, 1 reply; 18+ messages in thread
From: H.J. Lu @ 2015-09-04 15:16 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Mike Stump, gcc-patches
On Fri, Sep 4, 2015 at 7:59 AM, Christophe Lyon
<christophe.lyon@linaro.org> wrote:
> On 4 September 2015 at 16:54, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Sep 4, 2015 at 7:52 AM, Christophe Lyon
>> <christophe.lyon@linaro.org> wrote:
>>> On 4 September 2015 at 15:58, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Fri, Sep 4, 2015 at 6:15 AM, Christophe Lyon
>>>> <christophe.lyon@linaro.org> wrote:
>>>>> On 4 September 2015 at 14:13, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> On Fri, Sep 4, 2015 at 4:47 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>> On Fri, Sep 4, 2015 at 4:27 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>> On Fri, Sep 4, 2015 at 4:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>>> On Thu, Sep 3, 2015 at 8:03 AM, Christophe Lyon
>>>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>>>> On 3 September 2015 at 13:31, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>>>>> On Wed, Sep 2, 2015 at 7:02 AM, Christophe Lyon
>>>>>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>>>>>> On 1 September 2015 at 16:04, Christophe Lyon
>>>>>>>>>>>> <christophe.lyon@linaro.org> wrote:
>>>>>>>>>>>>> On 25 August 2015 at 17:31, Mike Stump <mikestump@comcast.net> wrote:
>>>>>>>>>>>>>> On Aug 25, 2015, at 1:14 AM, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>>>>>>>>>>>>>>> Some subsets of the tests override ALWAYS_CXXFLAGS or
>>>>>>>>>>>>>>> TEST_ALWAYS_FLAGS and perform effective_target support tests using
>>>>>>>>>>>>>>> these modified flags.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> This patch adds a new function 'clear_effective_target_cache', which
>>>>>>>>>>>>>>> is called at the end of every .exp file which overrides
>>>>>>>>>>>>>>> ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> So, a simple English directive somewhere that says, if one changes ALWAYS_CXXFLAGS or TEST_ALWAYS_FLAGS then they should do a clear_effective_target_cache at the end as the target cache can make decisions based upon the flags, and those decisions need to be redone when the flags change would be nice.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I do wonder, do we need to reexamine when setting the flags? I’m thinking of a sequence like: non-thumb default, is_thumb, set flags (thumb), is_thumb. Anyway, safe to punt this until someone discovers it or is reasonable sure it happens.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Anyway, all looks good. Ok.
>>>>>>>>>>>>>>
>>>>>>>>>>>>> Here is what I have committed (r227372).
>>>>>>>>>>>>
>>>>>>>>>>>> Hmmm, in fact this was r227401.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> It caused:
>>>>>>>>>>>
>>>>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(arm_neon_ok,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(dfp,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(fsanitize_address,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(ia32,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(ilp32,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(label_values,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(lp64,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>>>>>>> ERROR: can't unset "et_cache(ptr32plus,value)": no such element in array
>>>>>>>>>>> ...
>>>>>>>>>>>
>>>>>>>>>>> on Linux/x86-64:
>>>>>>>>>>>
>>>>>>>>>>> https://gcc.gnu.org/ml/gcc-testresults/2015-09/msg00167.html
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I'll have a look.
>>>>>>>>>> That's the configuration I used to check before committing, but I am
>>>>>>>>>> going to re-check.
>>>>>>>>>
>>>>>>>>> proc check_cached_effective_target { prop args } {
>>>>>>>>> global et_cache
>>>>>>>>> global et_prop_list
>>>>>>>>>
>>>>>>>>> set target [current_target_name]
>>>>>>>>> if {![info exists et_cache($prop,target)]
>>>>>>>>> || $et_cache($prop,target) != $target} {
>>>>>>>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>>>>>>>> set et_cache($prop,target) $target
>>>>>>>>> set et_cache($prop,value) [uplevel eval $args]
>>>>>>>>> lappend et_prop_list $prop
>>>>>>>>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>>>>>>
>>>>>>>>> Aren't you appending $pop to et_prop_list even if it may be already
>>>>>>>>> on the list?
>>>>>>>>>
>>>>>>>>> verbose "check_cached_effective_target cached list is now:
>>>>>>>>> $et_prop_list" 2
>>>>>>>>> }
>>>>>>>>> set value $et_cache($prop,value)
>>>>>>>>> verbose "check_cached_effective_target $prop: returning $value for
>>>>>>>>> $target" 2
>>>>>>>>> return $value
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>
>>>>>>>> Like this?
>>>>>>>>
>>>>>>>> --
>>>>>>>> H.J.
>>>>>>>> ---
>>>>>>>> diff --git a/gcc/testsuite/lib/target-supports.exp
>>>>>>>> b/gcc/testsuite/lib/target-supports.exp
>>>>>>>> index aad45f9..a6c16fe 100644
>>>>>>>> --- a/gcc/testsuite/lib/target-supports.exp
>>>>>>>> +++ b/gcc/testsuite/lib/target-supports.exp
>>>>>>>> @@ -125,7 +125,9 @@ proc check_cached_effective_target { prop args } {
>>>>>>>> verbose "check_cached_effective_target $prop: checking $target" 2
>>>>>>>> set et_cache($prop,target) $target
>>>>>>>> set et_cache($prop,value) [uplevel eval $args]
>>>>>>>> - lappend et_prop_list $prop
>>>>>>>> + if {[lsearch $et_prop_list $prop] < 0} {
>>>>>>>> + lappend et_prop_list $prop
>>>>>>>> + }
>>>>>>>> verbose "check_cached_effective_target cached list is now: $et_prop_list" 2
>>>>>>>> }
>>>>>>>> set value $et_cache($prop,value)
>>>>>>>
>>>>>>>
>>>>>>> It should be
>>>>>>>
>>>>>>> if {![info exists et_prop_list]
>>>>>>> || [lsearch $et_prop_list $prop] < 0} {
>>>>>>> lappend et_prop_list $prop
>>>>>>> }
>>>>>>>
>>>>>>
>>>>>> Here is a patch. OK for trunk?
>>>>>>
>>>>>
>>>>> It makes sense, indeed, although I still haven't managed to reproduce
>>>>> the issue you reported.
>>>>
>>>> The failure is random with parallel check on machines with >= 8 cores.
>>>>
>>> In fact that's because you are running the testsuite with several
>>> values for 'target' (unix and unix/-m32), which indeed result in
>>> appending $prop twice.
>>
>> Is my patch correct or you have a different fix?
>>
> It's OK for me, but I can't approve it.
>
I will check it in as an obvious fix.
--
H.J.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [testsuite] Clean up effective_target cache
2015-09-04 15:16 ` H.J. Lu
@ 2015-09-04 19:29 ` Mike Stump
0 siblings, 0 replies; 18+ messages in thread
From: Mike Stump @ 2015-09-04 19:29 UTC (permalink / raw)
To: H.J. Lu; +Cc: Christophe Lyon, gcc-patches
On Sep 4, 2015, at 8:02 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> It's OK for me, but I can't approve it.
>
> I will check it in as an obvious fix.
Thanks, my 12/24 core box will appreciate it.
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2015-09-04 19:24 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-25 8:17 [testsuite] Clean up effective_target cache Christophe Lyon
2015-08-25 15:44 ` Mike Stump
2015-09-01 14:12 ` Christophe Lyon
2015-09-02 14:02 ` Christophe Lyon
2015-09-03 11:36 ` H.J. Lu
2015-09-03 15:10 ` Christophe Lyon
2015-09-04 11:19 ` H.J. Lu
2015-09-04 11:28 ` H.J. Lu
2015-09-04 12:13 ` H.J. Lu
2015-09-04 12:35 ` H.J. Lu
2015-09-04 13:21 ` Christophe Lyon
2015-09-04 14:21 ` H.J. Lu
2015-09-04 14:54 ` Christophe Lyon
2015-09-04 14:59 ` H.J. Lu
2015-09-04 15:02 ` Christophe Lyon
2015-09-04 15:16 ` H.J. Lu
2015-09-04 19:29 ` Mike Stump
2015-08-25 20:28 ` Jeff Law
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).