-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
ImmutableArrays #42465
Closed
Closed
ImmutableArrays #42465
Changes from 24 commits
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
fcbafaa
Implement ImmutableArray
Keno 233bb0c
Implement maybecopy in BoundsError, start optimization, refactor memo…
424232a
Begin EscapeAnalysis.jl port
e031da8
Update memory_opt! for usage with EscapeState, begin port of EA.jl in…
c2ca5c5
Merge branch 'master' into HEAD
aviatesk 49f9337
fixup definitions of array primitives
aviatesk b02e7c0
fixup `@noinline` annotations within `BoundsError`
aviatesk 2a52e3d
disable EA global caching to succeed in sysimg building
aviatesk bd0bdb7
define EA in a separate module
aviatesk 51120cf
Merge branch 'master' into kf/immutablearray
aviatesk 4b33fc7
add simple `all`/`any` definitions to make `::BitSet ⊆ ::BitSet` work…
aviatesk cb4b1b8
comment dead code for now
aviatesk f59d58b
Merge branch 'master' into kf/immutablearray
aviatesk bca3078
update to latest EA
aviatesk 2c466e1
update to latest EA
aviatesk 235ab84
Restrict arrayfreeze/thaw arg # in base/compiler/tfuncs.jl
ianatol 0e5833b
Restrict nargs of arrayfreeze/thaw to 1 in builtins.c
4a57838
Merge branch 'master' into kf/immutablearray
aviatesk de437d4
update EA and get array supports
aviatesk a34166c
fixup `memory_opt!`
aviatesk 1b1babf
update EA and handle conflicting field information correctly
aviatesk eee43bd
minor optimization for `memory_opt!`
aviatesk 9b8233a
define tfuncs for `ImmutableArray` primitives
aviatesk 6fad97d
Correct maybecopy and begin implementing its optimization. Also some …
e1a1dc5
Merge branch 'master' into kf/immutablearray
aviatesk dd77189
update to latest EA
aviatesk 39c925b
fixup `memory_opt!`
aviatesk fbc2c20
update new array tfuncs to accout for `ImmutableArray`
aviatesk 5d93701
simplify `memory_opt!`
aviatesk 19d5d20
Merge branch 'master' into kf/immutablearray
aviatesk b16190e
Merge branch 'master' into kf/immutablearray
aviatesk 697b6f1
simplify
aviatesk 0403a04
Merge branch 'master' into kf/immutablearray
aviatesk 8507643
improve test implementations
aviatesk 77eec88
Fixup unescaped1_1 test
c958ac9
Cleanup memory_opt, add some tests
c718fb9
avoid unintended name leak, better naming
aviatesk c4c7acf
Add some basic tests and move non-compiler tests
2438fd4
Merge branch 'master' into kf/immutablearray
aviatesk 0d7d81a
add test cases https://github.com/aviatesk/EscapeAnalysis.jl/pull/72 …
aviatesk 765f59c
Merge branch 'master' into kf/immutablearray
aviatesk 150c490
update EA
aviatesk eaf5005
no broken tests now
aviatesk 1dcfa1b
Merge branch 'master' into kf/immutablearray
aviatesk eb519c8
Merge branch 'master' into kf/immutablearray
aviatesk 8110fad
update EA
aviatesk 7b3c099
export `ImmutableArray`, define `ImmutableVector` alias
aviatesk d494cf2
use `IndexLinear` for immutable array
aviatesk de410f1
enable `test/immutablearray` testset
aviatesk d3c21bd
Merge branch 'master' into kf/immutablearray
aviatesk 283c36b
fix tests
aviatesk eb221f6
Merge branch 'master' into kf/immutablearray
aviatesk 10c161f
Merge branch 'master' into kf/immutablearray
aviatesk 2560b73
update EA
aviatesk fe3717a
Merge branch 'master' into kf/immutablearray
aviatesk bbe8b3d
Merge branch 'master' into kf/immutablearray
aviatesk bca548f
Remove maybecopy code
df8bccc
Add tests
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we extend this beyond array? This is very much a problem for
MArray
and would greatly improve it's usability on the GPU.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can enforce
copy
implementation for anything implementinggetindex
.But note that all the purpose of this cop? is that it allows EA to skip possible escapes through
arrayref
and thus this optimization atm is very specific toArray
s. This could also be generalized if we implement some specific interface which tells EA that user type object is always copied uponBoundsError
though.Having said that, I'm not really fan of changing the program semantics this way. Why not always making
copy
of it?BoundsError
doesn't need to be high-performing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also making the default behavior of
maybecopy
no-op seems bad.. the default should be copy and we optimize it as no-op when the array is already escaped somewhere else.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think the plan is eventually to have similar functionality as ImmutableArrays for other types, but yeah Shuhei is right that this initial PR is very Array-centric.
As for the non-semantic copy, I agree that the default should be copy - I misremembered that this was the approach we were taking (possibly because of some old incorrect test I had written). I’ll correct it shortly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No hurry from my side ;)
We can also talk with other core devs again to discuss if we can make copies by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: #43738 is our new solution to this