| Message ID | CAMe9rOpPQ8g151238YguaH6xbXbtbm2dO3tYo4HanmokdE2xJA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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.
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.
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.
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?
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.
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.
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.
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)