Skip to content

Commit

Permalink
servo: Merge #15992 - Rewrite PropertyDeclaration::id to help the opt…
Browse files Browse the repository at this point in the history
…imizer (from servo:id-table); r=bholley

If I’m reading the release-mode assembly correctly, before this change `PropertyDeclaration::id` is implemented with a computed jump:

```assembly
	lea	rcx, [rip + .LJTI117_0]
	movsxd	rax, dword ptr [rcx + 4*rax]
	add	rax, rcx
	jmp	rax
.LBB117_3:
	mov	dword ptr [rdi], 65536
	mov	rax, rdi
	ret
.LBB117_2:
	mov	dword ptr [rdi], 0
	mov	rax, rdi
	ret
.LBB117_4:
	mov	dword ptr [rdi], 131072
	mov	rax, rdi
	ret
.LBB117_6:
	mov	dword ptr [rdi], 262144
	mov	rax, rdi
	ret
.LBB117_7:
	mov	dword ptr [rdi], 327680
	mov	rax, rdi
	ret

; Four similar lines repeated for each of the few hundred variants...
```

With Rust 1.15 (currently used for geckolib) this doesn’t change significantly. In Nightly 1.17 however, the compiled code uses a lookup table, possibly thanks to rust-lang/rust#39456.

```assembly
	movq	(%rsi), %rax
	cmpq	$171, %rax
	jne	.LBB23_1
	addq	$8, %rsi
	movq	%rsi, 8(%rdi)
	movb	$1, %al
	jmp	.LBB23_3
.LBB23_1:
	xorq	$128, %rax
	leaq	.Lswitch.table.6(%rip), %rcx
	movb	(%rax,%rcx), %al
	movb	%al, 1(%rdi)
	xorl	%eax, %eax
.LBB23_3:
	movb	%al, (%rdi)
	movq	%rdi, %rax
	retq
```

Source-Repo: https://github.com/servo/servo
Source-Revision: 9e8e1a47241c6906b4f5777da0d04e3655982fae

UltraBlame original commit: af90f70b05df8c81f440b222728ae48c2e2bf5f8
  • Loading branch information
marco-c committed Oct 1, 2019
1 parent cab5a6e commit 6727511
Showing 1 changed file with 126 additions and 35 deletions.
161 changes: 126 additions & 35 deletions servo/components/style/properties/properties.mako.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8359,6 +8359,69 @@ match
*
self
{
PropertyDeclaration
:
:
Custom
(
ref
name
_
)
=
>
{
return
PropertyDeclarationId
:
:
Custom
(
name
)
}
PropertyDeclaration
:
:
CSSWideKeyword
(
id
_
)
|
PropertyDeclaration
:
:
WithVariables
(
id
_
)
=
>
{
return
PropertyDeclarationId
:
:
Longhand
(
id
)
}
_
=
>
{
}
}
let
longhand_id
=
match
*
self
{
%
for
property
Expand All @@ -8382,11 +8445,6 @@ camel_case
=
>
{
PropertyDeclarationId
:
:
Longhand
(
LonghandId
:
:
Expand All @@ -8395,7 +8453,6 @@ property
.
camel_case
}
)
}
%
endfor
Expand All @@ -8404,57 +8461,91 @@ PropertyDeclaration
:
CSSWideKeyword
(
id
_
)
=
>
PropertyDeclarationId
:
:
Longhand
(
id
.
.
)
|
PropertyDeclaration
:
:
WithVariables
(
id
_
)
=
>
PropertyDeclarationId
:
:
Longhand
(
id
.
.
)
|
PropertyDeclaration
:
:
Custom
(
ref
name
_
.
.
)
=
>
{
debug_assert
!
(
false
"
unreachable
"
)
;
/
/
This
value
is
never
used
but
having
an
expression
of
the
same
"
shape
"
/
/
as
for
other
variants
helps
the
optimizer
compile
this
match
expression
/
/
to
a
lookup
table
.
LonghandId
:
:
BackgroundColor
}
}
;
PropertyDeclarationId
:
:
Custom
Longhand
(
name
longhand_id
)
}
}
}
fn
with_variables_from_shorthand
(
Expand Down

0 comments on commit 6727511

Please sign in to comment.