Re: [PATCH v2, Fortran] TS 29113 testsuite

[Tobias Burnus <tobias@codesourcery.com>]

Hi Sandra, hi Thomas, hi all,

@Thomas K: Comments about the following - and of course to the
testsuite itself - are highly welcome.

In my opinion, the testsuite LGTM and can be committed.

@Sandra:
- Thoughts on the directory name? (cf. below)
- Give others/Thomas a chance to comment on this,
   before committing it. (And remove the now passing xfails.)
   Thanks for the testsuite!

Regarding:

* XFAILS - as discussed before, I think having some XFAILS is
   not ideal but fine, especially if the XFAIL/PASS ratio is low
   and there are plans to fix the known fails, some posted patches
   for those, and open PRs for the issues.

(I think there is one patch pending review and two patches pending
committal with some modifications by Sandra - plus several patches
by José which still need to be reviewed.)


* Naming of the directory + .exp file:
      ts29113/ts29113.exp
   is okay. Given that 'select rank' (in F2018 but not in TS29113)
   is also tested, there was some controversy regarding the name
   and the coverage; additionally, TS29113 is a name which is not
   immediately clear. Thus, we could use some other name like:
      c-interop/c-interop.exp
   or .... (suggestions?).
   In any case, I do not feel strong about either name.

* I had a closer look at earlier versions of the testsuite, I did
   browse through the current one + looked at the diff to previous
   version, but it is big enough and the spec is complex enough that
   I have likely missed something.
   Thus: Additional reviews are highly welcome!


Other than that:

On 25.07.21 21:47, Sandra Loosemore wrote:
> Here is an updated version of my TS29113 testsuite.
...
> And this approved but not-yet-committed patch from Jose
> https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572725.html
> fixes 6 more.

The patch was committed as r12-2511-g0cbf03689e3e7d9d6002b8e5d159ef3716d0404c
[see also PR fortran/101635 for an open issue]:

I can confirm that gfortran.dg/ts29113/interoperability/cf-descriptor-2.f90
now shows XPASS (6 times for -m64, 6 times for -m32).

Overall, I see (x86-64-gnu-linux):

                 === gfortran Summary for unix ===
# of expected passes            1041
# of unexpected successes       6
# of expected failures          269
# of unresolved testcases       30

                 === gfortran Summary for unix/-m32 ===
# of expected passes            1029
# of unexpected successes       6
# of expected failures          257
# of unresolved testcases       30
# of unsupported tests          12

Unexpected success -> see XPASS above
Unsupported = dg-require-effective-target, unsupported for -m32.
Unresolved = dg-do run, but failed to compile

  * * *

Regarding:
gfortran.dg/ts29113/interoperability/contiguous-2.f90
     ! FIXME: is this correct?  "the Fortran processor will handle the
     ! difference in contiguity" may not mean that it's required to make
     ! the array contiguous, just that it can access it correctly?

I think the latter is permitted - and makes sense when the contiguity
is not needed. For instance,
   arg = 5  ! -> implicit loop + array-element access
does not require a contiguous array, even though assuming sm = sizeof(elem)
might generate faster code, e.g. by permitting vector ops. And for
   size(arg)
it does not really matter whether 'arg' is contiguous or not. But for any
call which needs a contiguous argument, contiguity is required -
which then implies that the compiler has to make the argument contiguous
(copy into a contiguous temporary).

The simplest is to always do the copy-in-into-temp (when not contiguous),
but of course there is room for optimization - like always.

In terms of the testsuite, I think the test as written is fine:
if later more optimizations are done in GCC, the GCC developer can still
inspect the FAIL, read your comment, and modify the testcase accordingly.

Thanks,

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955