| Message ID | CAKdteOZExg70QEBqnXku6zt174NjY=2nioA7tJAsfiX76e9uAQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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.
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
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.
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 }
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 }
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.
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 }