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 an optimizer bug in invoke with non-constant functions #26301

Merged
merged 1 commit into from
Mar 5, 2018

Conversation

JeffBezanson
Copy link
Member

In this case inlining was able to convert an invoke call to an :invoke Expr, but we had not inferred the target method instance, which led us to hit the fallback path in jl_invoke, which does not support this case yet.

@JeffBezanson JeffBezanson added bugfix This change fixes an existing bug compiler:inference Type inference labels Mar 2, 2018
In this case inlining was able to convert an `invoke` call to an
`:invoke` Expr, but we had not inferred the target method instance,
which led us to hit the fallback path in `jl_invoke`, which does not
support this case yet.
@JeffBezanson
Copy link
Member Author

cc @jrevels

@jrevels
Copy link
Member

jrevels commented Mar 3, 2018

Awesome, thanks!

This indeed fixes the MWE, but unfortunately doesn't seem to fix the real case; see JuliaLabs/Cassette.jl#33. I'll try and come up with another MWE at some point, but until then, there's example code in that PR to trigger the StackOverflowError if you're curious.

@JeffBezanson JeffBezanson merged commit c7da537 into master Mar 5, 2018
@JeffBezanson JeffBezanson deleted the jb/invokefix branch March 5, 2018 17:01
mbauman added a commit that referenced this pull request Mar 5, 2018
…luenonscalarindexedassignment

* origin/master: (28 commits)
  fix an optimizer bug in `invoke` with non-constant functions (#26301)
  lower top-level statements in such a way that the front-end knows (#26304)
  Make sure Sockets page has h1 header (#26315)
  fix doctests, and make them less prone to errors (#26275)
  FIx intro to manual chapter on types (#26312)
  Add a missing "that" (#26313)
  fix docstring for code_llvm (#26266)
  Remove the examples/ folder (#26153)
  download cert.pem from deps-getall, fixes #24903 (#25344)
  Slight update to doc string for at-enum to refer to instances (#26208)
  performance tweak in reverse(::String) (#26300)
  remove references to `TCPSocket` in Base docstrings (#26298)
  Deprecate adding integers to CartesianIndex (#26284)
  Deprecate conj(::Any), add real(::Missing) and imag(::Missing) (#26288)
  fix #26267, regression in `names` with `imported=false` (#26293)
  fix #25857, `typeinfo` problem in showing arrays with abstract tuple types (#26289)
  Add adjoint(::Missing) method (#25502)
  Use lowered form in logging macro (#26291)
  deprecate bin, oct, dec, hex, and base in favor of `string` and keyword args (#25804)
  deprecate `spawn(cmd)` to `run(cmd, wait=false)` (#26130)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug compiler:inference Type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants