Skip to content

Conversation

Keno
Copy link
Member

@Keno Keno commented Jun 8, 2024

PartialOpaque is a powerful mechanism to recover some amount of identity from opaque closure for inlining and other optimizations. However, there are two downsides:

  1. It causes additional inference work, which is unnecessary if you already know that you don't want the identity-based optimization.
  2. We (currently) disallow :new_opaque_closure in code returned from generated functions, because we cannot ensure that the identity of any resulting PartialOpaque will be stable. This somewhat defeats the purpose of having the opaque closure be, well, opaque.

This PR adds an additional argument to new_opaque_closure that decides whether or not inference is allowed to form PartialOpaque for this :new_opaque_closure. If not, it is also permitted to run during precompile.

@Keno Keno force-pushed the kf/ocnopartial branch from 7920e6d to e3033b4 Compare June 8, 2024 03:02
`PartialOpaque` is a powerful mechanism to recover some amount of
identity from opaque closure for inlining and other optimizations.
However, there are two downsides:
1. It causes additional inference work, which is unnecessary if you
   already know that you don't want the identity-based optimization.
2. We (currently) disallow :new_opaque_closure in code returned from
   generated functions, because we cannot ensure that the identity of
   any resulting PartialOpaque will be stable. This somewhat defeats the
   purpose of having the opaque closure be, well, opaque.

This PR adds an additional argument to `new_opaque_closure` that decides
whether or not inference is allowed to form `PartialOpaque` for this
`:new_opaque_closure`. If not, it is also permitted to run during
precompile.
@Keno Keno force-pushed the kf/ocnopartial branch from e3033b4 to ab444da Compare June 8, 2024 03:06
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

SGTM. I didn't look at everything, but couldn't think of any reason it would be problematic either.

@Keno Keno merged commit 1c59231 into master Jun 9, 2024
@Keno Keno deleted the kf/ocnopartial branch June 9, 2024 05:06
@topolarity topolarity mentioned this pull request Jul 13, 2024
3 tasks
serenity4 added a commit to serenity4/Diffractor.jl that referenced this pull request Feb 21, 2025
serenity4 added a commit to serenity4/Diffractor.jl that referenced this pull request Feb 21, 2025
Keno pushed a commit to JuliaDiff/Diffractor.jl that referenced this pull request Mar 3, 2025
* Adapt to JuliaLang/julia#56509

* Adapt to JuliaLang/julia#54734

* Use StmtRange explicitly

* Adapt to JuliaLang/julia#57230

* Reuse Cthulhu code structure for Compiler cache/finish overrides

* Adapt to JuliaLang/julia#57475

* Adapt to JuliaLang/julia#55976

* Adapt to JuliaLang/julia#54734

* Use CC instead of .Compiler

* Implement ir.argtypes[1] fix from JuliaLang/julia#54458

* Comment out failing tests

To highlight which are broken, should probably be fixed before merging

* Treat `getproperty(::Module, ::Symbol)` like GlobalRefs

* Uncomment passing tests, explicitly mark others as broken

* Evaluate GlobalRef only if binding is defined

* Use `rrule` for getproperty(::Module, ::Symbol)

* Bump compat bound for StructArrays

* Raise compat bound for Cthulhu

* Revert `isconst` change now that it is fixed

* Adapt to `finishinfer!` signature change

---------

Co-authored-by: Cédric Belmant <[email protected]>
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.

2 participants