wasm: manual `i16x8.narrow_i32x4_s` does not optimize well

[folkertdev]

https://godbolt.org/z/TTxnMMEaM

define <8 x i16> @narrow_manual(<4 x i32> %a, <4 x i32> %b) unnamed_addr {
bb2:
  %0 = shufflevector <4 x i32> %a, <4 x i32> %b, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
  %1 = tail call <8 x i32> @llvm.smax.v8i32(<8 x i32> %0, <8 x i32> splat (i32 -32768))
  %2 = tail call <8 x i32> @llvm.smin.v8i32(<8 x i32> %1, <8 x i32> splat (i32 32767))
  %3 = trunc nsw <8 x i32> %2 to <8 x i16>
  ret <8 x i16> %3
}

declare <8 x i32> @llvm.smin.v8i32(<8 x i32>, <8 x i32>) #2

declare <8 x i32> @llvm.smax.v8i32(<8 x i32>, <8 x i32>) #2

should optimize into a i16x8.narrow_i32x4_s. In the linked godbolt, we see that x86_64 and aarch64 are able to make this optimization. (s390x is not, see #153655).

Instead for wasm we get

narrow_manual:
        local.get       0
        v128.const      -32768, -32768, -32768, -32768
        local.tee       2
        i32x4.max_s
        v128.const      32767, 32767, 32767, 32767
        local.tee       0
        i32x4.min_s
        v128.const      65535, 65535, 65535, 65535
        local.tee       3
        v128.and
        local.get       1
        local.get       2
        i32x4.max_s
        local.get       0
        i32x4.min_s
        local.get       3
        v128.and
        i16x8.narrow_i32x4_u
        end_function

narrow_builtin:
        local.get       0
        local.get       1
        i16x8.narrow_i32x4_s
        end_function

[llvmbot]

@llvm/issue-subscribers-backend-webassembly

Author: Folkert de Vries (folkertdev)

https://godbolt.org/z/TTxnMMEaM

define &lt;8 x i16&gt; @<!-- -->narrow_manual(&lt;4 x i32&gt; %a, &lt;4 x i32&gt; %b) unnamed_addr {
bb2:
  %0 = shufflevector &lt;4 x i32&gt; %a, &lt;4 x i32&gt; %b, &lt;8 x i32&gt; &lt;i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7&gt;
  %1 = tail call &lt;8 x i32&gt; @<!-- -->llvm.smax.v8i32(&lt;8 x i32&gt; %0, &lt;8 x i32&gt; splat (i32 -32768))
  %2 = tail call &lt;8 x i32&gt; @<!-- -->llvm.smin.v8i32(&lt;8 x i32&gt; %1, &lt;8 x i32&gt; splat (i32 32767))
  %3 = trunc nsw &lt;8 x i32&gt; %2 to &lt;8 x i16&gt;
  ret &lt;8 x i16&gt; %3
}

declare &lt;8 x i32&gt; @<!-- -->llvm.smin.v8i32(&lt;8 x i32&gt;, &lt;8 x i32&gt;) #<!-- -->2

declare &lt;8 x i32&gt; @<!-- -->llvm.smax.v8i32(&lt;8 x i32&gt;, &lt;8 x i32&gt;) #<!-- -->2

should optimize into a i16x8.narrow_i32x4_s. In the linked godbolt, we see that x86_64 and aarch64 are able to make this optimization. (s390x is not, see #153655).

Instead for wasm we get

narrow_manual:
        local.get       0
        v128.const      -32768, -32768, -32768, -32768
        local.tee       2
        i32x4.max_s
        v128.const      32767, 32767, 32767, 32767
        local.tee       0
        i32x4.min_s
        v128.const      65535, 65535, 65535, 65535
        local.tee       3
        v128.and
        local.get       1
        local.get       2
        i32x4.max_s
        local.get       0
        i32x4.min_s
        local.get       3
        v128.and
        i16x8.narrow_i32x4_u
        end_function

narrow_builtin:
        local.get       0
        local.get       1
        i16x8.narrow_i32x4_s
        end_function

[badumbatish]

looks like a combine or isel to me

[folkertdev]

I've looked into this a bit. X86 does something fairly manual by specifying combineTruncate and combineTruncateWithSat. It's pretty gnarly. Aarch64 seems to have a much cleaner solution, using the generic

def truncssat_s : SDNode<"ISD::TRUNCATE_SSAT_S", SDTIntTruncOp>;

and mapping that to its saturating truncate instruction

defm SQXTN  : SIMDMixedTwoVector<0, 0b10100, "sqxtn", truncssat_s>;

TRUNCATE_SSAT_S appears to be recognized/inserted by these functions

  // Combine trunc(smin(smax(x, C1), C2)) -> truncssat_s(x)
  // or      trunc(smax(smin(x, C2), C1)) -> truncssat_s(x).
  bool matchTruncSSatS(MachineInstr &MI, Register &MatchInfo) const;
  void applyTruncSSatS(MachineInstr &MI, Register &MatchInfo) const;

which are a part of this combine rule

def trunc_ssats : GICombineRule<
  (defs root:$root, register_matchinfo:$matchinfo),
  (match (G_TRUNC $dst, $src):$root,
         [{ return Helper.matchTruncSSatS(*${root}, ${matchinfo}); }]),
  (apply [{ Helper.applyTruncSSatS(*${root}, ${matchinfo}); }])>;

def truncsat_combines : GICombineGroup<[trunc_ssats, trunc_ssatu, trunc_usatu, truncusatu_to_fptouisat]>;

Which, from what I can tell, is used only by the PostLegalizerCombiner for riscv, aarch64 and amdgpu. As far as I can tell, wasm does not define a PostLegalizerCombiner. Should that be added (it seems kind of boilerplaytey, even for targets that keep it simple like https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/Mips/MipsPostLegalizerCombiner.cpp

Or maybe there is some simpler way using this?

Another problem I ran into while attempting to solve this issue for s390x, and I think the same applies here, is that I don't see a good way handling a vector of a size larger than the registers for the target support (e.g. <8 x i32>). But I may just be missing some standard trick there.

[dschuff]

I guess you may have figured this out because your proposed PR just has some tablegen patterns, but the combine rule you posted and the LegalizerCombiner are for Global ISel, not SDag ISel (which is why wasm doesn't have one). I guess those just duplicate the approach you ended up using?