Add stdlib_experimental_kinds.f90 and use it

[certik]

This implements the latest discussion at #25.

[jvdp1]

Is int128 supported by all compilers?

[zbeekman]

That's a good question. If C has standardized int128 I would image that it is. But I need to consult the standard for more insight.

[jvdp1]

At least I don't find c_int128_t in iso_c_binding or int128iniso_fortran_env`

[certik]

But it compiled. It might be only supported by gfortran?

[zbeekman]

The standard says that it will compile, but be set to a special value when not supported (missing) if my memory is correct. So compilation doesn't guarantee existence of the kind. (I think the special value is 0 or a negative integer.)

[marshallward]

It looks like C99 has no guaranteed 128-bit integer (long long is "at least 64 bits" and intN_t is only guaranteed up to 64).

Looking at my own include files (libc 2.30), I don't see anything for 128-bit integers except a few places under an __ILP32__ preprocessor flag, which must have been unset when my platform-specific includes were preprocessed for my system.

[marshallward]

Also, I think one does need to be careful with quad precision. On its own, it does not have a clear meaning. There are (at least) three active definitions:

https://gcc.gnu.org/onlinedocs/gcc/Floating-Types.html

For example, on my machine, double and long double both point to _Float64. My libc also defines a _Float64x which as best I can tell corresponds to the x86 float80 format.

[marshallward]

BTW I am looking at a copy of the 2018 standard now (Table 18.2 in section 18.3.1), and I don't see any reference to c_int128_t in there? Only up to c_int64_t. Also no reference to c_float128.

[zbeekman]

Looks like the Makefile needs updating.

[certik]

@marshallward I think you were initially against having a kinds module, so I would like your feedback on this. This PR is based on the discussion in #25.

[zbeekman]

@marshallward I think you were initially against having a kinds module, so I would like your feedback on this. This PR is based on the discussion in #25.

I think @jacobwilliams expressed some reservation in #13 too, but I might be reading between the lines too much.

[zbeekman]

LGTM

[milancurcic]

Great! I learned something new about Makefiles too :)

[certik]

I rebased on top of the latest master and resolved all conflicts.

[certik]

I rebased again. The required Makefile changes are now minimal.

@marshallward, @jacobwilliams just a reminder that this waits for you to review it, as you might have some objections to this.

[marshallward]

@certik Sorry, missed this request back when I was still travelling, I will get caught up on this tonight and give my feedback.

[marshallward]

I think I've added my specific comments; the main point is that using types as defined by C do not necessarily define these things sufficiently once we get to 128-bit types (and, presumably, the half-floats etc).

I guess my preference is still to choose names which reflect the type (float64 rather than dp, for example) but I don't want such minor preferences to block progress in other modules.

I do think there's an opportunity to determine which hardware types are available at compile time of stdlib, and then conditionally define these inside stdlib (Edit: and determine their kind IDs). But that is also a much bigger job, and I am very much a novice with CMake to attempt it :).

As it stands, I'm OK with it, just take these words as reflections on the overall problem.

[marshallward]

I might revise my review and suggest that the 128 formats be dropped until they can be investigated robustly at build time (e.g. CMake).

[certik]

I personally prefer to import the kinds from iso_fortran_env, but it seems most others preferred C. But now with the kinds module, this is easy to change. Regarding dp, sp and qp, it's not set in stone, we are still in experimental. I plan to do a thorough review of Fortran projects out there to get a better idea what the community uses. Regarding the quadruple precision, I think there is only one type: https://en.m.wikipedia.org/wiki/Quadruple-precision_floating-point_format Besides double precision there is also 80 bit. I think what you meant to say is that the C float128 might not always map to quadruple precision and might sometimes map to 80bit? If that's the case, then we should use the iso_fortran_env

…

On Fri, Jan 3, 2020, at 6:02 PM, Marshall Ward wrote: ***@***.**** commented on this pull request. In src/stdlib_experimental_kinds.f90 <#63 (comment)>: > @@ -0,0 +1,10 @@ +module stdlib_experimental_kinds +! Instead of iso_fortran_env, we use iso_c_binding, to be compatible with C +!use iso_fortran_env, only: sp=>real32, dp=>real64, qp=>real128 +!use iso_fortran_env, only: int32, int64, int128 +use iso_c_binding, only: sp=>c_float, dp=>c_double, qp=>c_float128 +use iso_c_binding, only: int32=>c_int32_t, int64=>c_int64_t, int128=>c_int128_t Also, I think one does need to be careful with quad precision. On its own, it does not have a clear meaning. There are (at least) three active definitions: https://gcc.gnu.org/onlinedocs/gcc/Floating-Types.html For example, on my machine, `double` and `long double` both point to `_Float64`. My libc also defines a `_Float64x` which as best I can tell corresponds to the x86 float80 format. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#63?email_source=notifications&email_token=AAAFAWCIDSQYGNDYEKWYX6DQ37N27A5CNFSM4KCD6RGKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCQVHR6I#discussion_r363005223>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWDMI7IENQKQ2KPX5GLQ37N27ANCNFSM4KCD6RGA>.

[certik]

Given that they currently work in master, I would suggest to keep quadruple precision, but create an issue with a todo list of things that we need to review and possibly change in experimental, before considering to move to main.

…

On Fri, Jan 3, 2020, at 6:39 PM, Marshall Ward wrote: I might revise my review and suggest that the 128 formats be dropped until they can be investigated robustly at build time (e.g. CMake). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#63?email_source=notifications&email_token=AAAFAWFVGEEYB7NGCEPDNSLQ37SFLA5CNFSM4KCD6RGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEICOHSI#issuecomment-570745801>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWCMCU7QK2VE3FFXDA3Q37SFLANCNFSM4KCD6RGA>.

[marshallward]

@certik yes, that's a better way to put it. There is a "float128", but it's often not what one gets when one requests a quad precision number. (Maybe my brief experience on IBM Powers is showing?)

I agree that iso_fortran_env might be safer in this case, assuming that the Fortran compiler does the heavy lifting here.

[marshallward]

Also, given that this to be a reference library, are we sure about using c_int128_t and c_float128 when they are not defined in the standard?

Conditionally setting these inside a bit of GFortran preprocessing might be OK, though.

[certik]

@zbeekman, @scivision you were for using iso_c_binding here. Do you have any comments to @marshallward's review?

I think iso_fortran_env is probably the safest. But if we can't get an agreement on this, then let's revisit this later.

As a compromise, we can stay with iso_fortran_env, as that's what's already in master.

@marshallward would you be against introducing the stdlib_kinds module, even if we use iso_fortran_env inside?

[marshallward]

I don't have any objection to a stdlib_kinds module, and pointing to iso_fortran_env is probably the better option, since it would presumably(?) resolve the platform-specific issues. (It also helps that real128 is actually in the standard!)

I apologize if it seemed like I was against the idea of providing such a module, I think it is very valuable! I was only a bit resistant to the names (as usual), and the handling of quad types.

In the future I could see it being extended it to include kinds for more explicit types, such as float128, ibm128, and float80.

This situation could re-emerge with the many new architectures coming into the market and the newer formats like float16 and decimalN becoming more popular. There may be value in providing kind IDs for these explicit data types.

[certik]

@marshallward thanks for the clarification. So it seems we are all in agreement about having stdlib_kinds module. So I modified the PR to keep using iso_fortran_env as in master, but now it is in the module. Then let's create another PR or an issue where we can discuss if we want to change it to use iso_c_binding. I actually do not care at all about this, now when we have it at just one place in a stdlib_kinds module.

Regarding the names of sp, dp, qp, this PR does not change anything either. I plan to do a wide survey soon of the open source Fortran codes and gather some data on this and we can revisit this in the future. Definitely before moving from experimental to main.

Anyway, is anybody against merging this PR as is?

[scivision]

I think it's fine to use iso_fortran_env and when/if issues arise with C interoperability, fix it then.

[certik]

Ok then. I am going to merge this, as we seem to be in agreement or not opposed to this latest version of the PR. Now we can easily change how the constants are defined in one module.

As for the naming, I created #85, so that we do not forget.

[zbeekman]

Yes, given the points raised, I think the best thing for now is to use iso_fortran_env thanks for the careful discussion and consideration everyone, especially @marshallward!