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

Quality of life improvements for IR2OC branch #45103

Merged
merged 3 commits into from
Apr 29, 2022
Merged

Quality of life improvements for IR2OC branch #45103

merged 3 commits into from
Apr 29, 2022

Conversation

Keno
Copy link
Member

@Keno Keno commented Apr 27, 2022

Still tracking down some issues with this branch, but in the meantime, this makes code_llvm work and moves the constructor into base, so that users don't have to copy the internal details everywhere. Also makes it possible to specify the return type for the IRCode variant.

N.B.: PR is against jb/ircode2oc not master.

@Keno Keno requested review from JeffBezanson and aviatesk April 27, 2022 08:32
@Keno Keno force-pushed the kf/jb/ircode2oc branch from 5bbdb60 to e9bc3b2 Compare April 27, 2022 23:52
@@ -58,10 +58,12 @@ struct InliningState{S <: Union{EdgeTracker, Nothing}, MICache, I<:AbstractInter
interp::I
end

source_inferred(@nospecialize(src)) = ccall(:jl_ir_flag_inferred, Bool, (Any,), src)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_source_inferred? Also, the src is currently required to be CodeInfo or Vector{UInt8}. Being Any unspecialized value is not legal in jl_ir_flag_inferred.

Base automatically changed from jb/ircode2oc to master April 28, 2022 22:26
Keno added 3 commits April 28, 2022 22:36
They currently affect codegen, so they need to be preserved
until we decide to store the environment type somewhere else.
@Keno Keno force-pushed the kf/jb/ircode2oc branch from e9bc3b2 to 4206af5 Compare April 28, 2022 22:36
@Keno Keno merged commit 19a3ddd into master Apr 29, 2022
@Keno Keno deleted the kf/jb/ircode2oc branch April 29, 2022 04:06
end

function Core.OpaqueClosure(ir::IRCode, env...;
nargs::Int = length(ir.argtypes)-1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't nargs be a required argument? I think length(ir.argtypes)-1 doesn't necessarily correspond to the number of actual argument as it can include entries for arbitrary slots.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conceptually only the ir.argtypes that correspond to the arguments have any meaning. We sometimes leave the slottypes in there for convenience, but we should probably stop doing that.

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 this pull request may close these issues.

3 participants