-
-
Notifications
You must be signed in to change notification settings - Fork 320
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
[Breaking Change] Simplify Refcounting #7448
Conversation
Instead of -max_size to -1 for regular refcounts, use 1 to max_size. 0 still means constant refcount. The highest bit is used to signify atomic refcounting required. This does not turn on any sort of atomic refcounting.
I must be missing a refcount change somewhere. As such, nqueens is not mutating in place. Thus a huge regression. |
Hmm... The regression must be linux or x86_64 specific. I am not seeing a regression on my m1 mac. Will do more testing later. |
Benchmarking fixed. Even improves perf by a tiny bit. |
@@ -131,74 +127,7 @@ impl<'ctx> PointerToRefcount<'ctx> { | |||
.allocation_alignment_bytes(layout_interner) | |||
.max(env.target.ptr_width() as u32); | |||
|
|||
let context = env.context; |
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.
Very nice
|
@lukewilliamboswell should we merge this now? |
Signed-off-by: Brendan Hansknecht <[email protected]>
Instead of -max_size to -1 for regular refcounts, use 1 to max_size. 0 still means constant refcount.
The highest bit is used to signify atomic refcounting required. This does not turn on any sort of atomic refcounting.
WARNING: This is a breaking change for plaforms that interact with roc refcounts. The platforms will still initialize refcounts to
-max_size
which is now wrong.cc: @Anton-4 for thoughts on when to submit this.