Draft PR to do CI checking for backend rebase #436
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
noahgibbs
force-pushed
the
noah_backend_rebase
branch
3 times, most recently
from
c99ecd1
to
47292a2
Compare
* Split instructions if necessary * Add a reusable transform_insns function * Split out comments labels from transform_insns * Refactor alloc_regs to use transform_insns
* Initial setup for aarch64 * ADDS and SUBS * ADD and SUB for immediates * Revert moved code * Documentation * Rename Arm64* to A64* * Comments on shift types * Share sig_imm_size and unsig_imm_size
Currently we use macros to define the shape of each of the instruction building methods. This works while all of the instructions share the same fields, but is really hard to get working when they're an enum with different shapes. This is an incremental step toward a bigger refactor of changing the Insn from a struct to an enum.
When we're pushing instructions onto the assembler, we previously would iterate through the instruction's operands and then assign the output operand to it through the push_insn function. This is easy when all instructions have a vector of operands, but is much more difficult when the shape differs in an enum. This commit changes it so that we explicitly define the output operand for each instruction before it gets pushed onto the assembler. This has the added benefit of changing the definition of push_insn to no longer require a mutable instruction. This paves the way to make the out field on the instructions an Option<Opnd> instead which is going to more accurately reflect the behavior we're going to have once we switch the instructions over to an enum instead of a struct.
* Only check lowest bit for _Bool type The `test AL, AL` got lost during porting and we were generating `test RAX, RAX` instead. The upper bits of a `_Bool` return type is unspecified and we were failing `TestClass#test_singleton_class_should_has_own_namespace` due to interpreterting the return value incorrectly. * Enable test_class for test-all on x86_64
* Mutate in place for register allocation Currently we allocate a new instruction every time when we're doing register allocation by first splitting up the instruction into its component parts, mapping the operands and the output, and then pushing all of its parts onto the new assembler. Since we don't need the old instruction, we can mutate the existing one in place. While it's not that big of a win in and of itself, it matches much more closely to what we're going to have to do when we switch the instruction from being a struct to being an enum, because it's much easier for the instruction to modify itself since it knows its own shape than it is to push a new instruction that very closely matches. * Mutate in place for arm64 split When we're splitting instructions for the arm64 backend, we map all of the operands for a given instruction when it has an Opnd::Value. We can do this in place with the existing operand instead of allocating a new vector each time. This enables us to pattern match against the entire instruction instead of just the opcode, which is much closer to matching against an enum. * Match against entire instruction in arm64_emit Instead of matching against the opcode and then accessing all of the various fields on the instruction when emitting bytecode for arm64, we should instead match against the entire instruction. This makes it much closer to what's going to happen when we switch it over to being an enum. * Match against entire instruction in x86_64 backend When we're splitting or emitting code for x86_64, we should match against the entire instruction instead of matching against just the opcode. This gets us closer to matching against an enum instead of a struct. * Reuse instructions for arm64_split When we're splitting, the default behavior was previously to split up the instruction into its component parts and then reassemble them in a new instruction. Instead, we can reuse the existing instruction.
* Fix a bus error on regenerate_branch * Fix pad_size
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
Yet another case of `jit_mov_gc_ptr()` being yanked out during the transition to the new backend, causing a crash after object movement. The intresting wrinkle with this one is that not all callinfos are GC'ed objects, so the old code had an implicit assumption. https://github.com/ruby/ruby/blob/b0b9f7201acab05c2a3ad92c3043a1f01df3e17f/yjit/src/codegen.rs#L4087-L4095
* Operand iterators There are a couple of times when we're dealing with instructions that we need to iterate through their operands. At the moment this is relatively easy because there's an opnds field and we can work with it directly. When the instructions become enums, however, the shape of each variant will be different so we'll need an iterator to make sense of the shape. This commit introduces two new iterators that are created from an instruction. One iterates over references to each operand (for instances where they don't need to be mutable like updating live ranges) and one iterates over mutable references to each operand (for instances where you need to mutate them like loading values in arm64). Note that because iterators can't have generic items (i.e., be associated with lifetimes) the mutable iterator forces you to use the `while let Some` syntax as opposed to the for-loop like we did with instructions. This commit eliminates the last reference to insn.opnds, which is going to make it much easier to transition to an enum. * Consolidate output operand fetching Currently we always look at the .out field on instructions whenever we want to access the output operand. When the instructions become an enum, this is not going to be possible since the shape of the variants will be different. Instead, this commit introduces two functions on Insn: out_opnd() and out_opnd_mut(). These return an Option containing a reference to the output operand and a mutable reference to the output operand, respectively. This commit then uses those functions to replace all instances of accessing the output operand. For the most part this was straightforward; when we previously checked if it was Opnd::None we now check that it's None, when we assumed there was an output operand we now unwrap.
This should fix a version string test
* Remove references to explicit instruction parts Previously we would reference individual instruction fields manually. We can't do that with instructions that are enums, so this commit removes those references. As a side effect, we can remove the push_insn_parts() function from the assembler because we now explicitly push instruction structs every time. * Switch instructions to enum Instructions are now no longer a large struct with a bunch of optional fields. Instead they are an enum with individual shapes for the variants. In terms of size, the instruction struct was 120 bytes while the new instruction enum is 106 bytes. The bigger win however is that we're not allocating any vectors for instruction operands (except for CCall), which should help cut down on memory usage. Adding new instructions will be a little more complicated going forward, but every mission-critical function that needs to be touched will have an exhaustive match, so the compiler should guide any additions.
* When we're storing an immediate 0 value at a memory address, we can use STUR XZR, Xd instead of loading 0 into a register and then storing that register. * When we're moving 0 into an argument register, we can use MOV Xd, XZR instead of loading the value into a register first. * In the newarray instruction, we can skip looking at the stack at all if the number of values we're using is 0.
* Add --yjit-dump-disasm to dump every compiled code * Just use get_option * Carve out disasm_from_addr * Avoid push_str with format! * Share the logic through asm.compile * This seems to negatively impact the compilation speed
Add VMIL paper, update supported CPUs.
* Respect RUBY_TESTOPTS on test-all * Increase the Cirrus timeout * Increase the CSV test timeout
noahgibbs
force-pushed
the
noah_backend_rebase
branch
from
f70f192
to
370abe7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For issue #431, we'd like to see how CI tests are going.