Stream: git-wasmtime

Topic: wasmtime / PR #2168 CL/newBE/x64: Lowering of scalar shif...


view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 08:04):

julian-seward1 opened PR #2168 from fix-no-shift-by-imm to main:

The logic for generation of shifts-by-immediate was not quite right. The result was that even
shifts by an amount known at compile time were being done by moving the shift immediate into %cl
and then doing a variable shift by %cl. The effect is worse than it sounds, because all of
those shift constants are small and often used in multiple places, so they were GVN'd up and
often ended up at the entry block of the function. Hence these were connected to the use points
by long live ranges which got spilled. So all in all, most of the win here comes from avoiding
spilling.

The problem was caused by this line, in the Opcode::Ishl | Opcode::Ushr .. case:

   let (count, rhs) = if let Some(cst) = ctx.get_constant(inputs[1].insn) {

inputs[] appears to refer to this CLIF instruction's inputs, and bizarrely inputs[].insn all
refer to the instruction (the shift) itself. Hence ctx.get_constant(inputs[1].insn) asks
"does this shift instruction produce a constant" to which the answer is always "no", so the
shift-by-unknown amount code is always generated. The fix here is to change that expression to

   let (count, rhs) = if let Some(cst) = ctx.get_input(insn, 1).constant {

get_input's result conveniently includes a constant field of type Option<u64>, so we just
use that instead.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 08:06):

julian-seward1 requested bnjbvr for a review on PR #2168.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 09:46):

bnjbvr submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 27 2020 at 09:48):

bnjbvr merged PR #2168.


Last updated: Dec 23 2024 at 12:05 UTC