<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div apple-content-edited="true">Hi Cameron,

</div>
<br><div><div>On Oct 22, 2013, at 10:02 AM, Cameron McInally <<a href="mailto:cameron.mcinally@nyu.edu">cameron.mcinally@nyu.edu</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Hey Quentin,<br><br>On Mon, Oct 21, 2013 at 8:05 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><blockquote type="cite">Hi Cameron,<br><br>I do not think this is quite right.<br><br>CVTSD2SS has three operands for the SSE variant.<br>Indeed, the converted operand is set to the low quadword (or doubleword) of<br>the destination register ($dst), and the high quadword (doublewords) are<br>left unchanged ($src1).<br><br>CVTSD2SS does what you want for the FP variant (one source one destination):<br><br>def CVTSD2SSrr  : SDI<0x5A, MRMSrcReg, (outs FR32:$dst), (ins FR64:$src),<br>                     "cvtsd2ss\t{$src, $dst|$dst, $src}",<br>                     [(set FR32:$dst, (fround FR64:$src))],<br>                     IIC_SSE_CVT_Scalar_RR>, Sched<[WriteCvtF2F]>;<br><br><br>Am I missing something?<br></blockquote><br>This may be a difference in terminology.</div></blockquote><div>Indeed!</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">I will try to be more explicit.<br><br>The SSE2 cvtsd2ss instruction expects two operands according to the<br>"Intel Architecture Instructions Set Extensions Programming<br>Reference", i.e. 319433-016 Oct 2013. [Note: It may also be of<br>interest that the VEX/EVEX encoded variant of this instruction expects<br>three+ operands.]<br><br>I also agree that LLVM's CVTSD2SSrr pattern is correct. However, the<br>Int_CVTSD2SSrr pattern is not correct.<br><br>let Constraints = "$src1 = $dst" in {<br>def Int_CVTSD2SSrr: I<0x5A, MRMSrcReg,<br>                      (outs VR128:$dst), (ins VR128:$src1, VR128:$src2),<br>                      "cvtsd2ss\t{$src2, $src1, $dst|$dst, $src1, $src2}",<br>                      [(set VR128:$dst,<br>                        (int_x86_sse2_cvtsd2ss VR128:$src1, VR128:$src2))],<br>                      IIC_SSE_CVT_Scalar_RR>, XD, Requires<[UseSSE2]>,<br>                      Sched<[WriteCvtF2F]>;<br>}<br><br>This pattern will produce an assembly instruction with 3 operands.<br>[Note: the third operand is necessary for the VEX-encoded instruction,<br>but is superfluous for the SSE2 instruction.]<br><br>Incorrect SSE2 usage: cvtsd2ss %xmm0, %xmm2, %xmm2<br>Correct SSE2 usage: cvtsd2ss %xmm0, %xmm2<br></div></blockquote><div>I see, my mistake.</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>My compiler currently uses a version of GNU ld, which issues an error<br>with the three operand instruction:<br><br>[scrubbed]/sse2-cvtsd2ss-1_1.s: Assembler messages:<br>[scrubbed]/sse2-cvtsd2ss-1_1.s:209: Error: number of operands mismatch<br>for `cvtsd2ss'<br><br>I may have wrongly assumed that this patch was trivial, so did not<br>prepare a reduced test case. And, unfortunately, my test case and IR<br>is incompatible with ToT Clang/LLVM. If there is any further confusion<br>surrounding this patch, I will be happy to create a small compatible<br>test case for this problem.<br></div></blockquote><div>Generally speaking, if you can come up with a test case it would be best even for trivial fixes.</div><div>With a test case, this would look good for me.</div><div><br></div><div>Cheers,</div><div>-Quentin</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br>Also, if this confusion still isn't resolved, I would suggest looking<br>at the similar Int_CVTSS2SDrr pattern in ToT.<br><br>-Cameron</div></blockquote></div><br></body></html>