Make various flags booleans #1266
Conversation
rinon
added
the
low priority
There was a problem hiding this comment.
LGTM except for a few little things. I also wanted to ask why #[repr(C)] is removed for some structs that now have bools but not others. Is that based on perf measurements?
I was also wondering if it may help perf to combine a bunch of these bool flag fields into one bitflags! field if many of them are all accessed in hot fns, as that should presumably reduce the amount of loads and possibility of those loads not being cached.
rinon
force-pushed
the
sjc/bool_flags
branch
2 times, most recently
from
158fae6 to
0923af0
Compare
| let prev_mask = (sby - 1 >> !seq_hdr.sb128 as c_int) * f.sb128w; | ||
| let mask_offset = (sby >> !seq_hdr.sb128 as c_int) * f.sb128w; |
There was a problem hiding this comment.
| let prev_mask = (sby - 1 >> !seq_hdr.sb128 as c_int) * f.sb128w; | |
| let mask_offset = (sby >> !seq_hdr.sb128 as c_int) * f.sb128w; | |
| let prev_mask = (sby - 1 >> !seq_hdr.sb128 as u8) * f.sb128w; | |
| let mask_offset = (sby >> !seq_hdr.sb128 as u8) * f.sb128w; |
| ss_ver = false; | ||
| ss_hor = ss_ver; |
There was a problem hiding this comment.
| ss_ver = false; | |
| ss_hor = ss_ver; | |
| ss_ver = true; | |
| ss_hor = true; |
There was a problem hiding this comment.
Can you run the argon tests?
| match profile { | ||
| Rav1dProfile::Main => { | ||
| layout = Rav1dPixelLayout::I420; | ||
| ss_ver = 1; | ||
| ss_ver = true; | ||
| ss_hor = ss_ver; |
There was a problem hiding this comment.
| ss_hor = ss_ver; | |
| ss_hor = true; |
| ss_ver = gb.get_bit() as u8; | ||
| ss_hor = gb.get_bit(); | ||
| if ss_hor { | ||
| ss_ver = gb.get_bit(); | ||
| } else { | ||
| // Default initialization. | ||
| ss_ver = Default::default(); |
There was a problem hiding this comment.
| ss_ver = Default::default(); | |
| ss_ver = false; |
| let present = | ||
| (seqhdr.film_grain_present && (show_frame || showable_frame) && gb.get_bit()) as u8; | ||
| let update; |
There was a problem hiding this comment.
present and update can both be bools, too.
| for _ in 0..8 { | ||
| gb.get_bits(seqhdr.order_hint_n_bits.into()); | ||
| } | ||
| } | ||
| frame_ref_short_signaling = (seqhdr.order_hint != 0 && gb.get_bit()) as u8; | ||
| frame_ref_short_signaling = (seqhdr.order_hint && gb.get_bit()) as u8; |
| @@ -1522,17 +1520,17 @@ fn decode_b( | |||
| } | |||
|
|
|||
| // delta-q/lf | |||
| let not_sb128 = (seq_hdr.sb128 == 0) as c_int; | |||
| let not_sb128 = !seq_hdr.sb128 as c_int; | |||
There was a problem hiding this comment.
| let not_sb128 = !seq_hdr.sb128 as c_int; | |
| let not_sb128 = !seq_hdr.sb128 as u8; |
| @@ -1982,7 +1983,7 @@ fn decode_b( | |||
| case.set_disjoint(&dir.seg_pred, seg_pred.into()); | |||
| case.set_disjoint(&dir.skip_mode, 0); | |||
| case.set_disjoint(&dir.intra, 1); | |||
| case.set_disjoint(&dir.skip, b.skip); | |||
| case.set_disjoint(&dir.skip, b.skip as u8); | |||
There was a problem hiding this comment.
dir.skip can be made bools.
| @@ -3137,9 +3134,9 @@ fn decode_b( | |||
| [by4 as usize, bx4 as usize], | |||
| |case, (dir, dir_index)| { | |||
| case.set_disjoint(&dir.seg_pred, seg_pred.into()); | |||
| case.set_disjoint(&dir.skip_mode, b.skip_mode); | |||
| case.set_disjoint(&dir.skip_mode, b.skip_mode as u8); | |||
There was a problem hiding this comment.
dir.skip_mode can be made bools.
| @@ -3137,9 +3134,9 @@ fn decode_b( | |||
| [by4 as usize, bx4 as usize], | |||
| |case, (dir, dir_index)| { | |||
| case.set_disjoint(&dir.seg_pred, seg_pred.into()); | |||
| case.set_disjoint(&dir.skip_mode, b.skip_mode); | |||
| case.set_disjoint(&dir.skip_mode, b.skip_mode as u8); | |||
| case.set_disjoint(&dir.intra, 0); | |||
There was a problem hiding this comment.
dir.intra can be made bools, too.
I did this while trying to improve performance. It doesn't seem like a large effect but it's also cleaner, so I figured I would upload the changes anyway.