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

Fix llvmcall overdubbing #139

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix llvmcall overdubbing #139

wants to merge 2 commits into from

Conversation

vchuravy
Copy link
Member

First attempt at fixing #138

This is still not quite sufficient and the tests fail with:

  Got exception outside of a @test
  LoadError: error compiling recurse: error statically evaluating llvm IR argument: access to invalid slot number
  Stacktrace:
   [1] overdub(::Cassette.Context{nametype(LLVMCallCtx),Nothing,Nothing,getfield(Cassette, Symbol("##PassType#365")),Nothing,Nothing}, ::Function, ::Float64) at /home/vchuravy/src/Cassette/test/misctests.jl:683
   [2] recurse(::Cassette.Context{nametype(LLVMCallCtx),Nothing,Nothing,getfield(Cassette, Symbol("##PassType#365")),Nothing,Nothing}, ::getfield(Main, Symbol("##111#112"))) at /home/vchuravy/src/Cassette/src/overdub.jl:695
   [3] top-level scope at none:0

Inspired by https://github.com/vchuravy/GPUifyLoops.jl/blob/760700e12f88c4bebb072d5f0453bed1d24692a3/src/context.jl#L59-L87

I also tried to solve this by using various function overloads, but no joy.

@vchuravy
Copy link
Member Author

cc: @oxinabox @NHDaly

x = code[x.id]
end
return x
end
Copy link
Contributor

@oxinabox oxinabox Oct 11, 2019

Choose a reason for hiding this comment

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

this is super cute

@NHDaly
Copy link
Contributor

NHDaly commented Oct 11, 2019

I think this makes sense. We talked about this in person, but just recording here that this of course means that llvmcalls won't get overdubbed, so users who do want to make modifications to them will need to write a complete Pass to do that, which seems like a reasonable solution.

@vchuravy
Copy link
Member Author

Okay I think I am done with this for now. I still want to add the general intrinsic op-out, but that is for the next major version.

@vchuravy vchuravy changed the title WIP: Fix llvmcall usage Fix llvmcall overdubbing Oct 12, 2019
@vchuravy
Copy link
Member Author

Except that the tagging variant is broken. Grrml

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.

None yet

3 participants