<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;">Hi Cameron,<div><br></div><div>What you can probably do is having a very small test case like this:</div><div><br></div><div>$ cat test.ll</div><div>; RUN llc -mtriple x86_64 -matrr=+sse2 < %s -o - | FileCheck %s</div><div><br></div><div>define @fct(<some args>) {</div><div>; CHECK-NOT: <expected instruction> <regex op1>, <regex op2>, <regex op3></div><div><span class="Apple-tab-span" style="white-space:pre"> </span><simple use of your instrinsic></div><div>}</div><div><br></div><div>Basically, you just need to write a test case that use the intrinsic you want (no need to be a real workload) and check that it does not have three operands.</div><div>It would be best if you can actually check that your intrinsic is generated but I am not sure how it would play with CHECK-NOT. You can easily figure that out with a quick test :).</div><div><br></div><div>Regarding the RUN command, you will probably need to adapt the triple or mattr arguments, I did not check if it is the right syntax.</div><div>As soon as you have something you think is okay to go, name the test file appropriately and move it into test/CodeGen/X86. Then 'svn add' this file and produce the new diff to send to the mailing list.</div><div><br></div><div>By doing this, you would have a patch with a test case ;).</div><div><br></div><div>Cheers,</div><div><div apple-content-edited="true">
<div style="color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">-Quentin</div>
</div>
<br><div><div>On Oct 22, 2013, at 2:42 PM, Cameron McInally <<a href="mailto:cameron.mcinally@nyu.edu">cameron.mcinally@nyu.edu</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">On Tue, Oct 22, 2013 at 2:40 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><blockquote type="cite">Hi Cameron,<br><br>On Oct 22, 2013, at 10:02 AM, Cameron McInally <<a href="mailto:cameron.mcinally@nyu.edu">cameron.mcinally@nyu.edu</a>><br>wrote:<br><br>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>><br>wrote:<br><br>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><br><br>This may be a difference in terminology.<br><br>Indeed!<br><br>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><br>I see, my mistake.<br><br><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><br>Generally speaking, if you can come up with a test case it would be best<br>even for trivial fixes.<br>With a test case, this would look good for me.<br></blockquote><br>I've attached the .ll output for a rather thorough test case that<br>makes use of the llvm.x86.sse2.cvtsd2ss intrinsic with -msse2.<br><br>Clang's assembler does indeed complain about the extra operand. There<br>is, of course, no such complaint when directly producing the object<br>file. ;)<br><br>[scrubbed]-MacBook-Air:bin [scrubbed]$ ./clang -msse2 test.c -S<br>[scrubbed]-MacBook-Air:bin [scrubbed]$ grep cvtsd2ss test.s<br>cvtsd2ss %xmm1, %xmm0, %xmm0<br>cvtsd2ss -144(%rbp), %xmm0<br>[scrubbed]-MacBook-Air:bin [scrubbed]$ ./clang -msse2 test.s<br>test.s:37:25: error: invalid operand for instruction<br> cvtsd2ss %xmm1, %xmm0, %xmm0<br> ^~~~~<br>[scrubbed]-MacBook-Air:bin [scrubbed]$ ./clang -msse2 test.c<br>[scrubbed]-MacBook-Air:bin [scrubbed]$<br><br>Unfortunately, I do not have enough experience with Clang's toolchain<br>and test harness to suggest a plan of attack for catching bugs of this<br>class. Hopefully someone with a better understanding of the process<br>can chime in.<br><br>-Cameron<br><span><test.ll></span></blockquote></div><br></div></body></html>