Re: [PATCH] gdb/testsuite: Testing with the armflang compiler

[Richard Bunt <richard.bunt@linaro.org>]

On 09/06/2023 11:29, Luis Machado wrote:
> On 6/9/23 11:16, Richard Bunt via Gdb-patches wrote:
>> Currently the Fortran test suite does not run with armflang because the
>> compiler detection fails. This in turn means fortran_runto_main does not
>> know which main method to use to start a test case.
>>
>> Fortran compiler detection was added in 44d469c5f85; however, the commit
>> message notes that it was not tested with armflang.
>>
>> This commit tests and fixes up a minor issue to get the detection
>> working.
>>
>> The goal here is to get the tests running and preventing further
>> regressions during future work. This change does not do anything to fix
>> existing failures.
>>
>>> From what I can understand, the auto detection leverages the
>> preprocessor to extract the Fortran compiler identity from the defines.
>> This preprocessor output is then evaluated by the test suite to import
>> these defines.
>>
>> In the case of armflang, this evaluation step is disrupted by the
>> presence of the following warning:
>>
>>      $ armflang -E -fdiagnostics-color=never 
>> testsuite/lib/compiler.F90 -o compiler.exp
>>      $ clang-13: warning: argument unused during compilation: 
>> '-fdiagnostics-color=never' [-Wunused-command-line-argument]
>>
>> The evaluation logic is already set up to filter this warning, but the
>> prefix differs.
>>
>> This commit fixes the issue by updating the filter to exclude the
>> armflang flavour of warning.
>>
>> gdb.fortran regression tests run with GNU, Intel and Intel LLVM. No
>> regressions detected.
>>
>> The gdb.fortran test results with ACfL 23.04.1 are as follows.
>>
>> Before:
>>
>>   # of expected passes        560
>>   # of unexpected failures    113
>>   # of unresolved testcases    2
>>   # of untested testcases    5
>>   # of duplicate test names    2
>>
>> After:
>>
>>   # of expected passes        5388
>>   # of unexpected failures    628
>>   # of known failures        10
>>   # of untested testcases    8
>>   # of unsupported tests        5
>>   # of duplicate test names    5
>>
>> As can be seen from the above, there are now considerably more passing
>> assertions.
>>
>> ---
>>   gdb/testsuite/lib/gdb.exp | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index 220640210cd..b1d5ef3fef1 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -4531,7 +4531,7 @@ proc get_compiler_info {{language "c"}} {
>>           # eval this line
>>           verbose "get_compiler_info: $cppline" 2
>>           eval "$cppline"
>> -    } elseif { [ regexp "flang.*warning.*'-fdiagnostics-color=never'" 
>> "$cppline"] } {
>> +    } elseif { [ regexp 
>> "\[fc\]lang.*warning.*'-fdiagnostics-color=never'" "$cppline"] } {
>>           # Both flang preprocessors (llvm flang and classic flang) 
>> print a
>>           # warning for the unused -fdiagnostics-color=never, so we 
>> skip this
>>           # output line here.
> 
> Is my understanding correct that this is now handling both flang and 
> clang? And clang is output for the armflang compiler?
> 

That is correct. The regex has been extended to accept warnings prefixed 
with either "clang" or flang". This is because I observe the armflang 
preprocessor to emit warnings prefixed with clang.

> It looks OK to me, but you might want to add an explanation, in a 
> comment, about why we're trying to handle clang for a fortran compiler.


Agree, that is a little confusing without additional context. I plan to 
prepare a v2 with the following comment:

# The armflang preprocessor has been observed to output the
# warning prefixed with "clang", so the regex also accepts
# this.

 > Reviewed-By: Luis Machado <luis.machado@arm.com>

and the Reviewed-By tag.

Thanks for the review!

Rich