1507820 - Cranelift: pin HeapReg and maintain it

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement, P2)

Description

[Benjamin Bouvier [:bbouvier] (inactive)]

On x64, (ra)baldr excludes r15 from register allocation, and uses it as a pinned register for HeapReg. Cranelift isn't aware of this, and will use r15 for any purpose. There can be bad interactions during tiering because of this: say a main() function is in baseline code, which calls into cranelift code; baseline assumes the callee has preserved r15 and will try to reuse it.

It would be an interesting optimization to have Cranelift use the heap register for heap loads/stores. But in the first place, Cranelift needs to preserve the r15 register.

Comment 1

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: temporary-fix.patch

Here's a temporary quick fix. I've tried just pushing/popping HeapReg in the right places, but it didn't work for some reason.

With this and other changes from Cranelift 0.24, I could run the demo on webassembly.org \o/

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: heapbase.patch

Works with https://github.com/CraneStation/cranelift/pull/624

Comment 3

[Lars T Hansen [:lth]]

Note, at the moment the only pinned register (in the Ion sense) is HeapReg, which we use on all platforms except x86-32. To load the HeapReg we must first load the TLS, so there's a chained load here if the TLS is not already in a register.

(Of course, the TLS is also a pinned register at the ABI boundary, it is in R14 on x64. But Cranelift handles this as a "special" argument at the moment.)

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: wip.patch

Still wip-y; to be used with https://github.com/bnjbvr/cranelift/tree/fixed-heap-ssa

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: wip.patch

New wip patch, passes all tests, to run with a local Cranelift with https://github.com/bnjbvr/cranelift/tree/fixed-ssa-heapreg-wip .

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: wip.patch

Comment 7

[Benjamin Bouvier [:bbouvier] (inactive)]

Latest results on wasm_bench:

# mode=cranelift+cranelift, runs=5, problem size=default
# Lower score is better
benchmark      before after  speedup
box2d		894	850	1.052	[888, 889, 894, 894, 897]	[847, 849, 850, 853, 882]
bullet		1211	1132	1.07	[1210, 1211, 1211, 1214, 1215]	[1126, 1131, 1132, 1132, 1133]
conditionals	345	349	0.989	[336, 337, 345, 346, 347]	[346, 348, 349, 357, 358]
copy		691	690	1.001	[690, 690, 691, 693, 694]	[687, 688, 690, 693, 693]
corrections	931	930	1.001	[928, 929, 931, 937, 948]	[929, 930, 930, 930, 931]
fannkuch	2468	2392	1.032	[2433, 2460, 2468, 2468, 2539]	[2375, 2375, 2392, 2397, 2404]
fasta		811	789	1.028	[809, 811, 811, 813, 813]	[786, 788, 789, 791, 794]
fib		595	518	1.149	[590, 594, 595, 595, 603]	[517, 517, 518, 520, 520]
ifs		192	161	1.193	[183, 184, 192, 194, 194]	[159, 160, 161, 173, 175]
binarytrees	3996	3674	1.088	[3976, 3992, 3996, 4002, 4037]	[3668, 3671, 3674, 3677, 3683]
memops		1483	1421	1.044	[1478, 1479, 1483, 1486, 1494]	[1412, 1421, 1421, 1422, 1424]
primes		990	1012	0.978	[989, 989, 990, 991, 1000]	[1001, 1008, 1012, 1014, 1014]
raybench	1920	1877	1.023	[1915, 1918, 1920, 1920, 1939]	[1871, 1877, 1877, 1886, 1942]
rust-fannkuch	2392	2258	1.059	[2387, 2388, 2392, 2393, 2406]	[2249, 2253, 2258, 2264, 2281]
skinning	683	680	1.004	[680, 681, 683, 683, 691]	[678, 679, 680, 681, 687]
zlib		1271	1237	1.027	[1261, 1266, 1271, 1271, 1274]	[1233, 1234, 1237, 1238, 1249]

Comment 8

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1507820: Pin HeapReg in Cranelift and use it as the heap base; r=lth

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cbe50166fcf0
Pin HeapReg in Cranelift and use it as the heap base; r=lth

Comment 10

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/cbe50166fcf0