1375225 - Stylo: Redundant object copies in style::properties::PropertyDeclaration::parse_into

Status: NEW

(Core :: CSS Parsing and Computation, enhancement, P3)

Description

[(Away)]

I noticed some odd code sequences in style::properties::PropertyDeclaration::parse_into, for example:

00000001`80442f28 e8a334f5ff      call    xul!style::custom_properties::parse_non_custom_with_var (00000001`803963d0)
00000001`80442f2d 4c8ba5d8020000  mov     r12,qword ptr [rbp+2D8h]
00000001`80442f34 488b85e0020000  mov     rax,qword ptr [rbp+2E0h]
00000001`80442f3b 4c8bb5e8020000  mov     r14,qword ptr [rbp+2E8h]
00000001`80442f42 488b9df0020000  mov     rbx,qword ptr [rbp+2F0h]
00000001`80442f49 488bb5f8020000  mov     rsi,qword ptr [rbp+2F8h]
00000001`80442f50 4883bdd002000000 cmp     qword ptr [rbp+2D0h],0
00000001`80442f58 0f84d6000100    je      xul!style::properties::PropertyDeclaration::parse_into+0x62764 (00000001`80453034)
00000001`80442f5e 488b8d00030000  mov     rcx,qword ptr [rbp+300h]
00000001`80442f65 48c785d002000001000000 mov qword ptr [rbp+2D0h],1
00000001`80442f70 4c89a5d8020000  mov     qword ptr [rbp+2D8h],r12
00000001`80442f77 488985e0020000  mov     qword ptr [rbp+2E0h],rax
00000001`80442f7e 4c89b5e8020000  mov     qword ptr [rbp+2E8h],r14
00000001`80442f85 48899df0020000  mov     qword ptr [rbp+2F0h],rbx
00000001`80442f8c 4889b5f8020000  mov     qword ptr [rbp+2F8h],rsi
00000001`80442f93 48898d00030000  mov     qword ptr [rbp+300h],rcx

Where, assuming we don't take the je, this is more or less setting a bunch of memory equal to itself.

Another example:

00000001`80442fa6 0f104548        movups  xmm0,xmmword ptr [rbp+48h]
00000001`80442faa 0f104d58        movups  xmm1,xmmword ptr [rbp+58h]
00000001`80442fae 0f105568        movups  xmm2,xmmword ptr [rbp+68h]
00000001`80442fb2 0f299550040000  movaps  xmmword ptr [rbp+450h],xmm2
00000001`80442fb9 0f298d40040000  movaps  xmmword ptr [rbp+440h],xmm1
00000001`80442fc0 0f298530040000  movaps  xmmword ptr [rbp+430h],xmm0
00000001`80442fc7 0f288530040000  movaps  xmm0,xmmword ptr [rbp+430h]
00000001`80442fce 0f288d40040000  movaps  xmm1,xmmword ptr [rbp+440h]
00000001`80442fd5 0f289550040000  movaps  xmm2,xmmword ptr [rbp+450h]
00000001`80442fdc 0f1195f8020000  movups  xmmword ptr [rbp+2F8h],xmm2
00000001`80442fe3 0f118de8020000  movups  xmmword ptr [rbp+2E8h],xmm1
00000001`80442fea 0f1185d8020000  movups  xmmword ptr [rbp+2D8h],xmm0

This looks like it might be the result of something like:
 foo = bar;
 baz = foo;

And I suspect that we'll never use 'foo' again, in which case we could have just copied directly into baz.

But even if we did need to use 'foo' again, there's no need to load up the xmm registers a second time.

Comment 1

[Bobby Holley (:bholley)]

Michael, is there any plausible good reason for this?

Comment 2

[Michael Woerister]

It looks redundant to me. But I'm not very knowledgeable when it comes to low-level code generation.

Comment 3

[Michael Woerister]

I opened an issue [1] on the compiler repo in order to get it in front of people who know more about this topic.

[1] https://github.com/rust-lang/rust/issues/42853

Comment 4

[Firefox Bug Husbandry Bot]

https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals