-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
Switch LLVM reflection utilities to be CodeInstance
based
#56876
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: master
Are you sure you want to change the base?
Conversation
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.
The src
argument is not optional (nullable)
It is in this PR. |
Then this PR cannot be merged, since it cannot be optional, as codegen has no other reliable way to get that |
It just picks out the |
Can proceed with other changes here now, if still relevant
I rebased this because I thought I needed it, but turns out I don't immediately, but i think it might be useful for @topolarity shortly, so leaving this up anyway. |
|
||
jl_printf(stream, "---- dumping IR for ----\n"); | ||
jl_static_show(stream, (jl_value_t*)mi); | ||
jl_printf(stream, "\n----\n"); | ||
|
||
jl_printf(stream, "\n---- unoptimized IR ----\n"); | ||
jl_get_llvmf_defn(&llvmf_dump, mi, src, 0, false, jl_default_cgparams); | ||
jl_get_llvmf_defn(&llvmf_dump, ci, NULL, 0, false, jl_default_cgparams); |
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.
It is not legal now to pass NULL here as the second argument, as codegen (intentionally) lacks the ability now to do inference itself
This is a natural continuation of the stream of work begun in #53219. In particular, with #56555, it is important to be able to pass a CodeInstance, since the ABI may be overwritten and without knowing that, codegen will generated incorrect code for the CodeInstance.