This is a majorly reworked version of the patch, based on the observations in comment 3. It reinstates the existing scheme of allowing creation of overlapping bundles when splitting, but only when splitting to implement spilling (split-for-spill). For ad-hoc splitting (around calls, loops, etc) exact splitting is still enforced. More test cases work now. On x86_64-Linux, there are 20 failures out of 2468 jit_tests for wasm, mostly in the JS support code. And overall 88 failures out of 9224 jit_tests. So large amounts of JS also work. The top level changes in the patch are (with many names q-prefixed to avoid collisions with old code): * new exact-split routine ::qsplitAt, for non-spill splitting, plus helpers * new split-for-spill routine ::qsplitForSpill * new split-across-call policy routine, ::qtrySplitAcrossCalls * dedicated splitting to deal with fixed stack refs (wasm multi-value returns), ::qtrySplitOffFixedStackRefs * major rewrite of ::trySplitAfterLastRegisterUse New and modified code is extensively commented. A top level block comment describing how the parts hang together is in preparation but is not present in this patch. The following are arguably failures in separation-of-concerns across the allocator interface. They cause additional complication in the allocator that might be better handled outside the allocator: * OsiPoint insns. MoveGroups may not be inserted between an OsiPoint and the insn that precedes it. It follows that splitting is not allowed there. This causes a bunch of special-casing throughout the spilling logic -- in effect "insn ; OsiPoin" must be handled as a single insn. * fixed stack refs for wasm multivalue returns. These appear to require a special splitting rule because they sidestep the standard bundle-allocation mechanism. I wonder if there's a cleaner way to implement this. As it stands it is still unknown whether the patch can serve as the basis for improvements to the allocator. That requires at a minimum: * passing all tests, also on x86_32 and arm64, so we have some assurance that the rework has no terminal design-level problems * getting it to work when spill-bundle creation is optionally disabled for ad-hoc splitting. Spill bundles cause excessive interference and hence copying/spilling, so are to be avoided where possible. This patch creates a spill bundle for all splits, as an interim measure. Debugging continues.
Bug 1758274 Comment 4 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
This is a majorly reworked version of the patch, based on the observations in comment 3. It reinstates the existing scheme of allowing creation of overlapping bundles when splitting, but only when splitting to implement spilling (split-for-spill). For ad-hoc splitting (around calls, loops, etc) exact splitting is still enforced. More test cases work now. On x86_64-Linux, there are 20 failures out of 2468 jit_tests for wasm, mostly in the JS support code. And overall 88 failures out of 9224 jit_tests. So large amounts of JS also work. The top level changes in the patch are (with many names q-prefixed to avoid collisions with old code): * new exact-split routine ::qsplitAt, for non-spill splitting, plus helpers * new split-for-spill routine ::qsplitForSpill * new split-across-call policy routine, ::qtrySplitAcrossCalls * dedicated splitting to deal with fixed stack refs (wasm multi-value returns), ::qtrySplitOffFixedStackRefs * major rewrite of ::trySplitAfterLastRegisterUse New and modified code is extensively commented. A top level block comment describing how the parts hang together is in preparation but is not present in this patch. The following are arguably failures in separation-of-concerns across the allocator interface. They cause additional complication in the allocator that might be better handled outside the allocator: * OsiPoint insns. MoveGroups may not be inserted between an OsiPoint and the insn that precedes it. It follows that splitting is not allowed there. This causes a bunch of special-casing throughout the splitting logic -- in effect "insn ; OsiPoint" must be handled as a single insn. * fixed stack refs for wasm multivalue returns. These appear to require a special splitting rule because they sidestep the standard bundle-allocation mechanism. I wonder if there's a cleaner way to implement this. As it stands it is still unknown whether the patch can serve as the basis for improvements to the allocator. That requires at a minimum: * passing all tests, also on x86_32 and arm64, so we have some assurance that the rework has no terminal design-level problems * getting it to work when spill-bundle creation is optionally disabled for ad-hoc splitting. Spill bundles cause excessive interference and hence copying/spilling, so are to be avoided where possible. This patch creates a spill bundle for all splits, as an interim measure. Debugging continues.