Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for AArch64 #811

Closed
oschulz opened this issue Jan 24, 2021 · 3 comments · Fixed by #812
Closed

Support for AArch64 #811

oschulz opened this issue Jan 24, 2021 · 3 comments · Fixed by #812

Comments

@oschulz
Copy link

oschulz commented Jan 24, 2021

I tried to use HDF5 on an ARM AArch64 system (Linux), but ran into the error "cfunction: Closures are not supported on this platform" (see JuliaLang/julia#27174).

Would it be possible to get by without those closures in the cfunction callbacks in HDF5.jl? I'm not sure if the JuliaLang/julia#27174 situation will change or not.

@jmert
Copy link
Contributor

jmert commented Jan 24, 2021

I don't have an aarch64 machine to test with, but this patch does continue to work on x86_64. Could you try this out?

diff --git a/src/api_helpers.jl b/src/api_helpers.jl
index 82b5ee4..3acc56a 100644
--- a/src/api_helpers.jl
+++ b/src/api_helpers.jl
@@ -37,10 +37,19 @@ function h5a_get_name_by_idx(loc_id, obj_name, idx_type, order, idx, lapl_id)
     return String(buf)
 end
 
-function h5a_iterate(f, obj_id, idx_type, order, idx = 0)
+# Some architectures do not support runtime closures with @cfunction, so we instead
+# emulate the behavior by passing the intended callback (which may be a closure) through
+# as the user data pointer to a static Julia helper callback function. The helper then
+# just invokes the user callback via runtime dispatch.
+function h5a_iterate_helper(loc_id::hid_t, attr_name::Ptr{Cchar}, ainfo::Ptr{H5A_info_t}, @nospecialize(f))::herr_t
+    return f(loc_id, attr_name, ainfo, C_NULL)
+end
+function h5a_iterate(@nospecialize(f), obj_id, idx_type, order, idx = 0)
     idxref = Ref{hsize_t}(idx)
-    fptr = @cfunction($f, herr_t, (hid_t, Ptr{Cchar}, Ptr{H5A_info_t}, Ptr{Cvoid}))
-    h5a_iterate(obj_id, idx_type, order, idxref, fptr, C_NULL)
+    fptr = @cfunction(h5a_iterate_helper, herr_t, (hid_t, Ptr{Cchar}, Ptr{H5A_info_t}, Ref{Any}))
+    userf = Ref{Any}(f)
+    GC.@preserve userf h5a_iterate(obj_id, idx_type, order, idxref, fptr,
+                                   unsafe_convert(Ptr{Any}, userf))
     return idxref[]
 end
 
@@ -140,10 +149,16 @@ function h5l_get_name_by_idx(loc_id, group_name, idx_type, order, idx, lapl_id)
     return String(buf)
 end
 
+# See explanation for h5a_iterate above.
+function h5l_iterate_helper(group::hid_t, name::Ptr{Cchar}, info::Ptr{H5L_info_t}, @nospecialize(f))::herr_t
+    return f(group, name, info, C_NULL)
+end
 function h5l_iterate(f, group_id, idx_type, order, idx = 0)
     idxref = Ref{hsize_t}(idx)
-    fptr = @cfunction($f, herr_t, (hid_t, Ptr{Cchar}, Ptr{H5L_info_t}, Ptr{Cvoid}))
-    h5l_iterate(group_id, idx_type, order, idxref, fptr, C_NULL)
+    fptr = @cfunction(h5l_iterate_helper, herr_t, (hid_t, Ptr{Cchar}, Ptr{H5L_info_t}, Ref{Any}))
+    userf = Ref{Any}(f)
+    GC.@preserve userf h5l_iterate(group_id, idx_type, order, idxref, fptr,
+                                   unsafe_convert(Ptr{Any}, userf))
     return idxref[]
 end

I'm not sure this is actually the best implementation, but if it works, it's at least a proof of concept.

@oschulz
Copy link
Author

oschulz commented Jan 24, 2021

Wow, thanks @jmert that worked perfectly! Tested a (not too complex) use case on an Lenovo Chromebook Duet, no problems. And test HDF5 reports Pass 673, Broken 1 - I guess the 1 broken one is expected to be?

Could this be merged into HDF5, or are there performance (or other) implications?

I don't have an aarch64 machine to test with

Yes, I really hope GitHub actions will add ARM64 support for at least one OS soon - it's definitely on the way to be a relevant platform for both computation and desktop. But I think they'll have to do it on OS-X at least (though Linux would be very nice too): actions/runner-images#2187

@giordano
Copy link
Member

giordano commented Jan 25, 2021

@musm musm closed this as completed in #812 Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants