Convert getinstancevariable to new backend IR #352

k0kubun · 2022-08-02T20:55:52Z

I merged Maxime's register allocator fix to @noahgibbs's branch and addressed another panic in Opnd::mem.

…stancevariable

yjit/src/backend/ir.rs

yjit/src/backend/tests.rs

yjit/src/codegen.rs

maximecb

Added a few comments. I think we should probably add a few getivar tests to test_yjit_new_backend.rb as well.

k0kubun · 2022-08-03T22:22:53Z

Addressed the comments, and fixed one thing that was probably a mistake c3b7828.

CSelE generates cmovne, and CSelNE generates cmove; the opposite of what IR says. Since we want to move Qnil when it's equal to Qundef, I believe it needs to be cmove (CSelNE, given how the IR is implemented).

noahgibbs · 2022-08-04T09:57:34Z

CSelE generates cmovne, and CSelNE generates cmove; the opposite of what IR says. Since we want to move Qnil when it's equal to Qundef, I believe it needs to be cmove (CSelNE, given how the IR is implemented).

I believe this is intentional. I had read CSelE as "Select if Equal", so you'd get the first item if the equal/zero flag is true, and the second one if not. Which turns into a MOV <firstitem> / CMOVNE <seconditem> on x86 to get the same effect.

yjit/src/codegen.rs

k0kubun · 2022-08-04T16:02:01Z

I think I addressed all the past reviews, including two ivar test paths in test_yjit_new_backend.rb. Since I ended up relying on this branch from #357 and some conflict resolution effort would be needed after squash merge, let me merge this one 🙇 YJIT CIs are already passing, and I can address any further feedback after merging it. Thank you for having a look.

* Convert getinstancevariable to new backend IR * Support mem-based mem * Use more into() * Add tests for getivar * Just load obj_opnd to a register * Apply another into() * Flip the nil-out condition * Fix duplicated counts of side_exit PR: #352

* Convert getinstancevariable to new backend IR * Support mem-based mem * Use more into() * Add tests for getivar * Just load obj_opnd to a register * Apply another into() * Flip the nil-out condition * Fix duplicated counts of side_exit

noahgibbs and others added 3 commits July 28, 2022 13:16

Convert getinstancevariable to new backend IR

d80ccb2

Merge remote-tracking branch 'origin/yjit_backend_ir' into noah_getin…

36898f9

…stancevariable

Support mem-based mem

0288571

k0kubun marked this pull request as ready for review August 2, 2022 21:04

k0kubun requested review from maximecb, XrXr and tenderlove as code owners August 2, 2022 21:04