-
Notifications
You must be signed in to change notification settings - Fork 82
Test embedded bitcode path #2684
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
base: main
Are you sure you want to change the base?
Conversation
|
Your PR requires formatting changes to meet the project's style guidelines. Click here to view the suggested changes.diff --git a/test/embedded_bitcode.jl b/test/embedded_bitcode.jl
index c1dbf0ab..8895ca1f 100644
--- a/test/embedded_bitcode.jl
+++ b/test/embedded_bitcode.jl
@@ -43,7 +43,7 @@ tmp_so_file = joinpath(tmp_dir, "func.so")
run(
pipeline(
`$(clang()) -x ir - -Xclang -no-opaque-pointers -O3 -fPIC -fembed-bitcode -shared -o $(tmp_so_file)`;
- stdin=IOBuffer(FUNC_LLVM_IR)
+ stdin = IOBuffer(FUNC_LLVM_IR)
)
)
@@ -54,15 +54,19 @@ const fptr = Libdl.dlsym(lib, :func_wrap)
function func_ccall(t::Float64, arr::AbstractVector{Float64})
nitems = length(arr)
bitsize = Base.elsize(arr)
- GC.@preserve arr begin
+ return GC.@preserve arr begin
excinfo = Ptr{Ptr{Cvoid}}(C_NULL)
base::Ptr{Cdouble} = pointer(arr)
- ccall(fptr, Cdouble,
- (Ptr{Ptr{Cvoid}}, Cdouble, Ptr{Cvoid}, Ptr{Cvoid},
- Clong, Clong, Ptr{Cdouble}, Clong, Clong),
+ ccall(
+ fptr, Cdouble,
+ (
+ Ptr{Ptr{Cvoid}}, Cdouble, Ptr{Cvoid}, Ptr{Cvoid},
+ Clong, Clong, Ptr{Cdouble}, Clong, Clong,
+ ),
excinfo, t, C_NULL, C_NULL, nitems, bitsize,
- base, nitems, nitems * bitsize)
+ base, nitems, nitems * bitsize
+ )
end
end
@@ -77,7 +81,7 @@ end
err_llvmir = nothing
b = @view a[1:5]
- redirect_stdio(stdout=errstream, stderr=errstream, stdin=devnull) do
+ redirect_stdio(stdout = errstream, stderr = errstream, stdin = devnull) do
try
gradient(Reverse, func_ccall, Const(0.0), b)
catch e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I seem to remember LLVM IR isn't very portable across major versions, I presume that'd be a potential problem here when testing different Julia (and llvm) versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you, and I also didn't feel this was the best test case.
The only way I could reproduce this error was with ccall inside a wrapper function. On the contrary, if I usellvmcall, I could get the gradient correctly. When I inspected the LLVM IR when passing a view of an array, Julia inlines the wrapper function with weakly typed subarray pointers, hence the mismatch at the callsite and the inner ccall.
Also, have a look at this comment in the original issue.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2684 +/- ##
==========================================
- Coverage 72.58% 72.50% -0.08%
==========================================
Files 58 58
Lines 18739 18743 +4
==========================================
- Hits 13602 13590 -12
- Misses 5137 5153 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| tmp_dir = tempdir() | ||
| tmp_so_file = joinpath(tmp_dir, "func.so") | ||
| run( | ||
| pipeline( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this even necessary here?
if you're starting from llvm anyways, why not just use llvmcall?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surprisingly, I won't hit that execution path when using llvmcall. Have a look at this comment here
#2664 (comment)
The wrapper function around llvmcall handles the SubArray input with no issues, whereas the same wrapper function around ccall will fail to compute the gradient. I stopped short of investigating the core issue; the only clue I got from the emitted LLVM IR is that Julia inlines the wrapper function, which leads to a mismatch between the types at the callsite.
|
Could we use a small C library instead of using LLVM IR bitcode? |
Great point! Will consider this. |
Test added for capturing
Broken functionexception and its correspondingstdoutmessage regardingCalled function is not the same type as the call!due to automatic inlining ofccallfunction.See issue #2664