[PATCH][X86] SSE2 cvtsd2ss instruction expects two operands

[Cameron McInally]

Hey Quentin,

On Mon, Oct 21, 2013 at 8:05 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> Hi Cameron,
>
> I do not think this is quite right.
>
> CVTSD2SS has three operands for the SSE variant.
> Indeed, the converted operand is set to the low quadword (or doubleword) of
> the destination register ($dst), and the high quadword (doublewords) are
> left unchanged ($src1).
>
> CVTSD2SS does what you want for the FP variant (one source one destination):
>
> def CVTSD2SSrr  : SDI<0x5A, MRMSrcReg, (outs FR32:$dst), (ins FR64:$src),
>                       "cvtsd2ss\t{$src, $dst|$dst, $src}",
>                       [(set FR32:$dst, (fround FR64:$src))],
>                       IIC_SSE_CVT_Scalar_RR>, Sched<[WriteCvtF2F]>;
>
>
> Am I missing something?

This may be a difference in terminology. I will try to be more explicit.

The SSE2 cvtsd2ss instruction expects two operands according to the
"Intel Architecture Instructions Set Extensions Programming
Reference", i.e. 319433-016 Oct 2013. [Note: It may also be of
interest that the VEX/EVEX encoded variant of this instruction expects
three+ operands.]

I also agree that LLVM's CVTSD2SSrr pattern is correct. However, the
Int_CVTSD2SSrr pattern is not correct.

let Constraints = "$src1 = $dst" in {
def Int_CVTSD2SSrr: I<0x5A, MRMSrcReg,
                       (outs VR128:$dst), (ins VR128:$src1, VR128:$src2),
                       "cvtsd2ss\t{$src2, $src1, $dst|$dst, $src1, $src2}",
                       [(set VR128:$dst,
                         (int_x86_sse2_cvtsd2ss VR128:$src1, VR128:$src2))],
                       IIC_SSE_CVT_Scalar_RR>, XD, Requires<[UseSSE2]>,
                       Sched<[WriteCvtF2F]>;
}

This pattern will produce an assembly instruction with 3 operands.
[Note: the third operand is necessary for the VEX-encoded instruction,
but is superfluous for the SSE2 instruction.]

Incorrect SSE2 usage: cvtsd2ss %xmm0, %xmm2, %xmm2
Correct SSE2 usage: cvtsd2ss %xmm0, %xmm2

My compiler currently uses a version of GNU ld, which issues an error
with the three operand instruction:

[scrubbed]/sse2-cvtsd2ss-1_1.s: Assembler messages:
[scrubbed]/sse2-cvtsd2ss-1_1.s:209: Error: number of operands mismatch
for `cvtsd2ss'

I may have wrongly assumed that this patch was trivial, so did not
prepare a reduced test case. And, unfortunately, my test case and IR
is incompatible with ToT Clang/LLVM. If there is any further confusion
surrounding this patch, I will be happy to create a small compatible
test case for this problem.

Also, if this confusion still isn't resolved, I would suggest looking
at the similar Int_CVTSS2SDrr pattern in ToT.

-Cameron