1509176 - wr_dp_pop_stacking_context could have better code

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P3)

Description

[Jeff Muizelaar [:jrmuizel]]

pub extern "C" fn wr_dp_pop_stacking_context(state: &mut WrState,
                                             is_reference_frame: bool) {
    debug_assert!(unsafe { !is_in_render_thread() });
    state.frame_builder.dl_builder.pop_stacking_context();
    if is_reference_frame {
        state.frame_builder.dl_builder.pop_clip_id();
        state.frame_builder.dl_builder.pop_reference_frame();
    }
}

translates to:
+0x00	pushq               %rbp
+0x01	movq                %rsp, %rbp
+0x04	pushq               %r14
+0x06	pushq               %rbx
+0x07	subq                $576, %rsp
+0x0e	xorps               %xmm0, %xmm0
+0x11	movaps              %xmm0, -96(%rbp)
+0x15	movq                $0, -104(%rbp)
+0x1d	movq                $0, -112(%rbp)
+0x25	movq                40(%rdi), %rax
+0x29	testq               %rax, %rax
+0x2c	je                  "wr_dp_pop_stacking_context+0x2fc"
+0x32	movl                %esi, %r14d
+0x35	movq                %rdi, %rbx
+0x38	movq                24(%rdi), %rcx
+0x3c	leaq                (%rax,%rax,2), %rax
+0x40	shlq                $4, %rax
+0x44	movq                -8(%rcx,%rax), %rdx
+0x49	movq                %rdx, -32(%rbp)
+0x4d	movq                -16(%rcx,%rax), %rdx
+0x52	movq                %rdx, -40(%rbp)
+0x56	movq                -24(%rcx,%rax), %rdx
+0x5b	movq                %rdx, -48(%rbp)
+0x5f	movq                -32(%rcx,%rax), %rdx
+0x64	movq                %rdx, -56(%rbp)
+0x68	movq                -48(%rcx,%rax), %rdx
+0x6d	movq                -40(%rcx,%rax), %rax
+0x72	movq                %rax, -64(%rbp)
+0x76	movq                %rdx, -72(%rbp)
+0x7a	movq                $16, -416(%rbp)
+0x85	leaq                -408(%rbp), %rdi
+0x8c	leaq                -584(%rbp), %rsi
+0x93	movl                $168, %edx
+0x98	callq               "DYLD-STUB$$memcpy"
+0x9d	movq                -72(%rbp), %rax
+0xa1	movq                -64(%rbp), %rcx
+0xa5	movq                %rax, -240(%rbp)
+0xac	movq                %rcx, -232(%rbp)
+0xb3	movq                -56(%rbp), %rax
+0xb7	movq                %rax, -224(%rbp)
+0xbe	movq                -48(%rbp), %rax
+0xc2	movq                %rax, -216(%rbp)
+0xc9	movq                -40(%rbp), %rax
+0xcd	movq                %rax, -208(%rbp)
+0xd4	movq                -32(%rbp), %rax
+0xd8	movq                %rax, -200(%rbp)
+0xdf	movq                $0, -192(%rbp)
+0xea	movq                -128(%rbp), %rax
+0xee	movq                -120(%rbp), %rcx
+0xf2	movq                %rax, -184(%rbp)
+0xf9	movq                %rcx, -176(%rbp)
+0x100	movq                -112(%rbp), %rax
+0x104	movq                %rax, -168(%rbp)
+0x10b	movq                -104(%rbp), %rax
+0x10f	movq                %rax, -160(%rbp)
+0x116	movq                -96(%rbp), %rax
+0x11a	movq                -88(%rbp), %rcx
+0x11e	movq                %rcx, -144(%rbp)
+0x125	movq                %rax, -152(%rbp)
+0x12c	movb                $1, -136(%rbp)
+0x133	movl                -23(%rbp), %eax
+0x136	movl                %eax, -135(%rbp)
+0x13c	movzwl              -19(%rbp), %eax
+0x140	movw                %ax, -131(%rbp)
+0x147	movb                -17(%rbp), %al
+0x14a	movb                %al, -129(%rbp)
+0x150	leaq                -416(%rbp), %rsi
+0x157	movq                %rbx, %rdi
+0x15a	callq               "webrender_api::display_list::serialize_fast::h76b5d57b2d8432a7"
+0x15f	testb               %r14b, %r14b
+0x162	je                  "wr_dp_pop_stacking_context+0x2f0"
+0x168	movq                40(%rbx), %rax
+0x16c	testq               %rax, %rax
+0x16f	je                  "wr_dp_pop_stacking_context+0x19d"
+0x171	decq                %rax
+0x174	movq                %rax, 40(%rbx)
+0x178	cmpq                $1, 88(%rbx)
+0x17d	jne                 "wr_dp_pop_stacking_context+0x1a6"
+0x17f	    cmpq                104(%rbx), %rax
+0x183	    jae                 "wr_dp_pop_stacking_context+0x1a6"
+0x185	    leaq                92296172(%rip), %rdi
+0x18c	    leaq                112326277(%rip), %rdx
+0x193	    movl                $69, %esi
+0x198	    callq               "std::panicking::begin_panic::ha3be37a62fbfd382"
+0x19d	    xorl                %eax, %eax
+0x19f	    cmpq                $1, 88(%rbx)
+0x1a4	    je                  "wr_dp_pop_stacking_context+0x17f"
+0x1a6	testq               %rax, %rax
+0x1a9	je                  "wr_dp_pop_stacking_context+0x308"
+0x1af	leaq                -112(%rbp), %rcx
+0x1b3	xorps               %xmm0, %xmm0
+0x1b6	movaps              %xmm0, -96(%rbp)
+0x1ba	movq                $0, 8(%rcx)
+0x1c2	movq                $0, (%rcx)
+0x1c9	movq                24(%rbx), %rcx
+0x1cd	leaq                (%rax,%rax,2), %rax
+0x1d1	shlq                $4, %rax
+0x1d5	movq                -8(%rcx,%rax), %rdx
+0x1da	movq                %rdx, -32(%rbp)
+0x1de	movq                -16(%rcx,%rax), %rdx
+0x1e3	movq                %rdx, -40(%rbp)
+0x1e7	movq                -24(%rcx,%rax), %rdx
+0x1ec	movq                %rdx, -48(%rbp)
+0x1f0	movq                -32(%rcx,%rax), %rdx
+0x1f5	movq                %rdx, -56(%rbp)
+0x1f9	movq                -48(%rcx,%rax), %rdx
+0x1fe	movq                -40(%rcx,%rax), %rax
+0x203	movq                %rax, -64(%rbp)
+0x207	movq                %rdx, -72(%rbp)
+0x20b	movq                $18, -416(%rbp)
+0x216	leaq                -408(%rbp), %rdi
+0x21d	leaq                -584(%rbp), %rsi
+0x224	movl                $168, %edx
+0x229	callq               "DYLD-STUB$$memcpy"
+0x22e	movq                -72(%rbp), %rax
+0x232	movq                -64(%rbp), %rcx
+0x236	movq                %rax, -240(%rbp)
+0x23d	movq                %rcx, -232(%rbp)
+0x244	movq                -56(%rbp), %rax
+0x248	movq                %rax, -224(%rbp)
+0x24f	movq                -48(%rbp), %rax
+0x253	movq                %rax, -216(%rbp)
+0x25a	movq                -40(%rbp), %rax
+0x25e	movq                %rax, -208(%rbp)
+0x265	movq                -32(%rbp), %rax
+0x269	movq                %rax, -200(%rbp)
+0x270	movq                $0, -192(%rbp)
+0x27b	movq                -128(%rbp), %rax
+0x27f	movq                -120(%rbp), %rcx
+0x283	movq                %rax, -184(%rbp)
+0x28a	movq                %rcx, -176(%rbp)
+0x291	movq                -112(%rbp), %rax
+0x295	movq                %rax, -168(%rbp)
+0x29c	movq                -104(%rbp), %rax
+0x2a0	movq                %rax, -160(%rbp)
+0x2a7	movq                -96(%rbp), %rax
+0x2ab	movq                -88(%rbp), %rcx
+0x2af	movq                %rcx, -144(%rbp)
+0x2b6	movq                %rax, -152(%rbp)
+0x2bd	movb                $1, -136(%rbp)
+0x2c4	movb                -17(%rbp), %al
+0x2c7	movb                %al, -129(%rbp)
+0x2cd	movzwl              -19(%rbp), %eax
+0x2d1	movw                %ax, -131(%rbp)
+0x2d8	movl                -23(%rbp), %eax
+0x2db	movl                %eax, -135(%rbp)
+0x2e1	leaq                -416(%rbp), %rsi
+0x2e8	movq                %rbx, %rdi
+0x2eb	callq               "webrender_api::display_list::serialize_fast::h76b5d57b2d8432a7"
+0x2f0	addq                $576, %rsp
+0x2f7	popq                %rbx
+0x2f8	popq                %r14
+0x2fa	popq                %rbp
+0x2fb	retq
+0x2fc	leaq                112434165(%rip), %rdi
+0x303	callq               "core::panicking::panic::h3bfbdd68e26dba7d"
+0x308	leaq                92295854(%rip), %rdi
+0x30f	leaq                112325914(%rip), %rdx
+0x316	movl                $45, %esi
+0x31b	callq               "std::panicking::begin_panic::ha3be37a62fbfd382"

As can be seen most of the code is just moving data around. It should be possible to do better.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Here's a reduced test case showing the problem:

use std::mem;

#[inline(never)]
pub fn do_dance(clip: Option<&ClipAndScrollInfo2>) {
    do_dig(SpecificDisplayItem2::PopStackingContext, clip);
}

#[derive(Debug, Copy, Clone)]
pub struct LayoutPrimitiveInfo2 {
    pub rect: [f32; 4],
    pub clip_rect: [f32; 4],
    pub is_backface_visible: bool,
    pub tag: Option<(u64, u16)>,
}

impl LayoutPrimitiveInfo2 {
    fn new() -> Self {
        Self {
            rect: [0.; 4],
            clip_rect: [0.; 4],
            is_backface_visible: false,
            tag: None,
        }
    }
}

#[derive(Debug, Copy, Clone)]
pub struct ClipAndScrollInfo2([f64; 3]);

#[derive(Debug)]
pub enum SpecificDisplayItem2 {
    PopStackingContext,
    Other([f64; 22]),
}

#[derive(Debug)]
struct DI {
    item: SpecificDisplayItem2,
    clip_and_scroll: ClipAndScrollInfo2,
    info: LayoutPrimitiveInfo2,
}

pub fn do_dig(item: SpecificDisplayItem2, clip: Option<&ClipAndScrollInfo2>) {
    let clip = clip.unwrap();
    let info = LayoutPrimitiveInfo2::new();
    do_item(&DI {
        item,
        clip_and_scroll: *clip,
        info,
    });
}

#[inline(never)]
fn do_item(di: &DI) {
    println!("defo {:?}", di)
}

fn main() {
    let clip: ClipAndScrollInfo2 = unsafe { mem::uninitialized() };
    do_dance(Some(&clip));
}

Comment 2

[Jeff Muizelaar [:jrmuizel]]

I filed: https://github.com/rust-lang/rust/issues/56172 about this issue.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

And posted a work around here: https://github.com/servo/webrender/pull/3341

Comment 4

[Jeff Muizelaar [:jrmuizel]]

This should be better now anyways.