Skip to content

fix an optimizer bug in invoke with non-constant functions#26301

Merged
JeffBezanson merged 1 commit intomasterfrom
jb/invokefix
Mar 5, 2018
Merged

fix an optimizer bug in invoke with non-constant functions#26301
JeffBezanson merged 1 commit intomasterfrom
jb/invokefix

Conversation

@JeffBezanson
Copy link
Copy Markdown
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
Copy Markdown
Member Author

cc @jrevels

@jrevels
Copy link
Copy Markdown
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