ZFP Fortran Bindings and nvhpc #163
Comments
|
Update: I looked into it some more today, intercepting the calls in Fortran and in C. Calling |
|
@henryleberre Apologies for a very late response--I did not get notifications for this issue and just now saw it.
Does the Fortran test in tests/fortran/testFortran.f pass? In your example, |
|
@lindstro There is no need to apologize, thank you for responding. I had used tests/fortran/testFortran.f as a reference and do confirm it worked properly. However, adding a call to the Fortran equivalent of Indeed, the example I posted does seem to contain an error. While attempting to create this abridged excerpt I must have misplaced a few lines. The actual code tested different policies, buffer sizes, and rates. I had both C and Fortran versions which had identical structure, but the Fortran one would always fail when setting the execution policy. I had traced through the various layers of the API calls (from the user-facing Fortran function, to the Fortran I circumvented this issue by creating my own C wrapper around ZFP, and creating a Fortran interface for it. I also haven't checked whether recent compiler releases have addressed some of these issues. Interestingly, NVIDIA does somewhat tacitly advertise Fortran 2018 support (though only the "Language" section):
The impression I had after talking to a contact at NVIDIA was that they simply hadn't gotten around to supporting |
|
@henryleberre I can't think off the top of my head why possible lack of |
|
@lindstro I agree, I don't think |
|
No problem. I also found out that, after being a member for over a decade, GitHub decided that my email address was all of a sudden unverified, which presumably is why I've not been getting notifications (though for some reason I'm getting CI and commit status messages). Go figure! Sorry again about the delay. |
|
@henryleberre I've not forgotten about you--we're just dealing with a lot of issues after the 1.0.0 release. After some effort, I was able to build your main.cpp, which gives this output: I'm, however, unable to build main.f90: I'm guessing this has something to do with As you already pointed out, these errors have to do with I'm not sure how to proceed here. This seems more of a NVHPC issue than a zfp one. |
|
Thank you @lindstro for looking into this! As you mentioned, this is an issue with NVHPC, not with ZFP. We would all benefit from it being fixed on the compiler side, rather than having to resort to approximations. I posted on the NVHPC forum about this problem in hopes that they will consider supporting these additional interoperability features in a future release: https://forums.developer.nvidia.com/t/f2018-ts-29113-compliance/227197. The |
|
NVHPC Fortran does not claim support for TS 29113 nor do we support it. However, if all you need is ptrdiff_t, why not just use c_size_t or c_int64_t? Anyways, I'll reproduce and try to fix tomorrow. Unless I've missed something, there's a trivial workaround. Also, feel free to tag me in the future when you have NVHPC issues. |
|
The problem is that we're using |
|
Thank you @jeffhammond for the clarification. I had replaced |
|
I'll play around with it tomorrow (I'm 10 hours ahead of you). I've spent a lot of time on CFI features lately but haven't tested everything obviously. |
Thank you very much MatColgrove and jeffhammond for the clarification and help on this issue. I wasn't sure whether this was a bug or an unimplemented feature. Since some major F2018 features had already been implemented (particularly standard parallelism) and the ISO standard was referenced, I naïvely assumed NVFortran was an F2018 compiler. I expect most codes probably don't rely on all of the new features. Switching to the Flang front-end is an excellent idea. My experience with Flang and the LLVM toolchain in general has been excellent. The middle-end and the back-end are what make NVHPC such a great toolkit for HPC with NVIDIA GPUs. @jeffhammond I haven't looked into this issue in quite some time since I circumvented it by creating my own C wrapper & Fortran interface, but it would be my pleasure to help. |
|
This is what I see right now with NVHPC and GCC after applying a trivial patch. Is this success or failure in the test? diff --git a/fortran/zfp.f90 b/fortran/zfp.f90
index f671d14..861eaed 100644
--- a/fortran/zfp.f90
+++ b/fortran/zfp.f90
@@ -1,6 +1,6 @@
module zfp
- use, intrinsic :: iso_c_binding, only: c_int, c_int64_t, c_size_t, c_ptrdiff_t, c_double, c_ptr, c_null_ptr, c_loc
+ use, intrinsic :: iso_c_binding, only: c_int, c_int64_t, c_size_t, c_double, c_ptr, c_null_ptr, c_loc
implicit none
private
@@ -464,25 +464,25 @@ module zfp
subroutine zfp_field_set_stride_1d(field, sx) bind(c, name="zfp_field_set_stride_1d")
import
type(c_ptr), value :: field
- integer(c_ptrdiff_t) :: sx
+ integer(c_size_t) :: sx
end subroutine
subroutine zfp_field_set_stride_2d(field, sx, sy) bind(c, name="zfp_field_set_stride_2d")
import
type(c_ptr), value :: field
- integer(c_ptrdiff_t) :: sx, sy
+ integer(c_size_t) :: sx, sy
end subroutine
subroutine zfp_field_set_stride_3d(field, sx, sy, sz) bind(c, name="zfp_field_set_stride_3d")
import
type(c_ptr), value :: field
- integer(c_ptrdiff_t) :: sx, sy, sz
+ integer(c_size_t) :: sx, sy, sz
end subroutine
subroutine zfp_field_set_stride_4d(field, sx, sy, sz, sw) bind(c, name="zfp_field_set_stride_4d")
import
type(c_ptr), value :: field
- integer(c_ptrdiff_t) :: sx, sy, sz, sw
+ integer(c_size_t) :: sx, sy, sz, sw
end subroutine
function zfp_field_set_metadata(field, encoded_metadata) result(is_success) bind(c, name="zfp_field_set_metadata")
@@ -1065,26 +1065,26 @@ contains
subroutine zFORp_field_set_stride_1d(field, sx) bind(c, name="zforp_field_set_stride_1d")
type(zFORp_field), intent(in) :: field
integer, intent(in) :: sx
- call zfp_field_set_stride_1d(field%object, int(sx, c_ptrdiff_t))
+ call zfp_field_set_stride_1d(field%object, int(sx, c_size_t))
end subroutine zFORp_field_set_stride_1d
subroutine zFORp_field_set_stride_2d(field, sx, sy) bind(c, name="zforp_field_set_stride_2d")
type(zFORp_field), intent(in) :: field
integer, intent(in) :: sx, sy
- call zfp_field_set_stride_2d(field%object, int(sx, c_ptrdiff_t), int(sy, c_ptrdiff_t))
+ call zfp_field_set_stride_2d(field%object, int(sx, c_size_t), int(sy, c_size_t))
end subroutine zFORp_field_set_stride_2d
subroutine zFORp_field_set_stride_3d(field, sx, sy, sz) bind(c, name="zforp_field_set_stride_3d")
type(zFORp_field), intent(in) :: field
integer, intent(in) :: sx, sy, sz
- call zfp_field_set_stride_3d(field%object, int(sx, c_ptrdiff_t), int(sy, c_ptrdiff_t), int(sz, c_ptrdiff_t))
+ call zfp_field_set_stride_3d(field%object, int(sx, c_size_t), int(sy, c_size_t), int(sz, c_size_t))
end subroutine zFORp_field_set_stride_3d
subroutine zFORp_field_set_stride_4d(field, sx, sy, sz, sw) bind(c, name="zforp_field_set_stride_4d")
type(zFORp_field), intent(in) :: field
integer, intent(in) :: sx, sy, sz, sw
- call zfp_field_set_stride_4d(field%object, int(sx, c_ptrdiff_t), int(sy, c_ptrdiff_t), &
- int(sz, c_ptrdiff_t), int(sw, c_ptrdiff_t))
+ call zfp_field_set_stride_4d(field%object, int(sx, c_size_t), int(sy, c_size_t), &
+ int(sz, c_size_t), int(sw, c_size_t))
end subroutine zFORp_field_set_stride_4d
function zFORp_field_set_metadata(field, encoded_metadata) result(is_success) bind(c, name="zforp_field_set_metadata") |
|
Also, if you rename this file, you no longer need free source form flags. |
|
@jeffhammond Thanks for your help with this. Yes, the test output is as expected, but looking at our test in detail for the first time, it is not much of a "test." The decompressed data is not really validated (other than printing the difference between arrays), and the test essentially misuses zfp's support for integer compression, not to mention that it confuses ints and floats. We'll need to clean this up and add a bit more rigor. By the way, what's a portable way in Fortran to return success/error to the OS, which About your proposed fix, what happens if a stride is negative? The C function signature for |
|
Fortran STOP should return numerical values. I think that is what you want for ctest but I haven't verified. I'm not aware of any relevant system where size_t, ptrdiff_t and intptr_t aren't the same size. I also don't worry about C in theory, because all nontrivial C programs contain undefined behavior and therefore are permitted to launch nukes. I focus on the computers that exist and use 2s-complement integers. |
|
I know very little about Fortran, but my understanding is that STOP does not necessarily return a value (the PGI Fortran compiler just prints the value of the STOP argument, for instance). It turns out that I agree it's reasonable to expect that At any rate, the issue at hand is the problem with
That said, there are numerous zfp functions that accept enumerated types, so if this fails, it should fail elsewhere, e.g. in |
|
I guess I don't see where the undefined behavior comes in here. Fortran (ISO/IEC DIS 1539-1:2022) says the following:
Assuming 2s-complement behavior is also safe, because every computer does that and both C and C++ have standardized it:
If you are worried about the potentially unsigned nature of #ifdef __NVCOMPILER
integer, parameter :: ptrdiff_kind = c_int64_t
#else
integer, parameter :: ptrdiff_kind = c_ptrdiff_t
#endifand use this everywhere: |
|
Regarding C
The other options are to write a shim for Fortran to C that safely casts from whatever Fortran uses to the enum. You could always use You can also figure out what the C compiler with your enums and select from |
|
I was commenting on undefined behavior in the context of "all nontrivial C programs contain undefined behavior." :-) C90 and C99 (which are the standards zfp targets) allow signed integer trap representations, which invoke UB, and representations other than two's complement (as does C11--not sure about C17), though they are of course rare. To my knowledge, N2218 is still in proposal form and has not been adopted yet. Hence, it's possible (albeit very unlikely) that UB would occur with Even if these issues are or do get resolved in recent/future C standards, we've made a conscious effort to code to older standards. F2018 is an exception as we recently learned that On ILP32 systems, which we do accommodate, Thanks for the suggestion on enums. I'll see if that solves the problem. |
|
What I tried to say before is, I do not think you can overflow when Fortran passes I don't have an argument if you insist on assuming nothing more than what is in C99. All I can say is that everybody else, including the C++ committee, assumes 2s-complement, because there hasn't been a 1s-complement computer architecture designed since the 1960s. https://herbsutter.com/page/5/
I haven't looked at the voting logs but I thought JF tweeted about the C proposal passing already. From https://en.wikipedia.org/wiki/Signed_number_representations:
|
So what happens when a negative "plain" integer is passed as argument to a Fortran function that takes an Now, it is unclear to me when the conversion to Since signatures now differ, would this not confuse the linker? There is no function that takes a If, on the other hand, conversion does happen, that's where implementation defined behavior is needed. It's not so much whether the implementation uses two's complement but rather how it responds to integer conversion. From C99 6.3.1.3:
It's perfectly valid for an overflowing conversion from gives as output
Fair enough (though all C standards I'm aware of, including the latest C17 draft, explicitly allow sign-magnitude and one's complement), but I think your assumption is even stronger: that integer conversions do nothing but reinterpret the binary representation (i.e., in addition to using two's complement for signed integers). That may also be a good assumption when two's complement is used since then it makes perfect sense. But in general I think those are orthogonal issues. |
I ran into a few hurdles while attempting to compile and use ZFP with Cuda support using NVHPC v22.5 and its Fortran bindings. I might have misinterpreted part of the documentation, and would appreciate your help, especially with my last issue which I haven't yet resolved.
System details
Building ZFP
The CMake options bellow didn't change which host compiler was used. However, it does work if I export the
$CUDAHOSTCXXenvironment variable or specify-DCMAKE_CUDA_FLAGS="--compiler-bindir \"$(which nvc++)\".-DCMAKE_CUDA_COMPILER_ID=NVHPC-DCMAKE_CUDA_HOST_COMPILER="$(which nvc++)"Without
-DCMAKE_Fortran_FLAGS="-Mfree", an error (sometimes) occurs while linking zFORp. It may require an older version of NVHPC or a specific computer (mis)configuration to reproduce. I couldn't reproduce it. Some systems may have prepended it to the$FFLAGSenvironment variable.The commit c367213 brakes compatibility for me with NVHPC, citing the following error:
The error pertains to
c_ptrdiff_twhich NVHPC doesn't recognize correctly. From my understanding, it was part of the TS 29113 extension (source) which NVIDIA should support. Courtesy of @sbryngelson, here is a minimal reproduceable example:@sbryngelson verified that the above code does compile and run on both GNU and Intel compilers, so this issue is specific to NVHPC.
To get ZFP building, I replaced these lines with:
integer(c_int) ...However, the size of this type is most certainly not appropriate (32 vs 64 bits). I'm not currently using these functions should I proceeded with this temporary fix.
Here are the final build commands I used after some troubleshooting. Some options might be redundant.
Running ZFP
I haven't yet resolved this last issue, and would greatly appreciate your guidance. I wrote two test programs (in C++ and Fortran respectively), that benchmark compression and decompression with different buffer sizes, execution engines, and fixed-compression rates, writing the results to a file. These codes are available here.
The C version manages to to activate the available execution policies using:
I compiled and ran it with the bellow commands. (I cloned zfp into the root of my benchmarking code's directory).
However, the Fortran version fails on all policies, including
zFORp_exec_serialusing this code:Compiled with
cmake .. && make -j 8and this CMakeLists.txt file. I run it using$ LD_LIBRARY_PATH="zfp/build/lib:$LD_LIBRARY_PATH" ./build/zfp_experimentThe only Fortran example I could find doesn't use this function. I am new to Fortran (my background is in C++) so I might be candidly overseeing an obvious error.
The text was updated successfully, but these errors were encountered: