Open Bug 1375225 Opened 8 years ago Updated 2 years ago

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

Categories

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

enhancement

Tracking

()

Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- ?

People

(Reporter: away, Unassigned)

References

Details

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.
Michael, is there any plausible good reason for this?
Flags: needinfo?(mwoerister)
It looks redundant to me. But I'm not very knowledgeable when it comes to low-level code generation.
Flags: needinfo?(mwoerister)
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
Priority: -- → P3
Summary: Redundant object copies in style::properties::PropertyDeclaration::parse_into → Stylo: Redundant object copies in style::properties::PropertyDeclaration::parse_into
Priority: P3 → --
Priority: -- → P4
Priority: P4 → P3
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.