Cranelift: extra movdqa when lowering icmp

[abrown]

#3886 ports CLIF icmp operations to ISLE but in the process added new, unnecessary moves to the lowering:

wasmtime/cranelift/filetests/filetests/isa/x64/simd-comparison-legalize.clif

Lines 61 to 64 in e92cbfb

	; Inst 2: movdqa %xmm1, %xmm2
	; Inst 3: movdqa %xmm0, %xmm1
	; Inst 4: pmaxsw %xmm2, %xmm1
	; Inst 5: pcmpeqw %xmm1, %xmm0

The two initial movdqa instructions are not needed with the proper use of registers; my current thinking is that ISLE's move elision may not be working as it should but this could be due to re-ordering of operands in a lowering rule that ISLE cannot resolve.

[abrown]

This may be similar to #3744.

[alexcrichton]

With the new regalloc the current test looks like:

wasmtime/cranelift/filetests/filetests/isa/x64/simd-comparison-legalize.clif

Lines 47 to 49 in f85047b

	; movdqa %xmm0, %xmm5
	; pmaxsw %xmm5, %xmm5, %xmm1
	; pcmpeqw %xmm0, %xmm0, %xmm5

I'm not familiar enough with these instructions to know whether this is fixed though (there's still a single movdqa)

[abrown]

I mean... this just looks wrong: I can never quite remember Cranelift's order of operands when using the x64 backend but our use of % makes this look like AT&T syntax (i.e., destination on the right). %xmm0 and %xmm1 are probably used for ABI arguments so it looks like we copy %xmm0 to %xmm5 in order to use the value twice--so far so good. But then the problem: I think what we need to do is to a) find the maximum between %xmm5 and %xmm1 and put that into %xmm5 and b) equality-compare %xmm0 and %xmm5 and put that into %xmm0 for the return. but the last two instructions there don't quite say that! Looks like it should be:

pmaxsw %xmm5, %xmm1, %xmm5
pcmpeqw %xmm0, %xmm5, %xmm0

But even this is a bit silly because those SIMD instructions aren't really being used in their three-operand form.

---

@alexcrichton, I think this issue is fixed though 😁. The extra move is gone but we should resolve this debug output question...

[cfallin]

@abrown the output of clif-util compile on the function is:

Disassembly of 21 bytes:
   0:   55                      push    rbp
   1:   48 89 e5                mov     rbp, rsp
   4:   66 0f 6f e8             movdqa  xmm5, xmm0
   8:   66 0f ee e9             pmaxsw  xmm5, xmm1
   c:   66 0f 75 c5             pcmpeqw xmm0, xmm5
  10:   48 89 ec                mov     rsp, rbp
  13:   5d                      pop     rbp
  14:   c3                      ret

so this looks right in actuality (result is put in xmm0). I think it's a pretty-printing bug (argument swap) actually -- PR incoming to fix that.

The extra move is indeed elided now, so I think we can close this particular issue.