-
Notifications
You must be signed in to change notification settings - Fork 22
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
Rav1dFrameContext_frame_thread::pal
: Make into an AlignedVec
#732
Conversation
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.
@rinon made a good point about the alignment; we should use Align64
. Also, the pal_sz
field can now be folded into pal.len()
, after adjusting by 16 * 16
.
3505492
to
b1ffbf9
Compare
47bf8ed
to
90fa006
Compare
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.
nit: I think you meant pal_sz
instead of pal_idx
in the commit message.
74f5904
to
5a4a50d
Compare
Additional changes have been made that need review.
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.
See also #744 for a discussion of how to approach this for all aligned allocs.
c36f010
to
2f0be62
Compare
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.
Taking a look at this again, I realized we don't actually need bounds on T
for safe transmutability, since we leave the u8
s uninitialized and only ever initialize things as T
s. I had a bunch of other comments, though, and I thought of a way to make it generic over the alignment.
d91cd0b
to
55dc06d
Compare
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.
It mostly looks very good now! Though I had a bunch of suggestions for some comments, and a few other little things.
Rav1dFrameContext_frame_thread::pal
: Make into a Vec
Rav1dFrameContext_frame_thread::pal
: Make into an aligned Vec
LGTM overall, minor nitpicks aside. |
55dc06d
to
c23b4ed
Compare
The original C logic was doing an aligned allocation, so we use the `Align64` wrapper type to enforce that same alignment.
c23b4ed
to
7129605
Compare
Rav1dFrameContext_frame_thread::pal
: Make into an aligned Vec
Rav1dFrameContext_frame_thread::pal
: Make into an AlignedVec
|
This also adds
AlignedVec64
to have theVec
aligned, since aVec<Align64<T>>
would add extra padding between each element since it would align each element, and we only want to align the wholeVec
/slice.