Re: [PATCH] PowerPC, fix gdb.base/retval-large-struct.exp

[Tom de Vries <tdevries@suse.de>]

On 11/18/22 20:11, Carl Love wrote:
> Tom:
> 
> On Fri, 2022-11-18 at 17:25 +0100, Tom de Vries wrote:
>> On 11/18/22 17:04, Ulrich Weigand wrote:
>>> Tom de Vries <tdevries@suse.de> wrote:
>>>
>>>> AFAIU, needing -fvar-tracking is specific to powerpc, so we
>>>> should limit
>>>> it's impact to that target.
>>>>
>>>> And it's a gcc compiler flag, so perhaps we should limit it's
>>>> impact to
>>>> that as well.
>>>
>>> No, it's not really powerpc specific - the same mechanism can be
>>> used on many other platforms with an ABI that uses a return buffer
>>> address that is not preserved.  (E.g. we're currently looking into
>>> enabling it on s390x.)
>>>
>>
>> Right, so we can add those archs to the list of archs requiring
>> -fvar-tracking support when gcc is used.
>>
>>> And given that the flag is harmless if it's available (which the
>>> test verifies), I think it makes sense to just always enable it.
>>>
>>
>> That's fine by me.
>>
>> Then I get:
>> ...
>> set flags {}
>>
>> lappend flags debug
>>
>> if { [have_fvar_tracking] } {
>>       lappend flags -fvar-tracking
>> } else {
>>       if { ( [istarget powerpc*-*-*] || [istarget s390x*-*-* ])
>>            && [is_c_compiler_gcc] } {
>>           unsupported "gcc used, -fvar-tracking needed"
>>           return -1
>>       }
>> }
>>
>> if {[prepare_for_testing "failed to prepare" $testfile $srcfile
>> $flags]} {
>>          return -1
>> }
> 
> The [have_fvar_tracking] does a runtime test to see if the compiler
> supports the -fvar-tracking option.  So if it passes, then the option
> is supported.

Agreed.

> The test makes the additional check  [istarget powerpc*-
> *-*] || [istarget s390x*-*-* ]) && [is_c_compiler_gcc]  redundant.
> 

My concern is older gcc compilers that do not support -fvar-tracking. 
Those have no chance of succeeding, so we should bail out for those.

OTOH, non-gcc compilers may or may not provide sufficient debug 
information, so there's no need to bail out for those.

Thanks,
- Tom

> I don't think there is any need for the additional checks.
> 
>                    Carl
>