Re: [PATCH][GCC][testsuite] Fix caching of tests for multiple variant runs and update existing target-supports tests.

[Richard Sandiford <richard dot sandiford at arm dot com>]

Tamar Christina <Tamar.Christina@arm.com> writes:
> The 09/27/2018 09:40, Richard Sandiford wrote:
>> Tamar Christina <tamar.christina@arm.com> writes:
>> > @@ -120,22 +121,41 @@ proc check_cached_effective_target { prop args } {
>> >      global et_prop_list
>> > 
>> >      set target [current_target_name]
>> > -    if {![info exists et_cache($prop,target)]
>> > -	|| $et_cache($prop,target) != $target} {
>> > +    if {![info exists et_cache($prop,$target)]} {
>> >  	verbose "check_cached_effective_target $prop: checking $target" 2
>> > -	set et_cache($prop,target) $target
>> > -	set et_cache($prop,value) [uplevel eval $args]
>> > +	if {[string is true -strict $args] || [string is false -strict $args]} {
>> > +	    set et_cache($prop,$target) $args
>> > +	} else {
>> > +	    set et_cache($prop,$target) [uplevel eval $args]
>> > +	}
>> 
>> Why are the checks for true and false needed?  We shouldn't be
>> using this function for something that's already true or false.
>> 
>
> I had wanted to be a bit lenient here in accepting it if you have a
> function that already fully evaluated the expr before passing it to
> check_cached_effective_target.
>
> i.e. something like
>
>  proc check_effective_target_vect_int { } {
>      return [check_cached_effective_target_indexed <name> [
>        expr {
>           <condition>
>      }]]
>  }
>
> The error you would get if you do this is very confusing so I thought
> since it didn't matter much for the regexp only target triple tests
> that just accepting this would be fine.

Seems a good thing that that's a noisy failure; the function should
make up its mind whether it wants to cache (use curly braces) or not
(just return the expr directly).

> Should I drop it or keep it?

Think we should either drop it or make it into a more user-friendly
error, e.g.:

	if {[string is true -strict $args] || [string is false -strict $args]} {
	    error {check_cached_effective_target condition already evaluated; did you pass [...] instead of the expected {...}?}
	} else {
	    set et_cache($prop,$target) [uplevel eval $args]
	}

Thanks,
Richard