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
