Stream: git-wasmtime

Topic: wasmtime / Issue #2158 x64 small integer fixes

Wasmtime GitHub notifications bot (Aug 22 2020 at 09:33):

github-actions[bot] commented on Issue #2158:

Subscribe to Label Action

cc @bnjbvr

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:machinst", "cranelift:area:x64"

Thus the following users have been cc'd because of the following labels:

• bnjbvr: cranelift

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Aug 22 2020 at 11:36):

julian-seward1 commented on Issue #2158:

I am not sure how to implement sshr and ushr for small integers.

Sign/zero extend the values out to 32 bits and shift that right instead.

Wasmtime GitHub notifications bot (Aug 22 2020 at 14:30):

bjorn3 commented on Issue #2158:

Thanks, done. In this instruction family only rotl and rotr are now missing for small integers. Those are very rare when using cg_clif though. I think the rustc test suite has like 4 occurrences and other than that I haven't found it in the wild a single time.

Wasmtime GitHub notifications bot (Aug 22 2020 at 14:45):

julian-seward1 commented on Issue #2158:

On x86/x64, the rol and ror instructions do rotates. They are identical in "shape" to shl, shr and sar so you could extend that sub-group easily enough. Dealing with 8- and 16-bit rotates might be tricky though, since the "widen first" trick that works for shr/sar won't work.

Rotates sometimes do get used in hash computations (new_hash = (old_hash rotated_left 13) ^ next_byte_to_hash) kind of thing. One reason you may not see them coming "down the pipe" is that if the incoming IR doesn't have a way to express them directly, they may have been translated into two shifts and an OR, eg: rotate_left(x, N) = (x << N) | (x >>u (Word_Size - N)).

Wasmtime GitHub notifications bot (Aug 22 2020 at 14:49):

bjorn3 commented on Issue #2158:

One reason you may not see them coming "down the pipe" is that if the incoming IR doesn't have a way to express them directly

Rust does have a rotate_left and rotate_right intrinsic. I have seen cg_clif panic on the rustc test suite when using these, but they seem to be very rarely used on small integers.

Wasmtime GitHub notifications bot (Aug 22 2020 at 15:04):

julian-seward1 commented on Issue #2158:

Ah, I see x64::Inst::Shift_R can already handle rotates, but only 64- and 32-bit. Maybe the simplest fix is to extend it to take a size field (a la Mov_R_M just above) so it can be used for 64, 32, 16 and 8 bit things. The 16 bit encoding is the same as the 32 bit encoding with a leading 66 and the 8 bit encoding is the same as the 32 bit encoding with the primary opcode byte decreased by 1 (it looks like).

Wasmtime GitHub notifications bot (Aug 23 2020 at 08:51):

bjorn3 commented on Issue #2158:

Done

Wasmtime GitHub notifications bot (Aug 24 2020 at 09:57):

bnjbvr commented on Issue #2158:

Great work! I'll take a look.

Wasmtime GitHub notifications bot (Sep 08 2020 at 11:28):

bjorn3 commented on Issue #2158:

Rebased and fixed review comments.

Wasmtime GitHub notifications bot (Sep 08 2020 at 13:02):

bjorn3 commented on Issue #2158:

Thanks!

Last updated: Dec 23 2024 at 13:07 UTC