Skip to content

Commit

Permalink
Auto merge of #15992 - servo:id-table, r=bholley
Browse files Browse the repository at this point in the history
Rewrite PropertyDeclaration::id to help the optimizer.

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
```

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15992)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Mar 17, 2017
2 parents 99f5edb + 9ff0153 commit 9e8e1a4
Showing 1 changed file with 21 additions and 6 deletions.
27 changes: 21 additions & 6 deletions components/style/properties/properties.mako.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1094,17 +1094,32 @@ impl PropertyDeclaration {
/// Given a property declaration, return the property declaration id.
pub fn id(&self) -> PropertyDeclarationId {
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 in data.longhands:
PropertyDeclaration::${property.camel_case}(..) => {
PropertyDeclarationId::Longhand(LonghandId::${property.camel_case})
LonghandId::${property.camel_case}
}
% endfor
PropertyDeclaration::CSSWideKeyword(id, _) => PropertyDeclarationId::Longhand(id),
PropertyDeclaration::WithVariables(id, _) => PropertyDeclarationId::Longhand(id),
PropertyDeclaration::Custom(ref name, _) => {
PropertyDeclarationId::Custom(name)
PropertyDeclaration::CSSWideKeyword(..) |
PropertyDeclaration::WithVariables(..) |
PropertyDeclaration::Custom(..) => {
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::Longhand(longhand_id)
}

fn with_variables_from_shorthand(&self, shorthand: ShorthandId) -> Option< &str> {
Expand Down

0 comments on commit 9e8e1a4

Please sign in to comment.