Skip to content

YJIT: A64: Use ADDS/SUBS/CMP (immediate) when possible#10402

Merged
maximecb merged 3 commits intoruby:masterfrom
XrXr:yjit-a64-add-one
Apr 2, 2024
Merged

YJIT: A64: Use ADDS/SUBS/CMP (immediate) when possible#10402
maximecb merged 3 commits intoruby:masterfrom
XrXr:yjit-a64-add-one

Conversation

@XrXr
Copy link
Member

@XrXr XrXr commented Mar 28, 2024

We were loading 1 into a register and then doing ADDS/SUBS previously.
That was particularly bad since those come up in fixnum operations.

   # integer left shift with rhs=1
-  mov x11, #1
-  subs x11, x1, x11
+  subs x11, x1, #1
   lsl x12, x11, #1
   asr x13, x12, #1
   cmp x13, x11
-  b.ne #0x106ab60f8
-  mov x11, #1
-  adds x12, x12, x11
+  b.ne #0x10903a0f8
+  adds x12, x12, #1
   mov x1, x12

@XrXr XrXr requested a review from a team as a code owner March 28, 2024 21:48
@XrXr XrXr marked this pull request as draft March 28, 2024 21:51
@launchable-app

This comment has been minimized.

We were loading 1 into a register and then doing ADDS/SUBS previously.
That was particularly bad since those come up in fixnum operations.

```diff
   # integer left shift with rhs=1
-  mov x11, #1
-  subs x11, x1, x11
+  subs x11, x1, #1
   lsl x12, x11, #1
   asr x13, x12, #1
   cmp x13, x11
-  b.ne #0x106ab60f8
-  mov x11, #1
-  adds x12, x12, x11
+  b.ne #0x10903a0f8
+  adds x12, x12, #1
   mov x1, x12
```

Note that it's fine to cast between i64 and u64 since the bit pattern is
preserved, and the add/sub themselves don't care about the signedness of
the operands.

CMP is just another mnemonic for SUBS.
Copy link
Contributor

@kddnewton kddnewton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense!

Copy link
Contributor

@kddnewton kddnewton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to make sure every instruction it's passed to will handle this signature now.

@XrXr XrXr force-pushed the yjit-a64-add-one branch from fcba55c to 11e2f63 Compare March 28, 2024 22:17
@XrXr XrXr changed the title YJIT: A64: Use ADDS (immediate) when possible YJIT: A64: Use ADDS/SUBS/CMP (immediate) when possible Mar 28, 2024
There is in fact no MUL on A64 that takes an immediate, so this
instruction was using the wrong split method. No current usages of this
form in YJIT.
@XrXr XrXr marked this pull request as ready for review March 28, 2024 22:33
@maximecb
Copy link
Contributor

maximecb commented Apr 2, 2024

Good find. There were some failing tests that don't seem related to this PR. Updating the branch to run the tests again.

@maximecb maximecb merged commit 3c4de94 into ruby:master Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants